Re: [Qemu-devel] [PATCH v3] QMP: add snapshot_blkdev command
On 07/11/11 22:35, Luiz Capitulino wrote: Sorry that is no go, you just broke the hmp implementation - you cannot change the hmp behavior like that. HMP uses positional arguments, so changing argument names makes no difference. And, apart from some exceptions, it's not an stable interface, anyway... I guess you're right about the naming not affecting the hmp interface. However hmp is far more usable to end users than qmp, so yes it does matter not to change the interface at random. Jes
Re: [Qemu-devel] [PATCH v3] QMP: add snapshot_blkdev command
On Tue, 12 Jul 2011 11:26:09 +0200 Jes Sorensen jes.soren...@redhat.com wrote: On 07/11/11 22:35, Luiz Capitulino wrote: Sorry that is no go, you just broke the hmp implementation - you cannot change the hmp behavior like that. HMP uses positional arguments, so changing argument names makes no difference. And, apart from some exceptions, it's not an stable interface, anyway... I guess you're right about the naming not affecting the hmp interface. However hmp is far more usable to end users than qmp, so yes it does matter not to change the interface at random. Right, but we don't do it at random. Actually, it's not something that happens often and we always consider the impact. However, hmp doesn't have stability guarantees as qmp has. In this specific case, no hmp user visible change has been made.
[Qemu-devel] [PATCH v3] QMP: add snapshot_blkdev command
From: Jes Sorensen jes.soren...@redhat.com Add QMP bits for snapshot_blkdev command. This is the same as snapshot_blkdev in the human monitor. The command is synchronous. In the future async commands and or a break down of the functionality into multiple commands might be added. Signed-off-by: Jes Sorensen jes.soren...@redhat.com --- qmp-commands.hx | 35 +++ 1 files changed, 35 insertions(+), 0 deletions(-) diff --git a/qmp-commands.hx b/qmp-commands.hx index 92c5c3a..2b9f6af 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -694,6 +694,41 @@ Example: EQMP { +.name = blockdev-snapshot-sync, +.args_type = device:B,snapshot-file:s?,format:s?, +.params = device [new-image-file] [format], +.user_print = monitor_user_noop, +.mhandler.cmd_new = do_snapshot_blkdev, +}, + +SQMP +blockdev-snapshot-sync +-- + +Synchronous snapshot of a block device. snapshot_file specifies the +target of the new image. If the file exists, or if it is a device, the +snapshot will be created in the existing file/device. If does not +exist, a new file will be created. format specifies the format of the +snapshot image, default is qcow2. + +Arguments: + +- device: device name to snapshot (json-string) +- snapshot_file: name of new image file (json-string) +- format: format of new image (json-string) + +Example: + +- { execute: blockdev-snapshot, arguments: { device: ide-hd0, +snapshot-file: +/some/place/my-image, + format: qcow2 + } } +- { return: {} } + +EQMP + +{ .name = balloon, .args_type = value:M, .params = target, -- 1.7.4.4
Re: [Qemu-devel] [PATCH v3] QMP: add snapshot_blkdev command
On Mon, 11 Jul 2011 20:01:09 +0200 jes.soren...@redhat.com wrote: From: Jes Sorensen jes.soren...@redhat.com Add QMP bits for snapshot_blkdev command. This is the same as snapshot_blkdev in the human monitor. The command is synchronous. In the future async commands and or a break down of the functionality into multiple commands might be added. Signed-off-by: Jes Sorensen jes.soren...@redhat.com --- qmp-commands.hx | 35 +++ 1 files changed, 35 insertions(+), 0 deletions(-) diff --git a/qmp-commands.hx b/qmp-commands.hx index 92c5c3a..2b9f6af 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -694,6 +694,41 @@ Example: EQMP { +.name = blockdev-snapshot-sync, +.args_type = device:B,snapshot-file:s?,format:s?, You changed from an underline to a hyphen as I asked, but the implementation still expects an underline. I fixed that myself to avoid multiple submissions because of a small thing (also fixed the command name in the subject). The patch I merged follows for reference. Please, test your patches before submitting next time. Subject: [PATCH] QMP: add snapshot-blkdev-sync command Add QMP bits for snapshot_blkdev command. This is the same as snapshot_blkdev in the human monitor. The command is synchronous. In the future async commands and or a break down of the functionality into multiple commands might be added. Also change the 'snapshot_file' argument to 'snapshot-file' in the human monitor, so that it matches QMP. Signed-off-by: Jes Sorensen jes.soren...@redhat.com Signed-off-by: Luiz Capitulino lcapitul...@gmail.com --- blockdev.c |4 ++-- hmp-commands.hx |2 +- qmp-commands.hx | 34 ++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/blockdev.c b/blockdev.c index 7d579d6..1a96d3c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -572,7 +572,7 @@ void do_commit(Monitor *mon, const QDict *qdict) int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data) { const char *device = qdict_get_str(qdict, device); -const char *filename = qdict_get_try_str(qdict, snapshot_file); +const char *filename = qdict_get_try_str(qdict, snapshot-file); const char *format = qdict_get_try_str(qdict, format); BlockDriverState *bs; BlockDriver *drv, *old_drv, *proto_drv; @@ -581,7 +581,7 @@ int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data) char old_filename[1024]; if (!filename) { -qerror_report(QERR_MISSING_PARAMETER, snapshot_file); +qerror_report(QERR_MISSING_PARAMETER, snapshot-file); ret = -1; goto out; } diff --git a/hmp-commands.hx b/hmp-commands.hx index 6ad8806..c857827 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -840,7 +840,7 @@ ETEXI { .name = snapshot_blkdev, -.args_type = device:B,snapshot_file:s?,format:s?, +.args_type = device:B,snapshot-file:s?,format:s?, .params = device [new-image-file] [format], .help = initiates a live snapshot\n\t\t\t of device. If a new image file is specified, the\n\t\t\t diff --git a/qmp-commands.hx b/qmp-commands.hx index 92c5c3a..5d44edf 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -694,6 +694,40 @@ Example: EQMP { +.name = blockdev-snapshot-sync, +.args_type = device:B,snapshot-file:s?,format:s?, +.params = device [new-image-file] [format], +.user_print = monitor_user_noop, +.mhandler.cmd_new = do_snapshot_blkdev, +}, + +SQMP +blockdev-snapshot-sync +-- + +Synchronous snapshot of a block device. snapshot-file specifies the +target of the new image. If the file exists, or if it is a device, the +snapshot will be created in the existing file/device. If does not +exist, a new file will be created. format specifies the format of the +snapshot image, default is qcow2. + +Arguments: + +- device: device name to snapshot (json-string) +- snapshot-file: name of new image file (json-string) +- format: format of new image (json-string, optional) + +Example: + +- { execute: blockdev-snapshot, arguments: { device: ide-hd0, +snapshot-file: +/some/place/my-image, +format: qcow2 } } +- { return: {} } + +EQMP + +{ .name = balloon, .args_type = value:M, .params = target, -- 1.7.6.134.gcf13f
Re: [Qemu-devel] [PATCH v3] QMP: add snapshot_blkdev command
On 07/11/11 22:24, Luiz Capitulino wrote: On Mon, 11 Jul 2011 20:01:09 +0200 jes.soren...@redhat.com wrote: From: Jes Sorensen jes.soren...@redhat.com Add QMP bits for snapshot_blkdev command. This is the same as snapshot_blkdev in the human monitor. The command is synchronous. In the future async commands and or a break down of the functionality into multiple commands might be added. Signed-off-by: Jes Sorensen jes.soren...@redhat.com --- qmp-commands.hx | 35 +++ 1 files changed, 35 insertions(+), 0 deletions(-) diff --git a/qmp-commands.hx b/qmp-commands.hx index 92c5c3a..2b9f6af 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -694,6 +694,41 @@ Example: EQMP { +.name = blockdev-snapshot-sync, +.args_type = device:B,snapshot-file:s?,format:s?, You changed from an underline to a hyphen as I asked, but the implementation still expects an underline. I fixed that myself to avoid multiple submissions because of a small thing (also fixed the command name in the subject). The patch I merged follows for reference. Please, test your patches before submitting next time. Sorry that is no go, you just broke the hmp implementation - you cannot change the hmp behavior like that. Jes
Re: [Qemu-devel] [PATCH v3] QMP: add snapshot_blkdev command
On Mon, 11 Jul 2011 22:28:57 +0200 Jes Sorensen jes.soren...@redhat.com wrote: On 07/11/11 22:24, Luiz Capitulino wrote: On Mon, 11 Jul 2011 20:01:09 +0200 jes.soren...@redhat.com wrote: From: Jes Sorensen jes.soren...@redhat.com Add QMP bits for snapshot_blkdev command. This is the same as snapshot_blkdev in the human monitor. The command is synchronous. In the future async commands and or a break down of the functionality into multiple commands might be added. Signed-off-by: Jes Sorensen jes.soren...@redhat.com --- qmp-commands.hx | 35 +++ 1 files changed, 35 insertions(+), 0 deletions(-) diff --git a/qmp-commands.hx b/qmp-commands.hx index 92c5c3a..2b9f6af 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -694,6 +694,41 @@ Example: EQMP { +.name = blockdev-snapshot-sync, +.args_type = device:B,snapshot-file:s?,format:s?, You changed from an underline to a hyphen as I asked, but the implementation still expects an underline. I fixed that myself to avoid multiple submissions because of a small thing (also fixed the command name in the subject). The patch I merged follows for reference. Please, test your patches before submitting next time. Sorry that is no go, you just broke the hmp implementation - you cannot change the hmp behavior like that. HMP uses positional arguments, so changing argument names makes no difference. And, apart from some exceptions, it's not an stable interface, anyway...
[Qemu-devel] [PATCH v3] QMP: add snapshot_blkdev command
From: Jes Sorensen jes.soren...@redhat.com Add QMP bits for snapshot_blkdev command. This is the same as snapshot_blkdev in the human monitor. The command is synchronous. In the future async commands may be added with the name _async/-async. Signed-off-by: Jes Sorensen jes.soren...@redhat.com --- qmp-commands.hx | 38 ++ 1 files changed, 38 insertions(+), 0 deletions(-) diff --git a/qmp-commands.hx b/qmp-commands.hx index fbd98ee..24e9c3e 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -667,6 +667,44 @@ Example: EQMP { +.name = blockdev-snapshot, +.args_type = device:B,snapshot_file:s?,format:s?, +.params = device [new-image-file] [format], +.help = initiates a live snapshot\n\t\t\t + of device. If a new image file is specified, the\n\t\t\t + new image file will become the new root image.\n\t\t\t + If format is specified, the snapshot file will\n\t\t\t + be created in that format. Otherwise the\n\t\t\t + snapshot will be internal! (currently unsupported), +.user_print = monitor_user_noop, +.mhandler.cmd_new = do_snapshot_blkdev, +}, + +SQMP +blockdev-snapshot-sync +-- + +Synchronous snapshot of block device, using snapshot file as target, +if provided. + +Arguments: + +- device: device name to snapshot (json-string) +- snapshot_file: name of new image file (json-string) +- format: format of now image (json-string) + +Example: + +- { execute: blockdev-snapshot, arguments: { device: ide-hd0, +snapshot_file: +/some/place/my-image, + format: qcow2 + } } +- { return: {} } + +EQMP + +{ .name = balloon, .args_type = value:M, .params = target, -- 1.7.4.4