Re: [Qemu-devel] [Migration Bug? ] Occasionally, the content of VM's memory is inconsistent between Source and Destination of migration

[Paolo Bonzini <pbonzini@redhat.com>]

On 02/04/2015 11:14, Wen Congyang wrote:
> From ebc024702dd3147e0cbdfd173c599103dc87796c Mon Sep 17 00:00:00 2001
> From: Wen Congyang <wency@cn.fujitsu.com>
> Date: Thu, 2 Apr 2015 16:28:17 +0800
> Subject: [PATCH] fix qiov size
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  hw/block/virtio-blk.c          | 15 +++++++++++++++
>  include/hw/virtio/virtio-blk.h |  1 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 000c38d..13967bc 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -33,6 +33,7 @@ VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s)
>      VirtIOBlockReq *req = g_slice_new(VirtIOBlockReq);
>      req->dev = s;
>      req->qiov.size = 0;
> +    req->size = 0;
>      req->next = NULL;
>      req->mr_next = NULL;
>      return req;
> @@ -97,12 +98,20 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
>               * external iovec. It was allocated in submit_merged_requests
>               * to be able to merge requests. */
>              qemu_iovec_destroy(&req->qiov);
> +
> +            /* Restore qiov->size here */
> +            req->qiov.size = req->size;

At this point the iovec has been destroyed.

It's clearer, I think, to just use req->size in
virtio_blk_complete_request (as you suggest in the comment below!).  For
dataplane you'll need to do the same in complete_request_vring.

I'll send a patch to the mailing list, please test it---and then the
block device people can merge it.

Paolo

>          }
>  
>          if (ret) {
>              int p = virtio_ldl_p(VIRTIO_DEVICE(req->dev), &req->out.type);
>              bool is_read = !(p & VIRTIO_BLK_T_OUT);
>              if (virtio_blk_handle_rw_error(req, -ret, is_read)) {
> +                /*
> +                 * FIXME:
> +                 *  The memory may be dirtied on read failure, it will
> +                 *  break live migration.
> +                 */
>                  continue;
>              }
>          }
> @@ -323,6 +332,12 @@ static inline void submit_requests(BlockBackend *blk, MultiReqBuffer *mrb,
>          struct iovec *tmp_iov = qiov->iov;
>          int tmp_niov = qiov->niov;
>  
> +        /*
> +         * Save old qiov->size, which will used in
> +         * virtio_blk_complete_request()
> +         */
> +        mrb->reqs[start]->size = qiov->size;
> +
>          /* mrb->reqs[start]->qiov was initialized from external so we can't
>           * modifiy it here. We need to initialize it locally and then add the
>           * external iovecs. */
> diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
> index b3ffcd9..7d47310 100644
> --- a/include/hw/virtio/virtio-blk.h
> +++ b/include/hw/virtio/virtio-blk.h
> @@ -67,6 +67,7 @@ typedef struct VirtIOBlockReq {
>      struct virtio_blk_inhdr *in;
>      struct virtio_blk_outhdr out;
>      QEMUIOVector qiov;
> +    size_t size;
>      struct VirtIOBlockReq *next;
>      struct VirtIOBlockReq *mr_next;
>      BlockAcctCookie acct;
> -- 2.1.0 PS: I don't check if virtio-scsi, virtio-net... has the similar
> problem. If vhost=on, we can also reproduce this problem.