[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio-blk: correctly dirty guest memory
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] virtio-blk: correctly dirty guest memory |
Date: |
Thu, 02 Apr 2015 16:51:55 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 |
On 02/04/2015 16:39, Fam Zheng wrote:
> 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?
You would still have to add more code to differentiate reads and
writes---I think.
Paolo
> 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 <address@hidden>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>> 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
>>
>>
- [Qemu-devel] [PATCH] virtio-blk: correctly dirty guest memory, Paolo Bonzini, 2015/04/02
- Re: [Qemu-devel] [PATCH] virtio-blk: correctly dirty guest memory, Fam Zheng, 2015/04/02
- Re: [Qemu-devel] [PATCH] virtio-blk: correctly dirty guest memory,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH] virtio-blk: correctly dirty guest memory, Fam Zheng, 2015/04/02
- Re: [Qemu-devel] [PATCH] virtio-blk: correctly dirty guest memory, Paolo Bonzini, 2015/04/02
- Re: [Qemu-devel] [PATCH] virtio-blk: correctly dirty guest memory, Fam Zheng, 2015/04/02
- Re: [Qemu-devel] [PATCH] virtio-blk: correctly dirty guest memory, Paolo Bonzini, 2015/04/02
- Re: [Qemu-devel] [PATCH] virtio-blk: correctly dirty guest memory, Wen Congyang, 2015/04/02
- Re: [Qemu-devel] [PATCH] virtio-blk: correctly dirty guest memory, Bin Wu, 2015/04/02