| Message ID | 20221128164105.1191058-3-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fix vhost-user issues with CI | expand |
On Mon, Nov 28, 2022 at 04:41:00PM +0000, Alex Bennée wrote: > 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> Doesn't this need to be like the last patch in the series? Otherwise bisect will break on CI, right? > --- > v3 > - rm extra line > - fix fn name in comment for virtio_device_started() > --- > include/hw/virtio/virtio.h | 23 ++++++++++++++++++----- > 1 file changed, 18 insertions(+), 5 deletions(-) > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index 0f612067f7..24561e933a 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,16 @@ static inline bool virtio_is_big_endian(VirtIODevice *vdev) > return false; > } > > +/** > + * virtio_device_started() - 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 +445,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) > -- > 2.34.1
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 0f612067f7..24561e933a 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,16 @@ static inline bool virtio_is_big_endian(VirtIODevice *vdev) return false; } +/** + * virtio_device_started() - 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 +445,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)