Re: [PATCH 06/19] virtio: update last_avail_idx when inuse is decreased.

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Dec 24, 2010 at 12:18:15PM +0900, Yoshiaki Tamura wrote:
> virtio save/load is currently sending last_avail_idx, but inuse isn't.
> This causes inconsistent state when using Kemari which replays
> outstanding requests on the secondary.  By letting last_avail_idx to
> be updated after inuse is decreased, it would be possible to replay
> the outstanding requests.  Noth that live migration shouldn't be
> affected because it waits until flushing all requests.  Also in
> conjunction with event-tap, requests inversion should be avoided.
> 
> Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>

I think I understood the request inversion. My question now is,
event-tap transfers inuse events as well, wont the same
request be repeated twice?

> ---
>  hw/virtio.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/virtio.c b/hw/virtio.c
> index 07dbf86..f915c46 100644
> --- a/hw/virtio.c
> +++ b/hw/virtio.c
> @@ -72,7 +72,7 @@ struct VirtQueue
>      VRing vring;
>      target_phys_addr_t pa;
>      uint16_t last_avail_idx;
> -    int inuse;
> +    uint16_t inuse;
>      uint16_t vector;
>      void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq);
>      VirtIODevice *vdev;
> @@ -671,6 +671,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>          qemu_put_be32(f, vdev->vq[i].vring.num);
>          qemu_put_be64(f, vdev->vq[i].pa);
>          qemu_put_be16s(f, &vdev->vq[i].last_avail_idx);
> +        qemu_put_be16s(f, &vdev->vq[i].inuse);
>          if (vdev->binding->save_queue)
>              vdev->binding->save_queue(vdev->binding_opaque, i, f);
>      }
> @@ -710,6 +711,11 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>          vdev->vq[i].vring.num = qemu_get_be32(f);
>          vdev->vq[i].pa = qemu_get_be64(f);
>          qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
> +        qemu_get_be16s(f, &vdev->vq[i].inuse);
> +
> +        /* revert last_avail_idx if there are outstanding emulation. */

if there are outstanding emulation -> if requests
are outstanding in event-tap?

> +        vdev->vq[i].last_avail_idx -= vdev->vq[i].inuse;
> +        vdev->vq[i].inuse = 0;
>  

I don't understand it, if this is all we do we can equivalently
decrement on the sender side and avoid breaking migration compatibility?

>          if (vdev->vq[i].pa) {
>              uint16_t nheads;
> -- 
> 1.7.1.2