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