From patchwork Thu Jun 20 01:03:46 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Snow X-Patchwork-Id: 1119086 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com 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 ozlabs.org (Postfix) with ESMTPS id 45TklW1p2nz9s5c for ; Thu, 20 Jun 2019 11:30:15 +1000 (AEST) Received: from localhost ([::1]:43036 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hdlu1-0007Hr-B1 for incoming@patchwork.ozlabs.org; Wed, 19 Jun 2019 21:30:13 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:40718) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hdleD-0002Fb-Jw for qemu-devel@nongnu.org; Wed, 19 Jun 2019 21:13:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hdlUm-0004mz-Nk for qemu-devel@nongnu.org; Wed, 19 Jun 2019 21:04:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47306) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hdlUk-0004k4-JS; Wed, 19 Jun 2019 21:04:06 -0400 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D181111549; Thu, 20 Jun 2019 01:04:05 +0000 (UTC) Received: from probe.bos.redhat.com (dhcp-17-164.bos.redhat.com [10.18.17.164]) by smtp.corp.redhat.com (Postfix) with ESMTP id A0C551001E67; Thu, 20 Jun 2019 01:04:04 +0000 (UTC) From: John Snow To: qemu-devel@nongnu.org, qemu-block@nongnu.org Date: Wed, 19 Jun 2019 21:03:46 -0400 Message-Id: <20190620010356.19164-3-jsnow@redhat.com> In-Reply-To: <20190620010356.19164-1-jsnow@redhat.com> References: <20190620010356.19164-1-jsnow@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 20 Jun 2019 01:04:05 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 02/12] block/backup: Add mirror sync mode 'bitmap' 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 , Fam Zheng , vsementsov@virtuozzo.com, Wen Congyang , Xie Changlong , Markus Armbruster , Max Reitz , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" We don't need or want a new sync mode for simple differences in semantics. Create a new mode simply named "BITMAP" that is designed to make use of the new Bitmap Sync Mode field. Because the only bitmap mode is 'conditional', this adds no new functionality to the backup job (yet). The old incremental backup mode is maintained as a syntactic sugar for sync=bitmap, mode=conditional. Add all of the plumbing necessary to support this new instruction. Signed-off-by: John Snow --- qapi/block-core.json | 30 ++++++++++++++++++++++-------- include/block/block_int.h | 6 +++++- block/backup.c | 35 ++++++++++++++++++++++++++++------- block/mirror.c | 6 ++++-- block/replication.c | 2 +- blockdev.c | 8 ++++++-- 6 files changed, 66 insertions(+), 21 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index caf28a71a0..6d05ad8f47 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1127,12 +1127,15 @@ # # @none: only copy data written from now on # -# @incremental: only copy data described by the dirty bitmap. Since: 2.4 +# @incremental: only copy data described by the dirty bitmap. (since: 2.4) +# +# @bitmap: only copy data described by the dirty bitmap. (since: 4.1) +# Behavior on completion is determined by the BitmapSyncMode. # # Since: 1.3 ## { 'enum': 'MirrorSyncMode', - 'data': ['top', 'full', 'none', 'incremental'] } + 'data': ['top', 'full', 'none', 'incremental', 'bitmap'] } ## # @BitmapSyncMode: @@ -1352,10 +1355,14 @@ # # @speed: the maximum speed, in bytes per second # -# @bitmap: the name of dirty bitmap if sync is "incremental". -# Must be present if sync is "incremental", must NOT be present +# @bitmap: the name of dirty bitmap if sync is "bitmap". +# Must be present if sync is "bitmap", must NOT be present # otherwise. (Since 2.4) # +# @bitmap-mode: Specifies the type of data the bitmap should contain after +# the operation concludes. Must be present if sync is "bitmap". +# Must NOT be present otherwise. (Since 4.1) +# # @compress: true to compress data, if the target format supports it. # (default: false) (since 2.8) # @@ -1390,7 +1397,8 @@ 'data': { '*job-id': 'str', 'device': 'str', 'target': 'str', '*format': 'str', 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode', '*speed': 'int', - '*bitmap': 'str', '*compress': 'bool', + '*bitmap': 'str', '*bitmap-mode': 'BitmapSyncMode', + '*compress': 'bool', '*on-source-error': 'BlockdevOnError', '*on-target-error': 'BlockdevOnError', '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } } @@ -1412,10 +1420,14 @@ # @speed: the maximum speed, in bytes per second. The default is 0, # for unlimited. # -# @bitmap: the name of dirty bitmap if sync is "incremental". -# Must be present if sync is "incremental", must NOT be present +# @bitmap: the name of dirty bitmap if sync is "bitmap". +# Must be present if sync is "bitmap", must NOT be present # otherwise. (Since 3.1) # +# @bitmap-mode: Specifies the type of data the bitmap should contain after +# the operation concludes. Must be present if sync is "bitmap". +# Must NOT be present otherwise. (Since 4.1) +# # @compress: true to compress data, if the target format supports it. # (default: false) (since 2.8) # @@ -1449,7 +1461,9 @@ { 'struct': 'BlockdevBackup', 'data': { '*job-id': 'str', 'device': 'str', 'target': 'str', 'sync': 'MirrorSyncMode', '*speed': 'int', - '*bitmap': 'str', '*compress': 'bool', + '*bitmap': 'str', + '*bitmap-mode': 'BitmapSyncMode', + '*compress': 'bool', '*on-source-error': 'BlockdevOnError', '*on-target-error': 'BlockdevOnError', '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } } diff --git a/include/block/block_int.h b/include/block/block_int.h index d6415b53c1..89370c1b9b 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -1132,7 +1132,9 @@ void mirror_start(const char *job_id, BlockDriverState *bs, * @target: Block device to write to. * @speed: The maximum speed, in bytes per second, or 0 for unlimited. * @sync_mode: What parts of the disk image should be copied to the destination. - * @sync_bitmap: The dirty bitmap if sync_mode is MIRROR_SYNC_MODE_INCREMENTAL. + * @sync_bitmap: The dirty bitmap if sync_mode is 'bitmap' or 'incremental' + * @has_bitmap_mode: true if @bitmap_sync carries a meaningful value. + * @bitmap_mode: The bitmap synchronization policy to use. * @on_source_error: The action to take upon error reading from the source. * @on_target_error: The action to take upon error writing to the target. * @creation_flags: Flags that control the behavior of the Job lifetime. @@ -1148,6 +1150,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, BlockDriverState *target, int64_t speed, MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap, + bool has_bitmap_mode, + BitmapSyncMode bitmap_mode, bool compress, BlockdevOnError on_source_error, BlockdevOnError on_target_error, diff --git a/block/backup.c b/block/backup.c index 715e1d3be8..c4f83d4ef7 100644 --- a/block/backup.c +++ b/block/backup.c @@ -38,9 +38,9 @@ typedef struct CowRequest { typedef struct BackupBlockJob { BlockJob common; BlockBackend *target; - /* bitmap for sync=incremental */ BdrvDirtyBitmap *sync_bitmap; MirrorSyncMode sync_mode; + BitmapSyncMode bitmap_mode; BlockdevOnError on_source_error; BlockdevOnError on_target_error; CoRwlock flush_rwlock; @@ -452,7 +452,7 @@ static int coroutine_fn backup_run(Job *job, Error **errp) job_progress_set_remaining(job, s->len); - if (s->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) { + if (s->sync_mode == MIRROR_SYNC_MODE_BITMAP) { backup_incremental_init_copy_bitmap(s); } else { hbitmap_set(s->copy_bitmap, 0, s->len); @@ -536,6 +536,7 @@ static int64_t backup_calculate_cluster_size(BlockDriverState *target, BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, BlockDriverState *target, int64_t speed, MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap, + bool has_bitmap_mode, BitmapSyncMode bitmap_mode, bool compress, BlockdevOnError on_source_error, BlockdevOnError on_target_error, @@ -548,6 +549,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, int ret; int64_t cluster_size; HBitmap *copy_bitmap = NULL; + MirrorSyncMode effective_mode = sync_mode; assert(bs); assert(target); @@ -584,9 +586,28 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, } if (sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) { + if (has_bitmap_mode && + bitmap_mode != BITMAP_SYNC_MODE_CONDITIONAL) { + error_setg(errp, "Bitmap sync mode must be 'conditional' " + "when using sync mode '%s'", + MirrorSyncMode_str(sync_mode)); + return NULL; + } + has_bitmap_mode = true; + bitmap_mode = BITMAP_SYNC_MODE_CONDITIONAL; + effective_mode = MIRROR_SYNC_MODE_BITMAP; + } + + if (effective_mode == MIRROR_SYNC_MODE_BITMAP) { if (!sync_bitmap) { error_setg(errp, "must provide a valid bitmap name for " - "\"incremental\" sync mode"); + "'%s' sync mode", MirrorSyncMode_str(sync_mode)); + return NULL; + } + + if (!has_bitmap_mode) { + error_setg(errp, "must provide a valid bitmap mode for " + "'%s' sync mode", MirrorSyncMode_str(sync_mode)); return NULL; } @@ -596,8 +617,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, } } else if (sync_bitmap) { error_setg(errp, - "a sync_bitmap was provided to backup_run, " - "but received an incompatible sync_mode (%s)", + "a bitmap was given to backup_job_create, " + "but it received an incompatible sync_mode (%s)", MirrorSyncMode_str(sync_mode)); return NULL; } @@ -639,8 +660,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, job->on_source_error = on_source_error; job->on_target_error = on_target_error; job->sync_mode = sync_mode; - job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_INCREMENTAL ? - sync_bitmap : NULL; + job->sync_bitmap = sync_bitmap; + job->bitmap_mode = bitmap_mode; job->compress = compress; /* Detect image-fleecing (and similar) schemes */ diff --git a/block/mirror.c b/block/mirror.c index d17be4cdbc..42b3d9acd0 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1717,8 +1717,10 @@ void mirror_start(const char *job_id, BlockDriverState *bs, bool is_none_mode; BlockDriverState *base; - if (mode == MIRROR_SYNC_MODE_INCREMENTAL) { - error_setg(errp, "Sync mode 'incremental' not supported"); + if ((mode == MIRROR_SYNC_MODE_INCREMENTAL) || + (mode == MIRROR_SYNC_MODE_BITMAP)) { + error_setg(errp, "Sync mode '%s' not supported", + MirrorSyncMode_str(mode)); return; } is_none_mode = mode == MIRROR_SYNC_MODE_NONE; diff --git a/block/replication.c b/block/replication.c index b41bc507c0..a62aaeb879 100644 --- a/block/replication.c +++ b/block/replication.c @@ -543,7 +543,7 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, s->backup_job = backup_job_create( NULL, s->secondary_disk->bs, s->hidden_disk->bs, - 0, MIRROR_SYNC_MODE_NONE, NULL, false, + 0, MIRROR_SYNC_MODE_NONE, NULL, false, 0, false, BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL, backup_job_completed, bs, NULL, &local_err); diff --git a/blockdev.c b/blockdev.c index 5d6a13dea9..7abbd6bbf2 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3572,7 +3572,9 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn, } job = backup_job_create(backup->job_id, bs, target_bs, backup->speed, - backup->sync, bmap, backup->compress, + backup->sync, bmap, + backup->has_bitmap_mode, backup->bitmap_mode, + backup->compress, backup->on_source_error, backup->on_target_error, job_flags, NULL, NULL, txn, &local_err); if (local_err != NULL) { @@ -3677,7 +3679,9 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn, job_flags |= JOB_MANUAL_DISMISS; } job = backup_job_create(backup->job_id, bs, target_bs, backup->speed, - backup->sync, bmap, backup->compress, + backup->sync, bmap, + backup->has_bitmap_mode, backup->bitmap_mode, + backup->compress, backup->on_source_error, backup->on_target_error, job_flags, NULL, NULL, txn, &local_err); if (local_err != NULL) {