Re: [PATCH 05/21] virtio: modify save/load handler to handle inuse varialble.

Michael S. Tsirkin
2010-11-28T09:29:57Z

On Thu, Nov 25, 2010 at 03:06:44PM +0900, Yoshiaki Tamura wrote:
> Modify inuse type to uint16_t, let save/load to handle, and revert
> last_avail_idx with inuse if there are outstanding emulation.
>
> Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>

This changes migration format, so it will break compatibility with
existing drivers. More generally, I think migrating internal
state that is not guest visible is always a mistake
as it ties migration format to an internal implementation
(yes, I know we do this sometimes, but we should at least
try not to add such cases). I think the right thing to do in this case
is to flush outstanding
work when vm is stopped. Then, we are guaranteed that inuse is 0.
I sent patches that do this for virtio net and block.

> ---
> hw/virtio.c | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/hw/virtio.c b/hw/virtio.c
> index 849a60f..5509644 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);
> }
> @@ -711,6 +712,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) {
> virtqueue_init(&vdev->vq[i]);
> --
> 1.7.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html