[Qemu-devel] [PATCH 0/1] QMP command for snapshot_blkdev

[Qemu-devel] [PATCH 0/1] QMP command for snapshot_blkdev

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi,

I discussed the issue of a QMP command for live snapshot with Anthony,
and we have agreed that it is fine to have a QMP command that matches
the current human monitor command. This doesn't preclude that in the
future someone may want to add support breaking live snapshots into
multiple commands.

Cheers,
Jes

Jes Sorensen (1):
  QMP: add snapshot_blkdev command

 qmp-commands.hx |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

-- 
1.7.4.4

[Qemu-devel] [PATCH 1/1] QMP: add snapshot_blkdev command

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@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.Sorensen@redhat.com>
---
 qmp-commands.hx |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index 92c5c3a..eb135c1 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -694,6 +694,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 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 1/1] QMP: add snapshot_blkdev command

[Luiz Capitulino]

On Fri,  8 Jul 2011 14:00:13 +0200
Jes.Sorensen@redhat.com wrote:

> From: Jes Sorensen <Jes.Sorensen@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.Sorensen@redhat.com>
> ---
>  qmp-commands.hx |   38 ++++++++++++++++++++++++++++++++++++++
>  1 files changed, 38 insertions(+), 0 deletions(-)
> 
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 92c5c3a..eb135c1 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -694,6 +694,44 @@ Example:
>  EQMP
>  
>      {
> +        .name       = "blockdev-snapshot",

blockdev-snapshot-sync.

> +        .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)",

The 'otherwise' part is a bit confusing. You document something as if it were
supported then you say it's not supported. I recommend to just not document it
instead.

> +        .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. 

In QMP only this text is used, the text in '.help' is discarded. Please,
move all command documentation here.

> +
> +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":

We are trying to standardize the use of hyphen in QMP.

> +                                                    "/some/place/my-image",
> +						    "format": "qcow2"
> +                                                   } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
>          .name       = "balloon",
>          .args_type  = "value:M",
>          .params     = "target",

Re: [Qemu-devel] [PATCH 1/1] QMP: add snapshot_blkdev command

[Jes Sorensen]

On 07/11/11 18:35, Luiz Capitulino wrote:
> On Fri,  8 Jul 2011 14:00:13 +0200
> Jes.Sorensen@redhat.com wrote:
> 
>> From: Jes Sorensen <Jes.Sorensen@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.Sorensen@redhat.com>
>> ---
>>  qmp-commands.hx |   38 ++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 38 insertions(+), 0 deletions(-)
>>
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 92c5c3a..eb135c1 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -694,6 +694,44 @@ Example:
>>  EQMP
>>  
>>      {
>> +        .name       = "blockdev-snapshot",
> 
> blockdev-snapshot-sync.

argh, will fix

>> +        .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)",
> 
> The 'otherwise' part is a bit confusing. You document something as if it were
> supported then you say it's not supported. I recommend to just not document it
> instead.

Ok

>> +        .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. 
> 
> In QMP only this text is used, the text in '.help' is discarded. Please,
> move all command documentation here.

If .help isn't used, then please remove it from the struct. It is really
pointless to keep carrying both around.

>> +
>> +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":
> 
> We are trying to standardize the use of hyphen in QMP.

Sorry, but I haven't got a clue what you mean here.

Jes

Re: [Qemu-devel] [PATCH 1/1] QMP: add snapshot_blkdev command

[Luiz Capitulino]

On Mon, 11 Jul 2011 19:50:45 +0200
Jes Sorensen <Jes.Sorensen@redhat.com> wrote:

> On 07/11/11 18:35, Luiz Capitulino wrote:
> > On Fri,  8 Jul 2011 14:00:13 +0200
> > Jes.Sorensen@redhat.com wrote:
> > 
> >> From: Jes Sorensen <Jes.Sorensen@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.Sorensen@redhat.com>
> >> ---
> >>  qmp-commands.hx |   38 ++++++++++++++++++++++++++++++++++++++
> >>  1 files changed, 38 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >> index 92c5c3a..eb135c1 100644
> >> --- a/qmp-commands.hx
> >> +++ b/qmp-commands.hx
> >> @@ -694,6 +694,44 @@ Example:
> >>  EQMP
> >>  
> >>      {
> >> +        .name       = "blockdev-snapshot",
> > 
> > blockdev-snapshot-sync.
> 
> argh, will fix
> 
> >> +        .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)",
> > 
> > The 'otherwise' part is a bit confusing. You document something as if it were
> > supported then you say it's not supported. I recommend to just not document it
> > instead.
> 
> Ok
> 
> >> +        .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. 
> > 
> > In QMP only this text is used, the text in '.help' is discarded. Please,
> > move all command documentation here.
> 
> If .help isn't used, then please remove it from the struct. It is really
> pointless to keep carrying both around.

Yes, sorry for that. It will be completely dropped when we move to the QAPI
(which uses a schema file).

> 
> >> +
> >> +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":
> > 
> > We are trying to standardize the use of hyphen in QMP.
> 
> Sorry, but I haven't got a clue what you mean here.

The argument is called "snapshot_file" but it should be called snapshot-file.