From patchwork Wed Sep 1 15:15:52 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 12469665 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8EE95C432BE for ; Wed, 1 Sep 2021 15:49:25 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 19C926056C for ; Wed, 1 Sep 2021 15:49:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 19C926056C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:48530 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mLSUO-0002Yt-6w for qemu-devel@archiver.kernel.org; Wed, 01 Sep 2021 11:49:24 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49860) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mLRzW-0005Z2-OV for qemu-devel@nongnu.org; Wed, 01 Sep 2021 11:17:32 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:42492) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mLRzU-0003ry-1w for qemu-devel@nongnu.org; Wed, 01 Sep 2021 11:17:30 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1630509447; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XvlOWq+VufylovclDus/sYZnt3plpncNQIyQq6RZ4h4=; b=GeFgcz0dhsqCs9lK33VXTLRRG9lkHefmsME5gt3isN32nOclj+jYGT7/JED0OToJIP/lCe Hb7aKuzmyfrt26+9RptAbiqHEXuLLEb+2HjGXw4TrrldVkxSaXrHMBLDZJDqiYcl5z+u0B WqAqqotzK9XpQNV6i1HqXvvNhKVFB/o= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-287--keQTqelPjuCBM6jvKOfkA-1; Wed, 01 Sep 2021 11:17:25 -0400 X-MC-Unique: -keQTqelPjuCBM6jvKOfkA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C5C241B2C981; Wed, 1 Sep 2021 15:17:24 +0000 (UTC) Received: from localhost (unknown [10.39.193.250]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3D4055D9DC; Wed, 1 Sep 2021 15:17:24 +0000 (UTC) From: Hanna Reitz To: qemu-block@nongnu.org Subject: [PULL 29/56] block/backup: move cluster size calculation to block-copy Date: Wed, 1 Sep 2021 17:15:52 +0200 Message-Id: <20210901151619.689075-30-hreitz@redhat.com> In-Reply-To: <20210901151619.689075-1-hreitz@redhat.com> References: <20210901151619.689075-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=hreitz@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.392, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Peter Maydell , Hanna Reitz , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" From: Vladimir Sementsov-Ogievskiy The main consumer of cluster-size is block-copy. Let's calculate it here instead of passing through backup-top. We are going to publish copy-before-write filter soon, so it will be created through options. But we don't want for now to make explicit option for cluster-size, let's continue to calculate it automatically. So, now is the time to get rid of cluster_size argument for bdrv_cbw_append(). Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz Message-Id: <20210824083856.17408-10-vsementsov@virtuozzo.com> [hreitz: Add qemu/error-report.h include to block/block-copy.c] Signed-off-by: Hanna Reitz --- block/copy-before-write.h | 1 - include/block/block-copy.h | 5 +-- block/backup.c | 62 ++++++-------------------------------- block/block-copy.c | 52 +++++++++++++++++++++++++++++++- block/copy-before-write.c | 10 +++--- 5 files changed, 67 insertions(+), 63 deletions(-) diff --git a/block/copy-before-write.h b/block/copy-before-write.h index 538aab8bdb..b386fd8f01 100644 --- a/block/copy-before-write.h +++ b/block/copy-before-write.h @@ -32,7 +32,6 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source, BlockDriverState *target, const char *filter_node_name, - uint64_t cluster_size, bool compress, BlockCopyState **bcs, Error **errp); diff --git a/include/block/block-copy.h b/include/block/block-copy.h index dca6c4ce36..b8a2d63545 100644 --- a/include/block/block-copy.h +++ b/include/block/block-copy.h @@ -25,8 +25,8 @@ typedef struct BlockCopyState BlockCopyState; typedef struct BlockCopyCallState BlockCopyCallState; BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, - int64_t cluster_size, bool use_copy_range, - bool compress, Error **errp); + bool use_copy_range, bool compress, + Error **errp); /* Function should be called prior any actual copy request */ void block_copy_set_copy_opts(BlockCopyState *s, bool use_copy_range, @@ -91,6 +91,7 @@ void block_copy_kick(BlockCopyCallState *call_state); void block_copy_call_cancel(BlockCopyCallState *call_state); BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s); +int64_t block_copy_cluster_size(BlockCopyState *s); void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip); #endif /* BLOCK_COPY_H */ diff --git a/block/backup.c b/block/backup.c index b31fd99ab3..83516297cb 100644 --- a/block/backup.c +++ b/block/backup.c @@ -29,8 +29,6 @@ #include "block/copy-before-write.h" -#define BACKUP_CLUSTER_SIZE_DEFAULT (1 << 16) - typedef struct BackupBlockJob { BlockJob common; BlockDriverState *cbw; @@ -354,43 +352,6 @@ static const BlockJobDriver backup_job_driver = { .set_speed = backup_set_speed, }; -static int64_t backup_calculate_cluster_size(BlockDriverState *target, - Error **errp) -{ - int ret; - BlockDriverInfo bdi; - bool target_does_cow = bdrv_backing_chain_next(target); - - /* - * If there is no backing file on the target, we cannot rely on COW if our - * backup cluster size is smaller than the target cluster size. Even for - * targets with a backing file, try to avoid COW if possible. - */ - ret = bdrv_get_info(target, &bdi); - if (ret == -ENOTSUP && !target_does_cow) { - /* Cluster size is not defined */ - warn_report("The target block device doesn't provide " - "information about the block size and it doesn't have a " - "backing file. The default block size of %u bytes is " - "used. If the actual block size of the target exceeds " - "this default, the backup may be unusable", - BACKUP_CLUSTER_SIZE_DEFAULT); - return BACKUP_CLUSTER_SIZE_DEFAULT; - } else if (ret < 0 && !target_does_cow) { - error_setg_errno(errp, -ret, - "Couldn't determine the cluster size of the target image, " - "which has no backing file"); - error_append_hint(errp, - "Aborting, since this may create an unusable destination image\n"); - return ret; - } else if (ret < 0 && target_does_cow) { - /* Not fatal; just trudge on ahead. */ - return BACKUP_CLUSTER_SIZE_DEFAULT; - } - - return MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size); -} - BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, BlockDriverState *target, int64_t speed, MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap, @@ -448,11 +409,6 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, return NULL; } - cluster_size = backup_calculate_cluster_size(target, errp); - if (cluster_size < 0) { - goto error; - } - if (perf->max_workers < 1) { error_setg(errp, "max-workers must be greater than zero"); return NULL; @@ -464,13 +420,6 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, return NULL; } - if (perf->max_chunk && perf->max_chunk < cluster_size) { - error_setg(errp, "Required max-chunk (%" PRIi64 ") is less than backup " - "cluster size (%" PRIi64 ")", perf->max_chunk, cluster_size); - return NULL; - } - - if (sync_bitmap) { /* If we need to write to this bitmap, check that we can: */ if (bitmap_mode != BITMAP_SYNC_MODE_NEVER && @@ -503,12 +452,19 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, goto error; } - cbw = bdrv_cbw_append(bs, target, filter_node_name, - cluster_size, false, &bcs, errp); + cbw = bdrv_cbw_append(bs, target, filter_node_name, false, &bcs, errp); if (!cbw) { goto error; } + cluster_size = block_copy_cluster_size(bcs); + + if (perf->max_chunk && perf->max_chunk < cluster_size) { + error_setg(errp, "Required max-chunk (%" PRIi64 ") is less than backup " + "cluster size (%" PRIi64 ")", perf->max_chunk, cluster_size); + goto error; + } + /* job->len is fixed, so we can't allow resize */ job = block_job_create(job_id, &backup_job_driver, txn, cbw, 0, BLK_PERM_ALL, diff --git a/block/block-copy.c b/block/block-copy.c index e83870ff81..5d0076ac93 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -21,12 +21,14 @@ #include "qemu/units.h" #include "qemu/coroutine.h" #include "block/aio_task.h" +#include "qemu/error-report.h" #define BLOCK_COPY_MAX_COPY_RANGE (16 * MiB) #define BLOCK_COPY_MAX_BUFFER (1 * MiB) #define BLOCK_COPY_MAX_MEM (128 * MiB) #define BLOCK_COPY_MAX_WORKERS 64 #define BLOCK_COPY_SLICE_TIME 100000000ULL /* ns */ +#define BLOCK_COPY_CLUSTER_SIZE_DEFAULT (1 << 16) typedef enum { COPY_READ_WRITE_CLUSTER, @@ -342,14 +344,57 @@ void block_copy_set_copy_opts(BlockCopyState *s, bool use_copy_range, } } +static int64_t block_copy_calculate_cluster_size(BlockDriverState *target, + Error **errp) +{ + int ret; + BlockDriverInfo bdi; + bool target_does_cow = bdrv_backing_chain_next(target); + + /* + * If there is no backing file on the target, we cannot rely on COW if our + * backup cluster size is smaller than the target cluster size. Even for + * targets with a backing file, try to avoid COW if possible. + */ + ret = bdrv_get_info(target, &bdi); + if (ret == -ENOTSUP && !target_does_cow) { + /* Cluster size is not defined */ + warn_report("The target block device doesn't provide " + "information about the block size and it doesn't have a " + "backing file. The default block size of %u bytes is " + "used. If the actual block size of the target exceeds " + "this default, the backup may be unusable", + BLOCK_COPY_CLUSTER_SIZE_DEFAULT); + return BLOCK_COPY_CLUSTER_SIZE_DEFAULT; + } else if (ret < 0 && !target_does_cow) { + error_setg_errno(errp, -ret, + "Couldn't determine the cluster size of the target image, " + "which has no backing file"); + error_append_hint(errp, + "Aborting, since this may create an unusable destination image\n"); + return ret; + } else if (ret < 0 && target_does_cow) { + /* Not fatal; just trudge on ahead. */ + return BLOCK_COPY_CLUSTER_SIZE_DEFAULT; + } + + return MAX(BLOCK_COPY_CLUSTER_SIZE_DEFAULT, bdi.cluster_size); +} + BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, - int64_t cluster_size, bool use_copy_range, + bool use_copy_range, bool compress, Error **errp) { BlockCopyState *s; + int64_t cluster_size; BdrvDirtyBitmap *copy_bitmap; bool is_fleecing; + cluster_size = block_copy_calculate_cluster_size(target->bs, errp); + if (cluster_size < 0) { + return NULL; + } + copy_bitmap = bdrv_create_dirty_bitmap(source->bs, cluster_size, NULL, errp); if (!copy_bitmap) { @@ -960,6 +1005,11 @@ BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s) return s->copy_bitmap; } +int64_t block_copy_cluster_size(BlockCopyState *s) +{ + return s->cluster_size; +} + void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip) { qatomic_set(&s->skip_unallocated, skip); diff --git a/block/copy-before-write.c b/block/copy-before-write.c index 235251a640..a7996d54db 100644 --- a/block/copy-before-write.c +++ b/block/copy-before-write.c @@ -37,7 +37,6 @@ typedef struct BDRVCopyBeforeWriteState { BlockCopyState *bcs; BdrvChild *target; - int64_t cluster_size; } BDRVCopyBeforeWriteState; static coroutine_fn int cbw_co_preadv( @@ -52,13 +51,14 @@ static coroutine_fn int cbw_do_copy_before_write(BlockDriverState *bs, { BDRVCopyBeforeWriteState *s = bs->opaque; uint64_t off, end; + int64_t cluster_size = block_copy_cluster_size(s->bcs); if (flags & BDRV_REQ_WRITE_UNCHANGED) { return 0; } - off = QEMU_ALIGN_DOWN(offset, s->cluster_size); - end = QEMU_ALIGN_UP(offset + bytes, s->cluster_size); + off = QEMU_ALIGN_DOWN(offset, cluster_size); + end = QEMU_ALIGN_UP(offset + bytes, cluster_size); return block_copy(s->bcs, off, end - off, true); } @@ -169,7 +169,6 @@ BlockDriver bdrv_cbw_filter = { BlockDriverState *bdrv_cbw_append(BlockDriverState *source, BlockDriverState *target, const char *filter_node_name, - uint64_t cluster_size, bool compress, BlockCopyState **bcs, Error **errp) @@ -214,9 +213,8 @@ BlockDriverState *bdrv_cbw_append(BlockDriverState *source, } appended = true; - state->cluster_size = cluster_size; state->bcs = block_copy_state_new(top->backing, state->target, - cluster_size, false, compress, errp); + false, compress, errp); if (!state->bcs) { error_prepend(errp, "Cannot create block-copy-state: "); goto fail;