| Message ID | 1478697498-29833-1-git-send-email-felipe@nutanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/11/2016 14:18, Felipe Franciosi wrote: > Recent changes on vhost_dev_enable/disable_notifiers() produced a > VirtioBusState vbus variable which can be used instead of the > VIRTIO_BUS() macro. This commit just makes the code a little bit cleaner > and more consistent. > > Signed-off-by: Felipe Franciosi <felipe@nutanix.com> Michael, what do you think? Perhaps it's simplest to just squash the two patches (v2 of "vhost: Update 'ioeventfd_started' with host notifiers" and this one). Paolo > --- > hw/virtio/vhost.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index 1290963..7d29dad 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -1198,20 +1198,18 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) > > virtio_device_stop_ioeventfd(vdev); > for (i = 0; i < hdev->nvqs; ++i) { > - r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, > - true); > + r = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, true); > if (r < 0) { > error_report("vhost VQ %d notifier binding failed: %d", i, -r); > goto fail_vq; > } > } > - VIRTIO_BUS(qbus)->ioeventfd_started = true; > + vbus->ioeventfd_started = true; > > return 0; > fail_vq: > while (--i >= 0) { > - e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, > - false); > + e = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, false); > if (e < 0) { > error_report("vhost VQ %d notifier cleanup error: %d", i, -r); > } > @@ -1230,17 +1228,17 @@ fail: > void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) > { > BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); > + VirtioBusState *vbus = VIRTIO_BUS(qbus); > int i, r; > > for (i = 0; i < hdev->nvqs; ++i) { > - r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, > - false); > + r = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, false); > if (r < 0) { > error_report("vhost VQ %d notifier cleanup failed: %d", i, -r); > } > assert (r >= 0); > } > - VIRTIO_BUS(qbus)->ioeventfd_started = false; > + vbus->ioeventfd_started = false; > virtio_device_start_ioeventfd(vdev); > } > >
> On 9 Nov 2016, at 14:22, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > On 09/11/2016 14:18, Felipe Franciosi wrote: >> Recent changes on vhost_dev_enable/disable_notifiers() produced a >> VirtioBusState vbus variable which can be used instead of the >> VIRTIO_BUS() macro. This commit just makes the code a little bit cleaner >> and more consistent. >> >> Signed-off-by: Felipe Franciosi <felipe@nutanix.com> > > Michael, what do you think? Perhaps it's simplest to just squash the > two patches (v2 of "vhost: Update 'ioeventfd_started' with host > notifiers" and this one). > > Paolo Please, do. I just realised the repeated use of VIRTIO_BUS() after sending the first patch (Update 'ioeventfd_started' with host notifiers) and decided to ship this one too to get them both in if possible. Thanks, Felipe > >> --- >> hw/virtio/vhost.c | 14 ++++++-------- >> 1 file changed, 6 insertions(+), 8 deletions(-) >> >> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c >> index 1290963..7d29dad 100644 >> --- a/hw/virtio/vhost.c >> +++ b/hw/virtio/vhost.c >> @@ -1198,20 +1198,18 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) >> >> virtio_device_stop_ioeventfd(vdev); >> for (i = 0; i < hdev->nvqs; ++i) { >> - r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, >> - true); >> + r = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, true); >> if (r < 0) { >> error_report("vhost VQ %d notifier binding failed: %d", i, -r); >> goto fail_vq; >> } >> } >> - VIRTIO_BUS(qbus)->ioeventfd_started = true; >> + vbus->ioeventfd_started = true; >> >> return 0; >> fail_vq: >> while (--i >= 0) { >> - e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, >> - false); >> + e = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, false); >> if (e < 0) { >> error_report("vhost VQ %d notifier cleanup error: %d", i, -r); >> } >> @@ -1230,17 +1228,17 @@ fail: >> void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) >> { >> BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); >> + VirtioBusState *vbus = VIRTIO_BUS(qbus); >> int i, r; >> >> for (i = 0; i < hdev->nvqs; ++i) { >> - r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, >> - false); >> + r = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, false); >> if (r < 0) { >> error_report("vhost VQ %d notifier cleanup failed: %d", i, -r); >> } >> assert (r >= 0); >> } >> - VIRTIO_BUS(qbus)->ioeventfd_started = false; >> + vbus->ioeventfd_started = false; >> virtio_device_start_ioeventfd(vdev); >> } >> >>
On Wed, Nov 09, 2016 at 02:22:42PM +0100, Paolo Bonzini wrote: > > > On 09/11/2016 14:18, Felipe Franciosi wrote: > > Recent changes on vhost_dev_enable/disable_notifiers() produced a > > VirtioBusState vbus variable which can be used instead of the > > VIRTIO_BUS() macro. This commit just makes the code a little bit cleaner > > and more consistent. > > > > Signed-off-by: Felipe Franciosi <felipe@nutanix.com> > > Michael, what do you think? Perhaps it's simplest to just squash the > two patches (v2 of "vhost: Update 'ioeventfd_started' with host > notifiers" and this one). > > Paolo I think I'll apply both but why bother squashing? > > --- > > hw/virtio/vhost.c | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > > index 1290963..7d29dad 100644 > > --- a/hw/virtio/vhost.c > > +++ b/hw/virtio/vhost.c > > @@ -1198,20 +1198,18 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) > > > > virtio_device_stop_ioeventfd(vdev); > > for (i = 0; i < hdev->nvqs; ++i) { > > - r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, > > - true); > > + r = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, true); > > if (r < 0) { > > error_report("vhost VQ %d notifier binding failed: %d", i, -r); > > goto fail_vq; > > } > > } > > - VIRTIO_BUS(qbus)->ioeventfd_started = true; > > + vbus->ioeventfd_started = true; > > > > return 0; > > fail_vq: > > while (--i >= 0) { > > - e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, > > - false); > > + e = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, false); > > if (e < 0) { > > error_report("vhost VQ %d notifier cleanup error: %d", i, -r); > > } > > @@ -1230,17 +1228,17 @@ fail: > > void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) > > { > > BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); > > + VirtioBusState *vbus = VIRTIO_BUS(qbus); > > int i, r; > > > > for (i = 0; i < hdev->nvqs; ++i) { > > - r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, > > - false); > > + r = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, false); > > if (r < 0) { > > error_report("vhost VQ %d notifier cleanup failed: %d", i, -r); > > } > > assert (r >= 0); > > } > > - VIRTIO_BUS(qbus)->ioeventfd_started = false; > > + vbus->ioeventfd_started = false; > > virtio_device_start_ioeventfd(vdev); > > } > > > >
On 09/11/2016 16:47, Michael S. Tsirkin wrote: > On Wed, Nov 09, 2016 at 02:22:42PM +0100, Paolo Bonzini wrote: >> >> >> On 09/11/2016 14:18, Felipe Franciosi wrote: >>> Recent changes on vhost_dev_enable/disable_notifiers() produced a >>> VirtioBusState vbus variable which can be used instead of the >>> VIRTIO_BUS() macro. This commit just makes the code a little bit cleaner >>> and more consistent. >>> >>> Signed-off-by: Felipe Franciosi <felipe@nutanix.com> >> >> Michael, what do you think? Perhaps it's simplest to just squash the >> two patches (v2 of "vhost: Update 'ioeventfd_started' with host >> notifiers" and this one). >> >> Paolo > > I think I'll apply both but why bother squashing? Just because VIRTIO_BUS(qbus)->ioeventfd_started = true; from the first patch is ugly. :) Paolo >>> --- >>> hw/virtio/vhost.c | 14 ++++++-------- >>> 1 file changed, 6 insertions(+), 8 deletions(-) >>> >>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c >>> index 1290963..7d29dad 100644 >>> --- a/hw/virtio/vhost.c >>> +++ b/hw/virtio/vhost.c >>> @@ -1198,20 +1198,18 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) >>> >>> virtio_device_stop_ioeventfd(vdev); >>> for (i = 0; i < hdev->nvqs; ++i) { >>> - r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, >>> - true); >>> + r = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, true); >>> if (r < 0) { >>> error_report("vhost VQ %d notifier binding failed: %d", i, -r); >>> goto fail_vq; >>> } >>> } >>> - VIRTIO_BUS(qbus)->ioeventfd_started = true; >>> + vbus->ioeventfd_started = true; >>> >>> return 0; >>> fail_vq: >>> while (--i >= 0) { >>> - e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, >>> - false); >>> + e = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, false); >>> if (e < 0) { >>> error_report("vhost VQ %d notifier cleanup error: %d", i, -r); >>> } >>> @@ -1230,17 +1228,17 @@ fail: >>> void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) >>> { >>> BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); >>> + VirtioBusState *vbus = VIRTIO_BUS(qbus); >>> int i, r; >>> >>> for (i = 0; i < hdev->nvqs; ++i) { >>> - r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, >>> - false); >>> + r = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, false); >>> if (r < 0) { >>> error_report("vhost VQ %d notifier cleanup failed: %d", i, -r); >>> } >>> assert (r >= 0); >>> } >>> - VIRTIO_BUS(qbus)->ioeventfd_started = false; >>> + vbus->ioeventfd_started = false; >>> virtio_device_start_ioeventfd(vdev); >>> } >>> >>>
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 1290963..7d29dad 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1198,20 +1198,18 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) virtio_device_stop_ioeventfd(vdev); for (i = 0; i < hdev->nvqs; ++i) { - r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, - true); + r = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, true); if (r < 0) { error_report("vhost VQ %d notifier binding failed: %d", i, -r); goto fail_vq; } } - VIRTIO_BUS(qbus)->ioeventfd_started = true; + vbus->ioeventfd_started = true; return 0; fail_vq: while (--i >= 0) { - e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, - false); + e = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, false); if (e < 0) { error_report("vhost VQ %d notifier cleanup error: %d", i, -r); } @@ -1230,17 +1228,17 @@ fail: void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) { BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); + VirtioBusState *vbus = VIRTIO_BUS(qbus); int i, r; for (i = 0; i < hdev->nvqs; ++i) { - r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i, - false); + r = virtio_bus_set_host_notifier(vbus, hdev->vq_index + i, false); if (r < 0) { error_report("vhost VQ %d notifier cleanup failed: %d", i, -r); } assert (r >= 0); } - VIRTIO_BUS(qbus)->ioeventfd_started = false; + vbus->ioeventfd_started = false; virtio_device_start_ioeventfd(vdev); }
Recent changes on vhost_dev_enable/disable_notifiers() produced a VirtioBusState vbus variable which can be used instead of the VIRTIO_BUS() macro. This commit just makes the code a little bit cleaner and more consistent. Signed-off-by: Felipe Franciosi <felipe@nutanix.com> --- hw/virtio/vhost.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)