[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v6 04/10] qcow2: Implement copy offloading
From: |
Fam Zheng |
Subject: |
Re: [Qemu-block] [PATCH v6 04/10] qcow2: Implement copy offloading |
Date: |
Tue, 29 May 2018 11:34:11 +0800 |
User-agent: |
Mutt/1.9.2 (2017-12-15) |
On Mon, 05/28 11:36, Fam Zheng wrote:
> The two callbacks are implemented quite similarly to the read/write
> functions: bdrv_co_copy_range_from maps for read and calls into bs->file
> or bs->backing depending on the allocation status; bdrv_co_copy_range_to
> maps for write and calls into bs->file.
>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
> block/qcow2.c | 226
> ++++++++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 196 insertions(+), 30 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 6d532470a8..e32a3c1518 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1762,6 +1762,39 @@ static int coroutine_fn
> qcow2_co_block_status(BlockDriverState *bs,
> return status;
> }
>
> +static coroutine_fn int qcow2_handle_l2meta(BlockDriverState *bs,
> + QCowL2Meta **pl2meta,
> + bool link_l2)
> +{
> + int ret = 0;
> + QCowL2Meta *l2meta = *pl2meta;
> +
> + while (l2meta != NULL) {
> + QCowL2Meta *next;
> +
> + if (!ret && link_l2) {
> + ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
> + if (ret) {
> + goto out;
> + }
> + }
> +
> + /* Take the request off the list of running requests */
> + if (l2meta->nb_clusters != 0) {
> + QLIST_REMOVE(l2meta, next_in_flight);
> + }
> +
> + qemu_co_queue_restart_all(&l2meta->dependent_requests);
> +
> + next = l2meta->next;
> + g_free(l2meta);
> + l2meta = next;
> + }
> +out:
> + *pl2meta = l2meta;
> + return ret;
> +}
> +
> static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t
> offset,
> uint64_t bytes, QEMUIOVector *qiov,
> int flags)
> @@ -2048,24 +2081,9 @@ static coroutine_fn int
> qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
> }
> }
>
> - while (l2meta != NULL) {
> - QCowL2Meta *next;
> -
> - ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
> - if (ret < 0) {
> - goto fail;
> - }
> -
> - /* Take the request off the list of running requests */
> - if (l2meta->nb_clusters != 0) {
> - QLIST_REMOVE(l2meta, next_in_flight);
> - }
> -
> - qemu_co_queue_restart_all(&l2meta->dependent_requests);
> -
> - next = l2meta->next;
> - g_free(l2meta);
> - l2meta = next;
> + ret = qcow2_handle_l2meta(bs, &l2meta, true);
> + if (ret) {
> + goto fail;
> }
>
> bytes -= cur_bytes;
> @@ -2076,18 +2094,7 @@ static coroutine_fn int
> qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
> ret = 0;
>
> fail:
> - while (l2meta != NULL) {
> - QCowL2Meta *next;
> -
> - if (l2meta->nb_clusters != 0) {
> - QLIST_REMOVE(l2meta, next_in_flight);
> - }
> - qemu_co_queue_restart_all(&l2meta->dependent_requests);
> -
> - next = l2meta->next;
> - g_free(l2meta);
> - l2meta = next;
> - }
> + qcow2_handle_l2meta(bs, &l2meta, false);
>
> qemu_co_mutex_unlock(&s->lock);
>
> @@ -3274,6 +3281,163 @@ static coroutine_fn int
> qcow2_co_pdiscard(BlockDriverState *bs,
> return ret;
> }
>
> +static int coroutine_fn
> +qcow2_co_copy_range_from(BlockDriverState *bs,
> + BdrvChild *src, uint64_t src_offset,
> + BdrvChild *dst, uint64_t dst_offset,
> + uint64_t bytes, BdrvRequestFlags flags)
> +{
> + BDRVQcow2State *s = bs->opaque;
> + int ret;
> + unsigned int cur_bytes; /* number of bytes in current iteration */
> + BdrvChild *child = NULL;
> +
> + assert(!bs->encrypted);
> + qemu_co_mutex_lock(&s->lock);
> +
> + while (bytes != 0) {
NACK.
flags and dst_offset are calculated wrong in the loop body: bits in the flags
are not reset when starting a new iteration; dst_offset is not incremented.
These bugs are caught during testing the follow up drive-backup patches.
Will send v7 to fix it.
Fam
> + uint64_t copy_offset = 0;
> + /* prepare next request */
> + cur_bytes = MIN(bytes, INT_MAX);
> +
> + ret = qcow2_get_cluster_offset(bs, src_offset, &cur_bytes,
> ©_offset);
> + if (ret < 0) {
> + goto out;
> + }
> +
> + switch (ret) {
> + case QCOW2_CLUSTER_UNALLOCATED:
> + if (bs->backing && bs->backing->bs) {
> + int64_t backing_length = bdrv_getlength(bs->backing->bs);
> + if (src_offset >= backing_length) {
> + flags |= BDRV_REQ_ZERO_WRITE;
> + } else {
> + child = bs->backing;
> + cur_bytes = MIN(cur_bytes, backing_length - src_offset);
> + copy_offset = src_offset;
> + }
> + } else {
> + flags |= BDRV_REQ_ZERO_WRITE;
> + }
> + break;
> +
> + case QCOW2_CLUSTER_ZERO_PLAIN:
> + case QCOW2_CLUSTER_ZERO_ALLOC:
> + flags |= BDRV_REQ_ZERO_WRITE;
> + break;
> +
> + case QCOW2_CLUSTER_COMPRESSED:
> + ret = -ENOTSUP;
> + goto out;
> + break;
> +
> + case QCOW2_CLUSTER_NORMAL:
> + child = bs->file;
> + copy_offset += offset_into_cluster(s, src_offset);
> + if ((copy_offset & 511) != 0) {
> + ret = -EIO;
> + goto out;
> + }
> + break;
> +
> + default:
> + abort();
> + }
> + qemu_co_mutex_unlock(&s->lock);
> + ret = bdrv_co_copy_range_from(child,
> + copy_offset,
> + dst, dst_offset,
> + cur_bytes, flags);
> + qemu_co_mutex_lock(&s->lock);
> + if (ret < 0) {
> + goto out;
> + }
> +
> + bytes -= cur_bytes;
> + src_offset += cur_bytes;
> + }
> + ret = 0;
> +
> +out:
> + qemu_co_mutex_unlock(&s->lock);
> + return ret;
> +}
> +
> +static int coroutine_fn
> +qcow2_co_copy_range_to(BlockDriverState *bs,
> + BdrvChild *src, uint64_t src_offset,
> + BdrvChild *dst, uint64_t dst_offset,
> + uint64_t bytes, BdrvRequestFlags flags)
> +{
> + BDRVQcow2State *s = bs->opaque;
> + int offset_in_cluster;
> + int ret;
> + unsigned int cur_bytes; /* number of sectors in current iteration */
> + uint64_t cluster_offset;
> + uint8_t *cluster_data = NULL;
> + QCowL2Meta *l2meta = NULL;
> +
> + assert(!bs->encrypted);
> + s->cluster_cache_offset = -1; /* disable compressed cache */
> +
> + qemu_co_mutex_lock(&s->lock);
> +
> + while (bytes != 0) {
> +
> + l2meta = NULL;
> +
> + offset_in_cluster = offset_into_cluster(s, dst_offset);
> + cur_bytes = MIN(bytes, INT_MAX);
> +
> + /* TODO:
> + * If src->bs == dst->bs, we could simply copy by incrementing
> + * the refcnt, without copying user data.
> + * Or if src->bs == dst->bs->backing->bs, we could copy by
> discarding. */
> + ret = qcow2_alloc_cluster_offset(bs, dst_offset, &cur_bytes,
> + &cluster_offset, &l2meta);
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + assert((cluster_offset & 511) == 0);
> +
> + ret = qcow2_pre_write_overlap_check(bs, 0,
> + cluster_offset + offset_in_cluster, cur_bytes);
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + qemu_co_mutex_unlock(&s->lock);
> + ret = bdrv_co_copy_range_to(src, src_offset,
> + bs->file,
> + cluster_offset + offset_in_cluster,
> + cur_bytes, flags);
> + qemu_co_mutex_lock(&s->lock);
> + if (ret < 0) {
> + goto fail;
> + }
> +
> + ret = qcow2_handle_l2meta(bs, &l2meta, true);
> + if (ret) {
> + goto fail;
> + }
> +
> + bytes -= cur_bytes;
> + dst_offset += cur_bytes;
> + }
> + ret = 0;
> +
> +fail:
> + qcow2_handle_l2meta(bs, &l2meta, false);
> +
> + qemu_co_mutex_unlock(&s->lock);
> +
> + qemu_vfree(cluster_data);
> + trace_qcow2_writev_done_req(qemu_coroutine_self(), ret);
> +
> + return ret;
> +}
> +
> static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
> PreallocMode prealloc, Error **errp)
> {
> @@ -4522,6 +4686,8 @@ BlockDriver bdrv_qcow2 = {
>
> .bdrv_co_pwrite_zeroes = qcow2_co_pwrite_zeroes,
> .bdrv_co_pdiscard = qcow2_co_pdiscard,
> + .bdrv_co_copy_range_from = qcow2_co_copy_range_from,
> + .bdrv_co_copy_range_to = qcow2_co_copy_range_to,
> .bdrv_truncate = qcow2_truncate,
> .bdrv_co_pwritev_compressed = qcow2_co_pwritev_compressed,
> .bdrv_make_empty = qcow2_make_empty,
> --
> 2.14.3
>
- [Qemu-block] [PATCH v6 00/10] qemu-img convert with copy offloading, Fam Zheng, 2018/05/27
- [Qemu-block] [PATCH v6 01/10] block: Introduce API for copy offloading, Fam Zheng, 2018/05/27
- [Qemu-block] [PATCH v6 02/10] raw: Check byte range uniformly, Fam Zheng, 2018/05/27
- [Qemu-block] [PATCH v6 03/10] raw: Implement copy offloading, Fam Zheng, 2018/05/27
- [Qemu-block] [PATCH v6 04/10] qcow2: Implement copy offloading, Fam Zheng, 2018/05/27
- Re: [Qemu-block] [PATCH v6 04/10] qcow2: Implement copy offloading,
Fam Zheng <=
- [Qemu-block] [PATCH v6 05/10] file-posix: Implement bdrv_co_copy_range, Fam Zheng, 2018/05/27
- [Qemu-block] [PATCH v6 06/10] iscsi: Query and save device designator when opening, Fam Zheng, 2018/05/27
- [Qemu-block] [PATCH v6 07/10] iscsi: Create and use iscsi_co_wait_for_task, Fam Zheng, 2018/05/27
- [Qemu-block] [PATCH v6 08/10] iscsi: Implement copy offloading, Fam Zheng, 2018/05/27
- [Qemu-block] [PATCH v6 09/10] block-backend: Add blk_co_copy_range, Fam Zheng, 2018/05/27
- [Qemu-block] [PATCH v6 10/10] qemu-img: Convert with copy offloading, Fam Zheng, 2018/05/27