From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55855) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YdgIE-00010u-Sg for qemu-devel@nongnu.org; Thu, 02 Apr 2015 10:40:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YdgIA-0000HU-Pu for qemu-devel@nongnu.org; Thu, 02 Apr 2015 10:40:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39871) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YdgIA-0000HO-Ip for qemu-devel@nongnu.org; Thu, 02 Apr 2015 10:40:22 -0400 Date: Thu, 2 Apr 2015 22:39:52 +0800 From: Fam Zheng Message-ID: <20150402143952.GJ15412@fam-t430.nay.redhat.com> References: <1427981862-19783-1-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1427981862-19783-1-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH] virtio-blk: correctly dirty guest memory List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, hangaohuai@huawei.com, zhang.zhanghailiang@huawei.com, lizhijian@cn.fujitsu.com, mst@redhat.com, qemu-devel@nongnu.org, dgilbert@redhat.com, arei.gonglei@huawei.com, stefanha@redhat.com, amit.shah@redhat.com, dgibson@redhat.com, peter.huangpeng@huawei.com On Thu, 04/02 15:37, Paolo Bonzini wrote: > After qemu_iovec_destroy, the QEMUIOVector's size is zeroed and > the zero size ultimately is used to compute virtqueue_push's len > argument. Therefore, reads from virtio-blk devices did not > migrate their results correctly. (Writes were okay). Can't we move qemu_iovec_destroy to virtio_blk_free_request? Fam > > Save the size in submit_requests, and use it when the request is > completed. > > Based on a patch by Wen Congyang. > > Signed-off-by: Wen Congyang > Signed-off-by: Paolo Bonzini > --- > hw/block/dataplane/virtio-blk.c | 2 +- > hw/block/virtio-blk.c | 16 +++++++++++++++- > include/hw/virtio/virtio-blk.h | 1 + > 3 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c > index cd41478..b37ede3 100644 > --- a/hw/block/dataplane/virtio-blk.c > +++ b/hw/block/dataplane/virtio-blk.c > @@ -78,7 +78,7 @@ static void complete_request_vring(VirtIOBlockReq *req, unsigned char status) > stb_p(&req->in->status, status); > > vring_push(s->vdev, &req->dev->dataplane->vring, &req->elem, > - req->qiov.size + sizeof(*req->in)); > + req->read_size + sizeof(*req->in)); > > /* Suppress notification to guest by BH and its scheduled > * flag because requests are completed as a batch after io > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 000c38d..2f00dc4 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->read_size = 0; > req->next = NULL; > req->mr_next = NULL; > return req; > @@ -54,7 +55,7 @@ static void virtio_blk_complete_request(VirtIOBlockReq *req, > trace_virtio_blk_req_complete(req, status); > > stb_p(&req->in->status, status); > - virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in)); > + virtqueue_push(s->vq, &req->elem, req->read_size + sizeof(*req->in)); > virtio_notify(vdev, s->vq); > } > > @@ -102,6 +103,14 @@ static void virtio_blk_rw_complete(void *opaque, int ret) > if (ret) { > int p = virtio_ldl_p(VIRTIO_DEVICE(req->dev), &req->out.type); > bool is_read = !(p & VIRTIO_BLK_T_OUT); > + /* Note that memory may be dirtied on read failure. If the > + * virtio request is not completed here, as is the case for > + * BLOCK_ERROR_ACTION_STOP, the memory may not be copied > + * correctly during live migration. While this is ugly, > + * it is acceptable because the device is free to write to > + * the memory until the request is completed (which will > + * happen on the other side of the migration). > + */ > if (virtio_blk_handle_rw_error(req, -ret, is_read)) { > continue; > } > @@ -348,9 +357,14 @@ static inline void submit_requests(BlockBackend *blk, MultiReqBuffer *mrb, > } > > if (is_write) { > + mrb->reqs[start]->read_size = 0; > blk_aio_writev(blk, sector_num, qiov, nb_sectors, > virtio_blk_rw_complete, mrb->reqs[start]); > } else { > + /* Save old qiov->size, which will be used in > + * virtio_blk_complete_request() > + */ > + mrb->reqs[start]->read_size = qiov->size; > blk_aio_readv(blk, sector_num, qiov, nb_sectors, > virtio_blk_rw_complete, mrb->reqs[start]); > } > diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h > index b3ffcd9..d73ec06 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 read_size; > struct VirtIOBlockReq *next; > struct VirtIOBlockReq *mr_next; > BlockAcctCookie acct; > -- > 2.3.4 > >