| Message ID | 20221125173043.1998075-3-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | continuing efforts to fix vhost-user issues | expand |
Am 25. November 2022 17:30:40 UTC schrieb "Alex Bennée" <alex.bennee@linaro.org>: >The VM status should always preempt the device status for these >checks. This ensures the device is in the correct state when we >suspend the VM prior to migrations. This restores the checks to the >order they where in before the refactoring moved things around. > >While we are at it lets improve our documentation of the various >fields involved and document the two functions. > >Fixes: 9f6bcfd99f (hw/virtio: move vm_running check to virtio_device_started) >Fixes: 259d69c00b (hw/virtio: introduce virtio_device_should_start) >Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com> >--- > include/hw/virtio/virtio.h | 24 +++++++++++++++++++----- > 1 file changed, 19 insertions(+), 5 deletions(-) > >diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h >index 0f612067f7..48f539d0fe 100644 >--- a/include/hw/virtio/virtio.h >+++ b/include/hw/virtio/virtio.h >@@ -133,6 +133,13 @@ struct VirtIODevice > bool broken; /* device in invalid state, needs reset */ > bool use_disabled_flag; /* allow use of 'disable' flag when needed */ > bool disabled; /* device in temporarily disabled state */ >+ /** >+ * @use_started: true if the @started flag should be used to check the >+ * current state of the VirtIO device. Otherwise status bits >+ * should be checked for a current status of the device. >+ * @use_started is only set via QMP and defaults to true for all >+ * modern machines (since 4.1). >+ */ > bool use_started; > bool started; > bool start_on_kick; /* when virtio 1.0 feature has not been negotiated */ >@@ -408,6 +415,17 @@ static inline bool virtio_is_big_endian(VirtIODevice *vdev) > return false; > } > >+ This adds an extra empty line. >+/** >+ * virtio_device_should_start() - check if device started s/virtio_device_should_start/virtio_device_started/ Best regards, Bernhard >+ * @vdev - the VirtIO device >+ * @status - the devices status bits >+ * >+ * Check if the device is started. For most modern machines this is >+ * tracked via the @vdev->started field (to support migration), >+ * otherwise we check for the final negotiated status bit that >+ * indicates everything is ready. >+ */ > static inline bool virtio_device_started(VirtIODevice *vdev, uint8_t status) > { > if (vdev->use_started) { >@@ -428,15 +446,11 @@ static inline bool virtio_device_started(VirtIODevice *vdev, uint8_t status) > */ > static inline bool virtio_device_should_start(VirtIODevice *vdev, uint8_t status) > { >- if (vdev->use_started) { >- return vdev->started; >- } >- > if (!vdev->vm_running) { > return false; > } > >- return status & VIRTIO_CONFIG_S_DRIVER_OK; >+ return virtio_device_started(vdev, status); > } > > static inline void virtio_set_started(VirtIODevice *vdev, bool started)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 0f612067f7..48f539d0fe 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -133,6 +133,13 @@ struct VirtIODevice bool broken; /* device in invalid state, needs reset */ bool use_disabled_flag; /* allow use of 'disable' flag when needed */ bool disabled; /* device in temporarily disabled state */ + /** + * @use_started: true if the @started flag should be used to check the + * current state of the VirtIO device. Otherwise status bits + * should be checked for a current status of the device. + * @use_started is only set via QMP and defaults to true for all + * modern machines (since 4.1). + */ bool use_started; bool started; bool start_on_kick; /* when virtio 1.0 feature has not been negotiated */ @@ -408,6 +415,17 @@ static inline bool virtio_is_big_endian(VirtIODevice *vdev) return false; } + +/** + * virtio_device_should_start() - check if device started + * @vdev - the VirtIO device + * @status - the devices status bits + * + * Check if the device is started. For most modern machines this is + * tracked via the @vdev->started field (to support migration), + * otherwise we check for the final negotiated status bit that + * indicates everything is ready. + */ static inline bool virtio_device_started(VirtIODevice *vdev, uint8_t status) { if (vdev->use_started) { @@ -428,15 +446,11 @@ static inline bool virtio_device_started(VirtIODevice *vdev, uint8_t status) */ static inline bool virtio_device_should_start(VirtIODevice *vdev, uint8_t status) { - if (vdev->use_started) { - return vdev->started; - } - if (!vdev->vm_running) { return false; } - return status & VIRTIO_CONFIG_S_DRIVER_OK; + return virtio_device_started(vdev, status); } static inline void virtio_set_started(VirtIODevice *vdev, bool started)