[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2 v2] Add the blockdev-reopen and blockdev-mig
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 2/2 v2] Add the blockdev-reopen and blockdev-migrate commands |
Date: |
Fri, 24 Feb 2012 17:01:43 -0200 |
On Fri, 24 Feb 2012 16:49:04 +0000
Federico Simoncelli <address@hidden> wrote:
> Signed-off-by: Federico Simoncelli <address@hidden>
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
> address@hidden drive_reopen
> address@hidden 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
> address@hidden blkdev_migrate
> address@hidden 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.
- [Qemu-devel] [PATCH 1/2 v2] Add blkmirror block driver, (continued)
- Re: [Qemu-devel] [PATCH 2/2 v2] Add the blockdev-reopen and blockdev-migrate commands, Paolo Bonzini, 2012/02/24
- Re: [Qemu-devel] [PATCH 2/2 v2] Add the blockdev-reopen and blockdev-migrate commands, Luiz Capitulino, 2012/02/24
- Re: [Qemu-devel] [PATCH 2/2 v2] Add the blockdev-reopen and blockdev-migrate commands, Paolo Bonzini, 2012/02/24
- Re: [Qemu-devel] [PATCH 2/2 v2] Add the blockdev-reopen and blockdev-migrate commands, Eric Blake, 2012/02/24
- Re: [Qemu-devel] [PATCH 2/2 v2] Add the blockdev-reopen and blockdev-migrate commands, Paolo Bonzini, 2012/02/25
- Re: [Qemu-devel] [PATCH 2/2 v2] Add the blockdev-reopen and blockdev-migrate commands, Federico Simoncelli, 2012/02/27
- Re: [Qemu-devel] [PATCH 2/2 v2] Add the blockdev-reopen and blockdev-migrate commands, Luiz Capitulino, 2012/02/27