| Message ID | 20181002140947.4107-1-i.maximets@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vhost-user: Don't ask for reply on postcopy mem table set | expand |
* Ilya Maximets (i.maximets@samsung.com) wrote: > According to documentation, NEED_REPLY_MASK should not be set > for VHOST_USER_SET_MEM_TABLE request in postcopy mode. > This restriction was mistakenly applied to 'reply_supported' > variable, which is local and used only for non-postcopy case. > > CC: Dr. David Alan Gilbert <dgilbert@redhat.com> > Fixes: 9bb38019942c ("vhost+postcopy: Send address back to qemu") > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Yes I think that's right; the 2nd part (in vhost_user_set_mem_table) is easy; that's just left overs from before I split it into two. The postcopy side we've already got the reply from the client with the mappings, and now we've sent the OK at the end; so there's no need for the final reply. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > hw/virtio/vhost-user.c | 13 +------------ > 1 file changed, 1 insertion(+), 12 deletions(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index b041343632..c442daa562 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -374,8 +374,6 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, > int fds[VHOST_MEMORY_MAX_NREGIONS]; > int i, fd; > size_t fd_num = 0; > - bool reply_supported = virtio_has_feature(dev->protocol_features, > - VHOST_USER_PROTOCOL_F_REPLY_ACK); > VhostUserMsg msg_reply; > int region_i, msg_i; > > @@ -384,10 +382,6 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, > .hdr.flags = VHOST_USER_VERSION, > }; > > - if (reply_supported) { > - msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK; > - } > - > if (u->region_rb_len < dev->mem->nregions) { > u->region_rb = g_renew(RAMBlock*, u->region_rb, dev->mem->nregions); > u->region_rb_offset = g_renew(ram_addr_t, u->region_rb_offset, > @@ -503,10 +497,6 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, > return -1; > } > > - if (reply_supported) { > - return process_message_reply(dev, &msg); > - } > - > return 0; > } > > @@ -519,8 +509,7 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, > size_t fd_num = 0; > bool do_postcopy = u->postcopy_listen && u->postcopy_fd.handler; > bool reply_supported = virtio_has_feature(dev->protocol_features, > - VHOST_USER_PROTOCOL_F_REPLY_ACK) && > - !do_postcopy; > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > > if (do_postcopy) { > /* Postcopy has enough differences that it's best done in it's own > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 10/02/2018 04:09 PM, Ilya Maximets wrote: > According to documentation, NEED_REPLY_MASK should not be set > for VHOST_USER_SET_MEM_TABLE request in postcopy mode. > This restriction was mistakenly applied to 'reply_supported' > variable, which is local and used only for non-postcopy case. > > CC: Dr. David Alan Gilbert <dgilbert@redhat.com> > Fixes: 9bb38019942c ("vhost+postcopy: Send address back to qemu") > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > --- > hw/virtio/vhost-user.c | 13 +------------ > 1 file changed, 1 insertion(+), 12 deletions(-) > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> Thanks, Maxime
* Ilya Maximets (i.maximets@samsung.com) wrote: > According to documentation, NEED_REPLY_MASK should not be set > for VHOST_USER_SET_MEM_TABLE request in postcopy mode. > This restriction was mistakenly applied to 'reply_supported' > variable, which is local and used only for non-postcopy case. > > CC: Dr. David Alan Gilbert <dgilbert@redhat.com> > Fixes: 9bb38019942c ("vhost+postcopy: Send address back to qemu") > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Queued > --- > hw/virtio/vhost-user.c | 13 +------------ > 1 file changed, 1 insertion(+), 12 deletions(-) > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index b041343632..c442daa562 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -374,8 +374,6 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, > int fds[VHOST_MEMORY_MAX_NREGIONS]; > int i, fd; > size_t fd_num = 0; > - bool reply_supported = virtio_has_feature(dev->protocol_features, > - VHOST_USER_PROTOCOL_F_REPLY_ACK); > VhostUserMsg msg_reply; > int region_i, msg_i; > > @@ -384,10 +382,6 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, > .hdr.flags = VHOST_USER_VERSION, > }; > > - if (reply_supported) { > - msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK; > - } > - > if (u->region_rb_len < dev->mem->nregions) { > u->region_rb = g_renew(RAMBlock*, u->region_rb, dev->mem->nregions); > u->region_rb_offset = g_renew(ram_addr_t, u->region_rb_offset, > @@ -503,10 +497,6 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, > return -1; > } > > - if (reply_supported) { > - return process_message_reply(dev, &msg); > - } > - > return 0; > } > > @@ -519,8 +509,7 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, > size_t fd_num = 0; > bool do_postcopy = u->postcopy_listen && u->postcopy_fd.handler; > bool reply_supported = virtio_has_feature(dev->protocol_features, > - VHOST_USER_PROTOCOL_F_REPLY_ACK) && > - !do_postcopy; > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > > if (do_postcopy) { > /* Postcopy has enough differences that it's best done in it's own > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index b041343632..c442daa562 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -374,8 +374,6 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, int fds[VHOST_MEMORY_MAX_NREGIONS]; int i, fd; size_t fd_num = 0; - bool reply_supported = virtio_has_feature(dev->protocol_features, - VHOST_USER_PROTOCOL_F_REPLY_ACK); VhostUserMsg msg_reply; int region_i, msg_i; @@ -384,10 +382,6 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, .hdr.flags = VHOST_USER_VERSION, }; - if (reply_supported) { - msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK; - } - if (u->region_rb_len < dev->mem->nregions) { u->region_rb = g_renew(RAMBlock*, u->region_rb, dev->mem->nregions); u->region_rb_offset = g_renew(ram_addr_t, u->region_rb_offset, @@ -503,10 +497,6 @@ static int vhost_user_set_mem_table_postcopy(struct vhost_dev *dev, return -1; } - if (reply_supported) { - return process_message_reply(dev, &msg); - } - return 0; } @@ -519,8 +509,7 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev, size_t fd_num = 0; bool do_postcopy = u->postcopy_listen && u->postcopy_fd.handler; bool reply_supported = virtio_has_feature(dev->protocol_features, - VHOST_USER_PROTOCOL_F_REPLY_ACK) && - !do_postcopy; + VHOST_USER_PROTOCOL_F_REPLY_ACK); if (do_postcopy) { /* Postcopy has enough differences that it's best done in it's own
According to documentation, NEED_REPLY_MASK should not be set for VHOST_USER_SET_MEM_TABLE request in postcopy mode. This restriction was mistakenly applied to 'reply_supported' variable, which is local and used only for non-postcopy case. CC: Dr. David Alan Gilbert <dgilbert@redhat.com> Fixes: 9bb38019942c ("vhost+postcopy: Send address back to qemu") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> --- hw/virtio/vhost-user.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-)