From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51097) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uw7iO-00041F-Ss for qemu-devel@nongnu.org; Mon, 08 Jul 2013 05:26:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uw7dp-0000z2-Kf for qemu-devel@nongnu.org; Mon, 08 Jul 2013 05:22:03 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60123) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uw7dp-0000yX-Aj for qemu-devel@nongnu.org; Mon, 08 Jul 2013 05:21:53 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r689LqeX001263 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 8 Jul 2013 05:21:52 -0400 Date: Mon, 8 Jul 2013 17:21:50 +0800 From: Fam Zheng Message-ID: <20130708092150.GA11838@T430s.redhat.com> References: <1372386525-23155-1-git-send-email-imain@redhat.com> <1372386525-23155-2-git-send-email-imain@redhat.com> <51D1730C.6030902@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51D1730C.6030902@redhat.com> Subject: Re: [Qemu-devel] [PATCH V1 1/2] Implement sync modes for drive-backup. Reply-To: famz@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Ian Main , qemu-devel@nongnu.org On Mon, 07/01 14:16, Paolo Bonzini wrote: > Il 28/06/2013 04:28, Ian Main ha scritto: > > diff --git a/blockdev.c b/blockdev.c > > index c5abd65..000dea6 100644 > > --- a/blockdev.c > > +++ b/blockdev.c > > @@ -1430,17 +1430,13 @@ void qmp_drive_backup(const char *device, const char *target, > > Error **errp) > > { > > BlockDriverState *bs; > > - BlockDriverState *target_bs; > > + BlockDriverState *target_bs, *source; > > BlockDriver *drv = NULL; > > Error *local_err = NULL; > > int flags; > > int64_t size; > > int ret; > > > > - if (sync != MIRROR_SYNC_MODE_FULL) { > > - error_setg(errp, "only sync mode 'full' is currently supported"); > > - return; > > - } > > if (!has_speed) { > > speed = 0; > > } > > @@ -1483,6 +1479,13 @@ void qmp_drive_backup(const char *device, const char *target, > > > > flags = bs->open_flags | BDRV_O_RDWR; > > > > + /* See if we have a backing HD we can use to create our new image > > + * on top of. */ > > + source = bs->backing_hd; > > Should the source be "bs" for MIRROR_SYNC_MODE_NONE? Also in this case > you may want to default the format to "qcow2" instead of bs's format. > > Paolo > Maybe not. "source" only affects when sync=top below here. For reading the uncopied for target from source, we can't simply assign it to "bs" for sync=none and create the new image with with bs->filename as backing file, because that way we don't know how to open it with what we have now: the backing file is already opened RW (as "bs"). Fam > > + if (!source && sync == MIRROR_SYNC_MODE_TOP) { > > + sync = MIRROR_SYNC_MODE_FULL; > > + } > > + > > size = bdrv_getlength(bs); > > if (size < 0) { > > error_setg_errno(errp, -size, "bdrv_getlength failed"); > > @@ -1491,8 +1494,14 @@ void qmp_drive_backup(const char *device, const char *target, > > > > if (mode != NEW_IMAGE_MODE_EXISTING) { > > assert(format && drv); > > - bdrv_img_create(target, format, > > - NULL, NULL, NULL, size, flags, &local_err, false); > > + if (sync == MIRROR_SYNC_MODE_TOP) { > > + bdrv_img_create(target, format, source->filename, > > + source->drv->format_name, NULL, > > + size, flags, &local_err, false); > > + } else { > > + bdrv_img_create(target, format, NULL, NULL, NULL, > > + size, flags, &local_err, false); > > + } > > } > > > > if (error_is_set(&local_err)) { > > @@ -1508,7 +1517,7 @@ void qmp_drive_backup(const char *device, const char *target, > > return; > > } > > > > - backup_start(bs, target_bs, speed, on_source_error, on_target_error, > > + backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error, > > block_job_cb, bs, &local_err); > > if (local_err != NULL) { > > bdrv_delete(target_bs); > > diff --git a/include/block/block_int.h b/include/block/block_int.h > > index c6ac871..e45f2a0 100644 > > --- a/include/block/block_int.h > > +++ b/include/block/block_int.h > > @@ -404,6 +404,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, > > * @bs: Block device to operate on. > > * @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. > > * @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. > > * @cb: Completion function for the job. > > @@ -413,7 +414,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, > > * until the job is cancelled or manually completed. > > */ > > void backup_start(BlockDriverState *bs, BlockDriverState *target, > > - int64_t speed, BlockdevOnError on_source_error, > > + int64_t speed, MirrorSyncMode sync_mode, > > + BlockdevOnError on_source_error, > > BlockdevOnError on_target_error, > > BlockDriverCompletionFunc *cb, void *opaque, > > Error **errp); > > diff --git a/qapi-schema.json b/qapi-schema.json > > index cdd2d6b..b3f6c2a 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -1807,6 +1807,10 @@ > > # @format: #optional the format of the new destination, default is to > > # probe if @mode is 'existing', else the format of the source > > # > > +# @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). > > +# > > # @mode: #optional whether and how QEMU should create a new image, default is > > # 'absolute-paths'. > > # > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > index e075df4..f3f6b3d 100644 > > --- a/qmp-commands.hx > > +++ b/qmp-commands.hx > > @@ -957,6 +957,7 @@ Arguments: > > > > Example: > > -> { "execute": "drive-backup", "arguments": { "device": "drive0", > > + "sync": "full", > > "target": "backup.img" } } > > <- { "return": {} } > > EQMP > > > >