From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40899) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7o78-0004ZR-Bl for qemu-devel@nongnu.org; Wed, 24 Jun 2015 13:05:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7o76-0005NM-NB for qemu-devel@nongnu.org; Wed, 24 Jun 2015 13:05:30 -0400 Message-ID: <558AE34D.2000102@redhat.com> Date: Wed, 24 Jun 2015 19:05:17 +0200 From: Max Reitz MIME-Version: 1.0 References: <1433816027-32588-1-git-send-email-famz@redhat.com> <1433816027-32588-6-git-send-email-famz@redhat.com> In-Reply-To: <1433816027-32588-6-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 5/6] qmp: Add blockdev-mirror command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , qemu-block@nongnu.org, Jeff Cody , Markus Armbruster , Stefan Hajnoczi , pbonzini@redhat.com On 09.06.2015 04:13, Fam Zheng wrote: > This will start a mirror job from a named device to another named > device, its relation with drive-mirror is similar with blockdev-backup > to drive-backup. > > In blockdev-mirror, the target node should be prepared by blockdev-add, > which will be responsible for assigning a name to the new node, so > 'node-name' in drive-mirror is dropped. > > Signed-off-by: Fam Zheng > --- > blockdev.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > qapi/block-core.json | 47 ++++++++++++++++++++++++++++++++++++++++ > qmp-commands.hx | 48 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 155 insertions(+) > > diff --git a/blockdev.c b/blockdev.c > index 44030da..6726b8d 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2986,6 +2986,9 @@ void qmp_drive_mirror(const char *device, const char *target, > } > bs = blk_bs(blk); > > + if (!has_mode) { > + mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS; > + } I think this hunk should be in patch 3, not here. Other than that, looks good. (apart from a pedantic comment below) Max > if (!has_format) { > format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name; > } > @@ -3104,6 +3107,63 @@ out: > aio_context_release(aio_context); > } > > +void qmp_blockdev_mirror(const char *device, const char *target, > + bool has_replaces, const char *replaces, > + MirrorSyncMode sync, > + bool has_speed, int64_t speed, > + bool has_granularity, uint32_t granularity, > + bool has_buf_size, int64_t buf_size, > + bool has_on_source_error, > + BlockdevOnError on_source_error, > + bool has_on_target_error, > + BlockdevOnError on_target_error, > + Error **errp) > +{ > + BlockDriverState *bs; > + BlockBackend *blk; > + BlockDriverState *target_bs; > + AioContext *aio_context; > + Error *local_err = NULL; > + > + blk = blk_by_name(device); > + if (!blk) { > + error_setg(errp, "Device '%s' not found", device); > + return; > + } > + bs = blk_bs(blk); > + > + if (!bs) { > + error_setg(errp, "Device '%s' has no media", device); > + return; > + } > + > + target_bs = bdrv_lookup_bs(target, target, errp); > + if (!target_bs) { > + return; > + } > + > + aio_context = bdrv_get_aio_context(bs); > + aio_context_acquire(aio_context); > + > + bdrv_ref(target_bs); > + bdrv_set_aio_context(target_bs, aio_context); > + > + blockdev_mirror_common(bs, target_bs, > + has_replaces, replaces, sync, > + has_speed, speed, > + has_granularity, granularity, > + has_buf_size, buf_size, > + has_on_source_error, on_source_error, > + has_on_target_error, on_target_error, > + &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + bdrv_unref(target_bs); > + } > + > + aio_context_release(aio_context); > +} > + > /* Get the block job for a given device name and acquire its AioContext */ > static BlockJob *find_block_job(const char *device, AioContext **aio_context, > Error **errp) > diff --git a/qapi/block-core.json b/qapi/block-core.json > index b5c4559..bd163f7 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -1059,6 +1059,53 @@ > 'data': 'BlockDirtyBitmap' } > > ## > +# @blockdev-mirror > +# > +# Start mirroring a block device's writes to a new destination. > +# > +# @device: the name of the device whose writes should be mirrored. Superfluous space here. () > +# > +# @target: the name of the device to mirror to. > +# > +# @replaces: #optional with sync=full graph node name to be replaced by the new > +# image when a whole image copy is done. This can be used to repair > +# broken Quorum files. > +# > +# @speed: #optional the maximum speed, in bytes per second > +# > +# @sync: what parts of the disk image should be copied to the destination > +# (all the disk, only the sectors allocated in the topmost image, or > +# only new I/O). > +# > +# @granularity: #optional granularity of the dirty bitmap, default is 64K > +# if the image format doesn't have clusters, 4K if the clusters > +# are smaller than that, else the cluster size. Must be a > +# power of 2 between 512 and 64M > +# > +# @buf-size: #optional maximum amount of data in flight from source to > +# target > +# > +# @on-source-error: #optional the action to take on an error on the source, > +# default 'report'. 'stop' and 'enospc' can only be used > +# if the block device supports io-status (see BlockInfo). > +# > +# @on-target-error: #optional the action to take on an error on the target, > +# default 'report' (no limitations, since this applies to > +# a different block device than @device). > +# > +# Returns: nothing on success; error message on failure. > +# > +# Since 2.4 > +## > +{ 'command': 'blockdev-mirror', > + 'data': { 'device': 'str', 'target': 'str', > + '*replaces': 'str', > + 'sync': 'MirrorSyncMode', > + '*speed': 'int', '*granularity': 'uint32', > + '*buf-size': 'int', '*on-source-error': 'BlockdevOnError', > + '*on-target-error': 'BlockdevOnError' } } > + > +## > # @block_set_io_throttle: > # > # Change I/O throttle limits for a block drive. > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 90e0135..646db78 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -1560,6 +1560,54 @@ Example: > EQMP > > { > + .name = "blockdev-mirror", > + .args_type = "sync:s,device:B,target:B,replaces:s?,speed:i?," > + "on-source-error:s?,on-target-error:s?," > + "granularity:i?,buf-size:i?", > + .mhandler.cmd_new = qmp_marshal_input_blockdev_mirror, > + }, > + > +SQMP > +blockdev-mirror > +------------ > + > +Start mirroring a block device's writes to another block device. target > +specifies the target of mirror operation. > + > +Arguments: > + > +- "device": device name to operate on (json-string) > +- "target": device name to mirror to (json-string) > +- "replaces": the block driver node name to replace when finished > + (json-string, optional) > +- "speed": maximum speed of the streaming job, in bytes per second > + (json-int) > +- "granularity": granularity of the dirty bitmap, in bytes (json-int, optional) > +- "buf_size": maximum amount of data in flight from source to target, in bytes > + (json-int, default 10M) > +- "sync": what parts of the disk image should be copied to the destination; > + possibilities include "full" for all the disk, "top" for only the sectors > + allocated in the topmost image, or "none" to only replicate new I/O > + (MirrorSyncMode). > +- "on-source-error": the action to take on an error on the source > + (BlockdevOnError, default 'report') > +- "on-target-error": the action to take on an error on the target > + (BlockdevOnError, default 'report') > + > +The default value of the granularity is the image cluster size clamped > +between 4096 and 65536, if the image format defines one. If the format > +does not define a cluster size, the default value of the granularity > +is 65536. > + > +Example: > + > +-> { "execute": "blockdev-mirror", "arguments": { "device": "ide-hd0", > + "target": "target0", > + "sync": "full" } } > +<- { "return": {} } > + > +EQMP > + { > .name = "change-backing-file", > .args_type = "device:s,image-node-name:s,backing-file:s", > .mhandler.cmd_new = qmp_marshal_input_change_backing_file,