| Message ID | 1471596663-31298-1-git-send-email-felipe@nutanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi On Sat, Aug 20, 2016 at 3:28 AM Felipe Franciosi <felipe@nutanix.com> wrote: > Vhost-user requires an early GET_FEATURES call to determine if the > slave supports protocol feature negotiation. An extra GET_FEATURES > call is made after vhost_backend_init() to actually set the device > features. > > This patch moves the actual setting of the device features to both > implementations (kernel/user) of vhost_backend_init(), therefore > eliminating the need for a second call. > > Signed-off-by: Felipe Franciosi <felipe@nutanix.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > hw/virtio/vhost-backend.c | 27 ++++++++++++++++++--------- > hw/virtio/vhost-user.c | 1 + > hw/virtio/vhost.c | 9 --------- > 3 files changed, 19 insertions(+), 18 deletions(-) > > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c > index 7681f15..a519fe2 100644 > --- a/hw/virtio/vhost-backend.c > +++ b/hw/virtio/vhost-backend.c > @@ -25,15 +25,6 @@ static int vhost_kernel_call(struct vhost_dev *dev, > unsigned long int request, > return ioctl(fd, request, arg); > } > > -static int vhost_kernel_init(struct vhost_dev *dev, void *opaque) > -{ > - assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL); > - > - dev->opaque = opaque; > - > - return 0; > -} > - > static int vhost_kernel_cleanup(struct vhost_dev *dev) > { > int fd = (uintptr_t) dev->opaque; > @@ -172,6 +163,24 @@ static int vhost_kernel_get_vq_index(struct vhost_dev > *dev, int idx) > return idx - dev->vq_index; > } > > +static int vhost_kernel_init(struct vhost_dev *dev, void *opaque) > +{ > + uint64_t features; > + int r; > + > + assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL); > + > + dev->opaque = opaque; > + > + r = vhost_kernel_get_features(dev, &features); > + if (r < 0) { > + return r; > + } > + dev->features = features; > + > + return 0; > +} > + > static const VhostOps kernel_ops = { > .backend_type = VHOST_BACKEND_TYPE_KERNEL, > .vhost_backend_init = vhost_kernel_init, > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index b57454a..684f6d7 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -578,6 +578,7 @@ static int vhost_user_init(struct vhost_dev *dev, void > *opaque) > if (err < 0) { > return err; > } > + dev->features = features; > > if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) { > dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES; > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index 3d0c807..cb9870a 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -1037,7 +1037,6 @@ static void vhost_virtqueue_cleanup(struct > vhost_virtqueue *vq) > int vhost_dev_init(struct vhost_dev *hdev, void *opaque, > VhostBackendType backend_type, uint32_t > busyloop_timeout) > { > - uint64_t features; > int i, r, n_initialized_vqs = 0; > > hdev->migration_blocker = NULL; > @@ -1063,12 +1062,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void > *opaque, > goto fail; > } > > - r = hdev->vhost_ops->vhost_get_features(hdev, &features); > - if (r < 0) { > - VHOST_OPS_DEBUG("vhost_get_features failed"); > - goto fail; > - } > - > for (i = 0; i < hdev->nvqs; ++i, ++n_initialized_vqs) { > r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + i); > if (r < 0) { > @@ -1086,8 +1079,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void > *opaque, > } > } > > - hdev->features = features; > - > hdev->memory_listener = (MemoryListener) { > .begin = vhost_begin, > .commit = vhost_commit, > -- > 1.9.5 > > > -- Marc-André Lureau
Thanks for the reminder. 2.8 is open now so I can integrate this. I would appreciate it if you could rebase this top of commit d1b4259f1ab18af24e6a297edb6a8f71691f3256 Author: Maxime Coquelin <maxime.coquelin@redhat.com> Date: Tue Sep 13 15:30:30 2016 +0200 virtio-bus: Plug devices after features are negotiated test and repost if it still works. Thanks! On Mon, Sep 19, 2016 at 05:25:49PM +0000, Felipe Franciosi wrote: > Gentle ping. > > > > Thanks, > > Felipe > > > > From: Marc-André Lureau <marcandre.lureau@gmail.com> > Date: Thursday, 25 August 2016 at 12:14 > To: Felipe Franciosi <felipe@nutanix.com>, "qemu-devel@nongnu.org" > <qemu-devel@nongnu.org>, "Michael S. Tsirkin" <mst@redhat.com> > Subject: Re: [Qemu-devel] [PATCH] Avoid additional GET_FEATURES call on > vhost-user > > > > Hi > > > > On Sat, Aug 20, 2016 at 3:28 AM Felipe Franciosi <felipe@nutanix.com> > wrote: > > Vhost-user requires an early GET_FEATURES call to determine if the > slave supports protocol feature negotiation. An extra GET_FEATURES > call is made after vhost_backend_init() to actually set the device > features. > > This patch moves the actual setting of the device features to both > implementations (kernel/user) of vhost_backend_init(), therefore > eliminating the need for a second call. > > Signed-off-by: Felipe Franciosi <felipe@nutanix.com> > > > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > --- > hw/virtio/vhost-backend.c | 27 ++++++++++++++++++--------- > hw/virtio/vhost-user.c | 1 + > hw/virtio/vhost.c | 9 --------- > 3 files changed, 19 insertions(+), 18 deletions(-) > > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c > index 7681f15..a519fe2 100644 > --- a/hw/virtio/vhost-backend.c > +++ b/hw/virtio/vhost-backend.c > @@ -25,15 +25,6 @@ static int vhost_kernel_call(struct vhost_dev *dev, > unsigned long int request, > return ioctl(fd, request, arg); > } > > -static int vhost_kernel_init(struct vhost_dev *dev, void *opaque) > -{ > - assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL); > - > - dev->opaque = opaque; > - > - return 0; > -} > - > static int vhost_kernel_cleanup(struct vhost_dev *dev) > { > int fd = (uintptr_t) dev->opaque; > @@ -172,6 +163,24 @@ static int vhost_kernel_get_vq_index(struct > vhost_dev *dev, int idx) > return idx - dev->vq_index; > } > > +static int vhost_kernel_init(struct vhost_dev *dev, void *opaque) > +{ > + uint64_t features; > + int r; > + > + assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL); > + > + dev->opaque = opaque; > + > + r = vhost_kernel_get_features(dev, &features); > + if (r < 0) { > + return r; > + } > + dev->features = features; > + > + return 0; > +} > + > static const VhostOps kernel_ops = { > .backend_type = VHOST_BACKEND_TYPE_KERNEL, > .vhost_backend_init = vhost_kernel_init, > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index b57454a..684f6d7 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -578,6 +578,7 @@ static int vhost_user_init(struct vhost_dev *dev, > void *opaque) > if (err < 0) { > return err; > } > + dev->features = features; > > if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) > { > dev->backend_features |= 1ULL << > VHOST_USER_F_PROTOCOL_FEATURES; > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index 3d0c807..cb9870a 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -1037,7 +1037,6 @@ static void vhost_virtqueue_cleanup(struct > vhost_virtqueue *vq) > int vhost_dev_init(struct vhost_dev *hdev, void *opaque, > VhostBackendType backend_type, uint32_t > busyloop_timeout) > { > - uint64_t features; > int i, r, n_initialized_vqs = 0; > > hdev->migration_blocker = NULL; > @@ -1063,12 +1062,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void > *opaque, > goto fail; > } > > - r = hdev->vhost_ops->vhost_get_features(hdev, &features); > - if (r < 0) { > - VHOST_OPS_DEBUG("vhost_get_features failed"); > - goto fail; > - } > - > for (i = 0; i < hdev->nvqs; ++i, ++n_initialized_vqs) { > r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + > i); > if (r < 0) { > @@ -1086,8 +1079,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void > *opaque, > } > } > > - hdev->features = features; > - > hdev->memory_listener = (MemoryListener) { > .begin = vhost_begin, > .commit = vhost_commit, > -- > 1.9.5 > > > -- > > Marc-André Lureau >
> On 19 Sep 2016, at 18:42, Michael S. Tsirkin <mst@redhat.com> wrote: > > Thanks for the reminder. 2.8 is open now so I can integrate this. > I would appreciate it if you could rebase this top of > > commit d1b4259f1ab18af24e6a297edb6a8f71691f3256 > Author: Maxime Coquelin <maxime.coquelin@redhat.com> > Date: Tue Sep 13 15:30:30 2016 +0200 > virtio-bus: Plug devices after features are negotiated > > test and repost if it still works. > > Thanks! No problem! Done! :) http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg05582.html Felipe > > > On Mon, Sep 19, 2016 at 05:25:49PM +0000, Felipe Franciosi wrote: >> Gentle ping. >> >> >> >> Thanks, >> >> Felipe >> >> >> >> From: Marc-André Lureau <marcandre.lureau@gmail.com> >> Date: Thursday, 25 August 2016 at 12:14 >> To: Felipe Franciosi <felipe@nutanix.com>, "qemu-devel@nongnu.org" >> <qemu-devel@nongnu.org>, "Michael S. Tsirkin" <mst@redhat.com> >> Subject: Re: [Qemu-devel] [PATCH] Avoid additional GET_FEATURES call on >> vhost-user >> >> >> >> Hi >> >> >> >> On Sat, Aug 20, 2016 at 3:28 AM Felipe Franciosi <felipe@nutanix.com> >> wrote: >> >> Vhost-user requires an early GET_FEATURES call to determine if the >> slave supports protocol feature negotiation. An extra GET_FEATURES >> call is made after vhost_backend_init() to actually set the device >> features. >> >> This patch moves the actual setting of the device features to both >> implementations (kernel/user) of vhost_backend_init(), therefore >> eliminating the need for a second call. >> >> Signed-off-by: Felipe Franciosi <felipe@nutanix.com> >> >> >> >> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> >> >> >> --- >> hw/virtio/vhost-backend.c | 27 ++++++++++++++++++--------- >> hw/virtio/vhost-user.c | 1 + >> hw/virtio/vhost.c | 9 --------- >> 3 files changed, 19 insertions(+), 18 deletions(-) >> >> diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c >> index 7681f15..a519fe2 100644 >> --- a/hw/virtio/vhost-backend.c >> +++ b/hw/virtio/vhost-backend.c >> @@ -25,15 +25,6 @@ static int vhost_kernel_call(struct vhost_dev *dev, >> unsigned long int request, >> return ioctl(fd, request, arg); >> } >> >> -static int vhost_kernel_init(struct vhost_dev *dev, void *opaque) >> -{ >> - assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL); >> - >> - dev->opaque = opaque; >> - >> - return 0; >> -} >> - >> static int vhost_kernel_cleanup(struct vhost_dev *dev) >> { >> int fd = (uintptr_t) dev->opaque; >> @@ -172,6 +163,24 @@ static int vhost_kernel_get_vq_index(struct >> vhost_dev *dev, int idx) >> return idx - dev->vq_index; >> } >> >> +static int vhost_kernel_init(struct vhost_dev *dev, void *opaque) >> +{ >> + uint64_t features; >> + int r; >> + >> + assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL); >> + >> + dev->opaque = opaque; >> + >> + r = vhost_kernel_get_features(dev, &features); >> + if (r < 0) { >> + return r; >> + } >> + dev->features = features; >> + >> + return 0; >> +} >> + >> static const VhostOps kernel_ops = { >> .backend_type = VHOST_BACKEND_TYPE_KERNEL, >> .vhost_backend_init = vhost_kernel_init, >> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c >> index b57454a..684f6d7 100644 >> --- a/hw/virtio/vhost-user.c >> +++ b/hw/virtio/vhost-user.c >> @@ -578,6 +578,7 @@ static int vhost_user_init(struct vhost_dev *dev, >> void *opaque) >> if (err < 0) { >> return err; >> } >> + dev->features = features; >> >> if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) >> { >> dev->backend_features |= 1ULL << >> VHOST_USER_F_PROTOCOL_FEATURES; >> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c >> index 3d0c807..cb9870a 100644 >> --- a/hw/virtio/vhost.c >> +++ b/hw/virtio/vhost.c >> @@ -1037,7 +1037,6 @@ static void vhost_virtqueue_cleanup(struct >> vhost_virtqueue *vq) >> int vhost_dev_init(struct vhost_dev *hdev, void *opaque, >> VhostBackendType backend_type, uint32_t >> busyloop_timeout) >> { >> - uint64_t features; >> int i, r, n_initialized_vqs = 0; >> >> hdev->migration_blocker = NULL; >> @@ -1063,12 +1062,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void >> *opaque, >> goto fail; >> } >> >> - r = hdev->vhost_ops->vhost_get_features(hdev, &features); >> - if (r < 0) { >> - VHOST_OPS_DEBUG("vhost_get_features failed"); >> - goto fail; >> - } >> - >> for (i = 0; i < hdev->nvqs; ++i, ++n_initialized_vqs) { >> r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + >> i); >> if (r < 0) { >> @@ -1086,8 +1079,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void >> *opaque, >> } >> } >> >> - hdev->features = features; >> - >> hdev->memory_listener = (MemoryListener) { >> .begin = vhost_begin, >> .commit = vhost_commit, >> -- >> 1.9.5 >> >> >> -- >> >> Marc-André Lureau >>
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c index 7681f15..a519fe2 100644 --- a/hw/virtio/vhost-backend.c +++ b/hw/virtio/vhost-backend.c @@ -25,15 +25,6 @@ static int vhost_kernel_call(struct vhost_dev *dev, unsigned long int request, return ioctl(fd, request, arg); } -static int vhost_kernel_init(struct vhost_dev *dev, void *opaque) -{ - assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL); - - dev->opaque = opaque; - - return 0; -} - static int vhost_kernel_cleanup(struct vhost_dev *dev) { int fd = (uintptr_t) dev->opaque; @@ -172,6 +163,24 @@ static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx) return idx - dev->vq_index; } +static int vhost_kernel_init(struct vhost_dev *dev, void *opaque) +{ + uint64_t features; + int r; + + assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL); + + dev->opaque = opaque; + + r = vhost_kernel_get_features(dev, &features); + if (r < 0) { + return r; + } + dev->features = features; + + return 0; +} + static const VhostOps kernel_ops = { .backend_type = VHOST_BACKEND_TYPE_KERNEL, .vhost_backend_init = vhost_kernel_init, diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index b57454a..684f6d7 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -578,6 +578,7 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque) if (err < 0) { return err; } + dev->features = features; if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) { dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES; diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 3d0c807..cb9870a 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1037,7 +1037,6 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq) int vhost_dev_init(struct vhost_dev *hdev, void *opaque, VhostBackendType backend_type, uint32_t busyloop_timeout) { - uint64_t features; int i, r, n_initialized_vqs = 0; hdev->migration_blocker = NULL; @@ -1063,12 +1062,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, goto fail; } - r = hdev->vhost_ops->vhost_get_features(hdev, &features); - if (r < 0) { - VHOST_OPS_DEBUG("vhost_get_features failed"); - goto fail; - } - for (i = 0; i < hdev->nvqs; ++i, ++n_initialized_vqs) { r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + i); if (r < 0) { @@ -1086,8 +1079,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, } } - hdev->features = features; - hdev->memory_listener = (MemoryListener) { .begin = vhost_begin, .commit = vhost_commit,
Vhost-user requires an early GET_FEATURES call to determine if the slave supports protocol feature negotiation. An extra GET_FEATURES call is made after vhost_backend_init() to actually set the device features. This patch moves the actual setting of the device features to both implementations (kernel/user) of vhost_backend_init(), therefore eliminating the need for a second call. Signed-off-by: Felipe Franciosi <felipe@nutanix.com> --- hw/virtio/vhost-backend.c | 27 ++++++++++++++++++--------- hw/virtio/vhost-user.c | 1 + hw/virtio/vhost.c | 9 --------- 3 files changed, 19 insertions(+), 18 deletions(-)