From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46354) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7wyx-0003H0-FD for qemu-devel@nongnu.org; Wed, 24 Jun 2015 22:33:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7wyw-00085Y-5n for qemu-devel@nongnu.org; Wed, 24 Jun 2015 22:33:39 -0400 Date: Thu, 25 Jun 2015 10:33:28 +0800 From: Fam Zheng Message-ID: <20150625023328.GF17695@ad.nay.redhat.com> References: <1433816027-32588-1-git-send-email-famz@redhat.com> <1433816027-32588-4-git-send-email-famz@redhat.com> <558ADC32.9060205@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <558ADC32.9060205@redhat.com> Subject: Re: [Qemu-devel] [PATCH 3/6] block: Extract blockdev part of qmp_drive_mirror List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , qemu-block@nongnu.org, Jeff Cody , qemu-devel@nongnu.org, Markus Armbruster , Stefan Hajnoczi , pbonzini@redhat.com On Wed, 06/24 18:34, Max Reitz wrote: > On 09.06.2015 04:13, Fam Zheng wrote: > >This is the part that will be reused by blockdev-mirror. > > > >Signed-off-by: Fam Zheng > >--- > > blockdev.c | 158 +++++++++++++++++++++++++++++++++++-------------------------- > > 1 file changed, 92 insertions(+), 66 deletions(-) > > > >diff --git a/blockdev.c b/blockdev.c > >index b573e56..c32a9a9 100644 > >--- a/blockdev.c > >+++ b/blockdev.c > >@@ -2883,29 +2883,22 @@ out: > > #define DEFAULT_MIRROR_BUF_SIZE (10 << 20) > >-void qmp_drive_mirror(const char *device, const char *target, > >- bool has_format, const char *format, > >- bool has_node_name, const char *node_name, > >- bool has_replaces, const char *replaces, > >- enum MirrorSyncMode sync, > >- bool has_mode, enum NewImageMode mode, > >- 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) > >+/* Parameter check and block job starting for mirroring. > >+ * Caller should hold @device and @target's aio context (They must to be on the > >+ * same aio context). */ > >+static void blockdev_mirror_common(BlockDriverState *bs, > >+ BlockDriverState *target, > >+ bool has_replaces, const char *replaces, > >+ enum 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) > > { > >- BlockBackend *blk; > >- BlockDriverState *bs; > >- BlockDriverState *source, *target_bs; > >- AioContext *aio_context; > >- BlockDriver *drv = NULL; > >- Error *local_err = NULL; > >- QDict *options = NULL; > >- int flags; > >- int64_t size; > >- int ret; > > if (!has_speed) { > > speed = 0; > >@@ -2916,9 +2909,6 @@ void qmp_drive_mirror(const char *device, const char *target, > > if (!has_on_target_error) { > > on_target_error = BLOCKDEV_ON_ERROR_REPORT; > > } > >- if (!has_mode) { > >- mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS; > >- } > > if (!has_granularity) { > > granularity = 0; > > } > >@@ -2936,35 +2926,73 @@ void qmp_drive_mirror(const char *device, const char *target, > > return; > > } > >- blk = blk_by_name(device); > >- if (!blk) { > >- error_set(errp, QERR_DEVICE_NOT_FOUND, device); > >- return; > >- } > >- > >- aio_context = blk_get_aio_context(blk); > >- aio_context_acquire(aio_context); > >- > >- if (!blk_is_available(blk)) { > >- error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); > >- goto out; > >- } > >- bs = blk_bs(blk); > >- > >- if (!has_format) { > >- format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name; > >- } > >- if (format) { > >- drv = bdrv_find_format(format); > >- if (!drv) { > >- error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); > >- goto out; > >- } > >- } > >- > > if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) { > >+ return; > >+ } > >+ > >+ if (!bs->backing_hd && sync == MIRROR_SYNC_MODE_TOP) { > >+ sync = MIRROR_SYNC_MODE_FULL; > >+ } > >+ > >+ /* pass the node name to replace to mirror start since it's loose coupling > >+ * and will allow to check whether the node still exist at mirror completion > >+ */ > >+ mirror_start(bs, target, > >+ has_replaces ? replaces : NULL, > >+ speed, granularity, buf_size, sync, > >+ on_source_error, on_target_error, > >+ block_job_cb, bs, errp); > >+} > >+ > >+void qmp_drive_mirror(const char *device, const char *target, > >+ bool has_format, const char *format, > >+ bool has_node_name, const char *node_name, > >+ bool has_replaces, const char *replaces, > >+ enum MirrorSyncMode sync, > >+ bool has_mode, enum NewImageMode mode, > >+ 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 *source, *target_bs; > >+ AioContext *aio_context; > >+ BlockDriver *drv = NULL; > >+ Error *local_err = NULL; > >+ QDict *options = NULL; > >+ int flags; > >+ int64_t size; > >+ int ret; > >+ > >+ blk = blk_by_name(device); > >+ if (!blk) { > >+ error_set(errp, QERR_DEVICE_NOT_FOUND, device); > >+ return; > >+ } > >+ > >+ aio_context = blk_get_aio_context(blk); > >+ aio_context_acquire(aio_context); > >+ > >+ if (!blk_is_available(blk)) { > >+ error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); > > goto out; > > } > >+ bs = blk_bs(blk); > >+ > >+ if (!has_format) { > >+ format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name; > >+ } > >+ if (format) { > >+ drv = bdrv_find_format(format); > >+ if (!drv) { > >+ error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); > >+ goto out; > >+ } > >+ } > > flags = bs->open_flags | BDRV_O_RDWR; > > source = bs->backing_hd; > >@@ -2989,14 +3017,14 @@ void qmp_drive_mirror(const char *device, const char *target, > > if (!has_node_name) { > > error_setg(errp, "a node-name must be provided when replacing a" > > " named node of the graph"); > >- goto out; > >+ return; > > } > > to_replace_bs = check_to_replace_node(replaces, &local_err); > > if (!to_replace_bs) { > > error_propagate(errp, local_err); > >- goto out; > >+ return; > > } > > replace_aio_context = bdrv_get_aio_context(to_replace_bs); > >@@ -3007,7 +3035,7 @@ void qmp_drive_mirror(const char *device, const char *target, > > if (size != replace_size) { > > error_setg(errp, "cannot replace image with a mirror image of " > > "different size"); > >- goto out; > >+ return; > > } > > } > > There are still a couple of "return;" statements left in the "if > (has_replaces)" block. I think they should be replaced, too. You're right! Will fix. Fam > > Max