[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V1 1/2] Implement sync modes for drive-backup.
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH V1 1/2] Implement sync modes for drive-backup. |
Date: |
Mon, 8 Jul 2013 17:21:50 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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
> >
>
>