Message ID | 1293160708-30881-7-git-send-email-tamura.yoshiaki@lab.ntt.co.jp |
---|---|
State | New |
Headers | show |
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
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. */ + vdev->vq[i].last_avail_idx -= vdev->vq[i].inuse; + vdev->vq[i].inuse = 0; if (vdev->vq[i].pa) { uint16_t nheads;
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> --- hw/virtio.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)