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 <pa...@linux.vnet.ibm.com> > Fixes: bccdef6b ("virtio: recalculate vq->inuse after migration") > CC: qemu-sta...@nongnu.org > --- > > 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