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

[Wen Congyang <wency@cn.fujitsu.com>]

On 04/02/2015 09:17 PM, Paolo Bonzini wrote:
> 
> 
> 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.

Dataplane will be disabled before migration. But it should be also
fixed if it has the same problem.

Thanks
Wen Congyang

> 
> 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.
> .
>