From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36909) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eqbiZ-0005r5-BJ for qemu-devel@nongnu.org; Tue, 27 Feb 2018 04:38:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eqbiV-0005WK-78 for qemu-devel@nongnu.org; Tue, 27 Feb 2018 04:38:39 -0500 Date: Tue, 27 Feb 2018 17:38:09 +0800 From: Fam Zheng Message-ID: <20180227093809.GF25412@lemon.usersys.redhat.com> References: <20180122220806.22154-1-mreitz@redhat.com> <20180122220806.22154-16-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180122220806.22154-16-mreitz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 15/16] block/mirror: Add copy mode QAPI interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Kevin Wolf , John Snow , Stefan Hajnoczi On Mon, 01/22 23:08, Max Reitz wrote: > This patch allows the user to specify whether to use active or only > passive mode for mirror block jobs. Currently, this setting will remain I think you want s/passive/background/ in the whole patch. > constant for the duration of the entire block job. > > Signed-off-by: Max Reitz > --- > qapi/block-core.json | 11 +++++++++-- > include/block/block_int.h | 4 +++- > block/mirror.c | 12 +++++++----- > blockdev.c | 9 ++++++++- > 4 files changed, 27 insertions(+), 9 deletions(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index ba1fd736f5..5fafa5fcac 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -1573,6 +1573,9 @@ > # written. Both will result in identical contents. > # Default is true. (Since 2.4) > # > +# @copy-mode: when to copy data to the destination; defaults to 'passive' > +# (Since: 2.12) > +# > # Since: 1.3 > ## > { 'struct': 'DriveMirror', > @@ -1582,7 +1585,7 @@ > '*speed': 'int', '*granularity': 'uint32', > '*buf-size': 'int', '*on-source-error': 'BlockdevOnError', > '*on-target-error': 'BlockdevOnError', > - '*unmap': 'bool' } } > + '*unmap': 'bool', '*copy-mode': 'MirrorCopyMode' } } > > ## > # @BlockDirtyBitmap: > @@ -1761,6 +1764,9 @@ > # above @device. If this option is not given, a node name is > # autogenerated. (Since: 2.9) > # > +# @copy-mode: when to copy data to the destination; defaults to 'passive' > +# (Since: 2.12) > +# > # Returns: nothing on success. > # > # Since: 2.6 > @@ -1781,7 +1787,8 @@ > '*speed': 'int', '*granularity': 'uint32', > '*buf-size': 'int', '*on-source-error': 'BlockdevOnError', > '*on-target-error': 'BlockdevOnError', > - '*filter-node-name': 'str' } } > + '*filter-node-name': 'str', > + '*copy-mode': 'MirrorCopyMode' } } > > ## > # @block_set_io_throttle: > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 03f3fdd129..1fda4d3d43 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -948,6 +948,7 @@ void commit_active_start(const char *job_id, BlockDriverState *bs, > * @filter_node_name: The node name that should be assigned to the filter > * driver that the mirror job inserts into the graph above @bs. NULL means that > * a node name should be autogenerated. > + * @copy_mode: When to trigger writes to the target. > * @errp: Error object. > * > * Start a mirroring operation on @bs. Clusters that are allocated > @@ -961,7 +962,8 @@ void mirror_start(const char *job_id, BlockDriverState *bs, > MirrorSyncMode mode, BlockMirrorBackingMode backing_mode, > BlockdevOnError on_source_error, > BlockdevOnError on_target_error, > - bool unmap, const char *filter_node_name, Error **errp); > + bool unmap, const char *filter_node_name, > + MirrorCopyMode copy_mode, Error **errp); > > /* > * backup_job_create: > diff --git a/block/mirror.c b/block/mirror.c > index 83082adb64..3b23886a5a 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -1409,7 +1409,7 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, > const BlockJobDriver *driver, > bool is_none_mode, BlockDriverState *base, > bool auto_complete, const char *filter_node_name, > - bool is_mirror, > + bool is_mirror, MirrorCopyMode copy_mode, > Error **errp) > { > MirrorBlockJob *s; > @@ -1515,7 +1515,7 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, > s->on_target_error = on_target_error; > s->is_none_mode = is_none_mode; > s->backing_mode = backing_mode; > - s->copy_mode = MIRROR_COPY_MODE_BACKGROUND; > + s->copy_mode = copy_mode; > s->base = base; > s->granularity = granularity; > s->buf_size = ROUND_UP(buf_size, granularity); > @@ -1582,7 +1582,8 @@ void mirror_start(const char *job_id, BlockDriverState *bs, > MirrorSyncMode mode, BlockMirrorBackingMode backing_mode, > BlockdevOnError on_source_error, > BlockdevOnError on_target_error, > - bool unmap, const char *filter_node_name, Error **errp) > + bool unmap, const char *filter_node_name, > + MirrorCopyMode copy_mode, Error **errp) > { > bool is_none_mode; > BlockDriverState *base; > @@ -1597,7 +1598,7 @@ void mirror_start(const char *job_id, BlockDriverState *bs, > speed, granularity, buf_size, backing_mode, > on_source_error, on_target_error, unmap, NULL, NULL, > &mirror_job_driver, is_none_mode, base, false, > - filter_node_name, true, errp); > + filter_node_name, true, copy_mode, errp); > } > > void commit_active_start(const char *job_id, BlockDriverState *bs, > @@ -1620,7 +1621,8 @@ void commit_active_start(const char *job_id, BlockDriverState *bs, > MIRROR_LEAVE_BACKING_CHAIN, > on_error, on_error, true, cb, opaque, > &commit_active_job_driver, false, base, auto_complete, > - filter_node_name, false, &local_err); > + filter_node_name, false, MIRROR_COPY_MODE_BACKGROUND, I think for active commit, MIRROR_COPY_MODE_WRITE_BLOCKING is more appealing? Can be a task for another day, though. Fam