[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio: fix vring->inuse recalc after migr
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH] virtio: fix vring->inuse recalc after migr |
Date: |
Fri, 16 Dec 2016 10:25:28 +0000 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Thu, Dec 15, 2016 at 04:43:30PM +0100, Halil Pasic wrote:
> Correct recalculation of vring->inuse after migration for
> the corner case where the avail_idx has already wrapped
> but used_idx not yet.
>
> Signed-off-by: Halil Pasic <address@hidden>
> Fixes: bccdef6b ("virtio: recalculate vq->inuse after migration")
> CC: address@hidden
> ---
>
> I think we could also change the type of inuse to uint16_t.
> Would this be considered a good idea?
VRing.num is unsigned int. I would use the same type as num since this
is a size, not an index.
> ---
> hw/virtio/virtio.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 1af2de2..089c6f6 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1855,9 +1855,12 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int
> version_id)
> /*
> * Some devices migrate VirtQueueElements that have been popped
> * from the avail ring but not yet returned to the used ring.
> + * Cast to uint16_t is OK because max ring size is 0x8000. Thus
> + * no the size of largest array indexable by an integral type
> + * can not be represented by the same type problem.
I think it would be safe up to max ring size UINT16_MAX - 1 (we need
that extra value to distinguish between empty and full avail rings)?
The last sentence is hard to understand due to the double negative. I
think you are saying "Since max ring size < UINT16_MAX it's safe to use
uint16_t to represent the size of the ring".
> */
> - vdev->vq[i].inuse = vdev->vq[i].last_avail_idx -
> - vdev->vq[i].used_idx;
> + vdev->vq[i].inuse = (uint16_t)(vdev->vq[i].last_avail_idx -
> + vdev->vq[i].used_idx);
> if (vdev->vq[i].inuse > vdev->vq[i].vring.num) {
> error_report("VQ %d size 0x%x < last_avail_idx 0x%x - "
> "used_idx 0x%x",
> --
> 2.8.4
>
>
signature.asc
Description: PGP signature