qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/2] virtio: Set status early in VirtIODevice pa


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH 2/2] virtio: Set status early in VirtIODevice parent object
Date: Wed, 27 Jun 2018 11:21:42 +0100
User-agent: Mutt/1.10.0 (2018-05-17)

On Wed, Jun 27, 2018 at 01:52:48PM +0530, Pankaj Gupta wrote:
>  To process pending requests for driver status 'VIRTIO_CONFIG_S_DRIVER_OK',
>  virtio-rng 'set_status' calls 'is_guest_ready' function. This checks if 
>  virtqueue is ready and status is set to 'VIRTIO_CONFIG_S_DRIVER_OK'. 
> 
>  This call is made before new status is updated in VirtIODevice parent object 
>  and calling function uses old status value.
> 
> Signed-off-by: Pankaj Gupta <address@hidden>
> ---
>  hw/virtio/virtio.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index d4e4d98b59..37dc59a686 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1154,10 +1154,11 @@ int virtio_set_status(VirtIODevice *vdev, uint8_t val)
>              }
>          }
>      }
> +    vdev->status = val;
> +
>      if (k->set_status) {
>          k->set_status(vdev, val);
>      }
> -    vdev->status = val;

This breaks existing ->set_status() callbacks that rely on vdev->status
containing the old value.  Have you audited them?

For example, virtio_net_set_status -> virtio_net_vnet_endian_status uses
vdev->status!

It may be easier to modify virtio-rng.c so that the old status value
isn't used.

Stefan

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]