On Fri, 24 Feb 2012 16:49:04 +0000 Federico Simoncelli <fsimo...@redhat.com> wrote:
> Signed-off-by: Federico Simoncelli <fsimo...@redhat.com> How does this relate to migrate -b? Should it be deprecated? Btw, would be nice to have a 0/2 intro email describing the feature and changelog info. > --- > blockdev.c | 107 > ++++++++++++++++++++++++++++++++++++++++++++++++------ > hmp-commands.hx | 38 +++++++++++++++++++ > hmp.c | 24 ++++++++++++ > hmp.h | 2 + > qapi-schema.json | 54 +++++++++++++++++++++++++++ > 5 files changed, 213 insertions(+), 12 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index 2c132a3..f51bd6f 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -646,9 +646,8 @@ void do_commit(Monitor *mon, const QDict *qdict) > } > } > > -void qmp_blockdev_snapshot_sync(const char *device, const char > *snapshot_file, > - bool has_format, const char *format, > - Error **errp) > +static void change_blockdev_image(const char *device, const char > *new_image_file, > + const char *format, bool create, Error > **errp) > { > BlockDriverState *bs; > BlockDriver *drv, *old_drv, *proto_drv; > @@ -671,7 +670,7 @@ void qmp_blockdev_snapshot_sync(const char *device, const > char *snapshot_file, > old_drv = bs->drv; > flags = bs->open_flags; > > - if (!has_format) { > + if (!format) { > format = "qcow2"; > } > > @@ -681,24 +680,26 @@ void qmp_blockdev_snapshot_sync(const char *device, > const char *snapshot_file, > return; > } > > - proto_drv = bdrv_find_protocol(snapshot_file); > + proto_drv = bdrv_find_protocol(new_image_file); > if (!proto_drv) { > error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); > return; > } > > - ret = bdrv_img_create(snapshot_file, format, bs->filename, > - bs->drv->format_name, NULL, -1, flags); > - if (ret) { > - error_set(errp, QERR_UNDEFINED_ERROR); > - return; > + if (create) { > + ret = bdrv_img_create(new_image_file, format, bs->filename, > + bs->drv->format_name, NULL, -1, flags); > + if (ret) { > + error_set(errp, QERR_UNDEFINED_ERROR); > + return; > + } > } > > bdrv_drain_all(); > bdrv_flush(bs); > > bdrv_close(bs); > - ret = bdrv_open(bs, snapshot_file, flags, drv); > + ret = bdrv_open(bs, new_image_file, flags, drv); > /* > * If reopening the image file we just created fails, fall back > * and try to re-open the original image. If that fails too, we > @@ -709,11 +710,93 @@ void qmp_blockdev_snapshot_sync(const char *device, > const char *snapshot_file, > if (ret != 0) { > error_set(errp, QERR_OPEN_FILE_FAILED, old_filename); > } else { > - error_set(errp, QERR_OPEN_FILE_FAILED, snapshot_file); > + error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file); > } > } > } > > +void qmp_drive_reopen(const char *device, const char *new_image_file, > + bool has_format, const char *format, Error **errp) > +{ > + change_blockdev_image(device, new_image_file, > + has_format ? format : NULL, false, errp); > +} > + > +void qmp_blockdev_snapshot_sync(const char *device, const char > *snapshot_file, > + bool has_format, const char *format, > + Error **errp) > +{ > + change_blockdev_image(device, snapshot_file, > + has_format ? format : NULL, true, errp); > +} > + > +static void qmp_blockdev_mirror(const char *device, const char *destination, > + const char *new_image_file, Error **errp) > +{ Minor: this is a not a qmp function, please drop the qmp_ prefix then. > + BlockDriver *drv; > + int i, j, escape; > + char new_filename[2048], *filename; I'd use PATH_MAX for new_filename's size. > + > + drv = bdrv_find_format("blkmirror"); > + if (!drv) { > + error_set(errp, QERR_INVALID_BLOCK_FORMAT, "blkmirror"); > + return; > + } > + > + escape = 0; > + for (i = 0, j = 0; j < strlen(new_image_file); j++) { > + loop: > + if (!(i < sizeof(new_filename) - 2)) { > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, > + "new-image-file", "shorter filename"); > + return; > + } > + > + if (new_image_file[j] == ':' || new_image_file[j] == '\\') { > + if (!escape) { > + new_filename[i++] = '\\', escape = 1; > + goto loop; > + } else { > + escape = 0; > + } > + } > + > + new_filename[i++] = new_image_file[j]; > + } IMO, you could require the filename arguments to be escaped already. > + > + filename = g_strdup_printf("blkmirror:%s:%s", new_filename, destination); > + change_blockdev_image(device, filename, "blkmirror", false, errp); > + g_free(filename); > +} > + > +void qmp_blockdev_migrate(const char *device, bool incremental, > + const char *destination, bool has_new_image_file, > + const char *new_image_file, Error **errp) > +{ > + BlockDriverState *bs; > + > + bs = bdrv_find(device); > + if (!bs) { > + error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + return; > + } > + if (bdrv_in_use(bs)) { > + error_set(errp, QERR_DEVICE_IN_USE, device); > + return; > + } > + > + if (incremental) { > + if (!has_new_image_file) { > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, > + "incremental", "a new image file"); > + } else { > + qmp_blockdev_mirror(device, destination, new_image_file, errp); > + } > + } else { > + error_set(errp, QERR_NOT_SUPPORTED); > + } The command documentation says that 'incremental' and 'new_image_file' are optionals, but the code makes them required. Why? > +} > + > static void eject_device(BlockDriverState *bs, int force, Error **errp) > { > if (bdrv_in_use(bs)) { > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 573b823..4aacf2a 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -886,6 +886,44 @@ Snapshot device, using snapshot file as target if > provided > ETEXI > > { > + .name = "drive_reopen", > + .args_type = "device:B,new-image-file:s?,format:s?", > + .params = "device [new-image-file] [format]", > + .help = "Assigns a new image file to a device.\n\t\t\t" > + "The image will be opened using the format\n\t\t\t" > + "specified or 'qcow2' by default.\n\t\t\t" > + "This command is deprecated.", > + .mhandler.cmd = hmp_drive_reopen, > + }, > + > +STEXI > +@item drive_reopen > +@findex drive_reopen > +Assigns a new image file to a device. > +ETEXI > + > + { > + .name = "blkdev_migrate", > + .args_type = > "incremental:-i,device:B,destination:s,new-image-file:s?", > + .params = "device mode destination [new-image-file]", > + .help = "Migrates the device to a new destination.\n\t\t\t" > + "If the incremental (-i) option is used then\n\t\t\t" > + "only the new writes are sent to the > destination.\n\t\t\t" > + "This method is particularly useful if used in\n\t\t\t" > + "conjunction with new-image-file (a new image > for\n\t\t\t" > + "the device) allowing the current image to be\n\t\t\t" > + "transferred to the destination by an external\n\t\t\t" > + "manager.", > + .mhandler.cmd = hmp_blkdev_migrate, > + }, > + > +STEXI > +@item blkdev_migrate > +@findex blkdev_migrate > +Migrates the device to a new destination. > +ETEXI > + > + { > .name = "drive_add", > .args_type = "pci_addr:s,opts:s", > .params = "[[<domain>:]<bus>:]<slot>\n" > diff --git a/hmp.c b/hmp.c > index 8ff8c94..282d3e5 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -701,6 +701,30 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict > *qdict) > hmp_handle_error(mon, &errp); > } > > +void hmp_drive_reopen(Monitor *mon, const QDict *qdict) > +{ > + const char *device = qdict_get_str(qdict, "device"); > + const char *filename = qdict_get_str(qdict, "new-image-file"); > + const char *format = qdict_get_try_str(qdict, "format"); > + Error *errp = NULL; > + > + qmp_drive_reopen(device, filename, !!format, format, &errp); > + hmp_handle_error(mon, &errp); > +} > + > +void hmp_blkdev_migrate(Monitor *mon, const QDict *qdict) > +{ > + bool incremental = qdict_get_try_bool(qdict, "incremental", 0); > + const char *device = qdict_get_str(qdict, "device"); > + const char *destination = qdict_get_str(qdict, "destination"); > + const char *new_image_file = qdict_get_try_str(qdict, "new-image-file"); > + Error *errp = NULL; > + > + qmp_blockdev_migrate(device, incremental, destination, > + !!new_image_file, new_image_file, &errp); > + hmp_handle_error(mon, &errp); > +} > + > void hmp_migrate_cancel(Monitor *mon, const QDict *qdict) > { > qmp_migrate_cancel(NULL); > diff --git a/hmp.h b/hmp.h > index 18eecbd..3c66257 100644 > --- a/hmp.h > +++ b/hmp.h > @@ -47,6 +47,8 @@ void hmp_block_passwd(Monitor *mon, const QDict *qdict); > void hmp_balloon(Monitor *mon, const QDict *qdict); > void hmp_block_resize(Monitor *mon, const QDict *qdict); > void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict); > +void hmp_drive_reopen(Monitor *mon, const QDict *qdict); > +void hmp_blkdev_migrate(Monitor *mon, const QDict *qdict); > void hmp_migrate_cancel(Monitor *mon, const QDict *qdict); > void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict); > void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict); > diff --git a/qapi-schema.json b/qapi-schema.json > index d02ee86..df1248c 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -1136,6 +1136,60 @@ > 'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str' } } > > ## > +# @drive-reopen > +# > +# Assigns a new image file to a device. > +# > +# @device: the name of the device for which we are chainging the image file. > +# > +# @new-image-file: the target of the new image. If the file doesn't exists > the > +# command will fail. > +# > +# @format: #optional the format of the new image, default is 'qcow2'. > +# > +# Returns: nothing on success > +# If @device is not a valid block device, DeviceNotFound > +# If @new-image-file can't be opened, OpenFileFailed > +# If @format is invalid, InvalidBlockFormat > +# > +# Notes: This command is deprecated. As I said in the other thread, we need a replacement to deprecate something. > +# > +# Since 1.1 > +## > + > +{ 'command': 'drive-reopen', > + 'data': { 'device': 'str', 'new-image-file': 'str', '*format': 'str' } } > + > +## > +# @blockdev-migrate > +# > +# Migrates the device to a new destination. > +# > +# @device: the name of the block device to migrate. > +# > +# @incremental: if true only the new writes are sent to the destination. > +# This method is particularly useful if used in conjunction > +# with new-image-file allowing the current image to be > +# transferred to the destination by an external manager. > +# > +# @destination: the destination of the block migration. > +# > +# @new-image-file: #optional an existing image file that will replace > +# the current one in the device. > +# > +# Returns: nothing on success > +# If @device is not a valid block device, DeviceNotFound > +# If @mode is not a valid migration mode, InvalidParameterValue > +# If @destination can't be opened, OpenFileFailed > +# If @new-image-file can't be opened, OpenFileFailed > +# > +# Since 1.1 > +## > +{ 'command': 'blockdev-migrate', > + 'data': { 'device': 'str', 'incremental' : 'bool', > + 'destination': 'str', '*new-image-file': 'str' } } > + > +## > # @human-monitor-command: > # > # Execute a command on the human monitor and return the output.