[Qemu-devel] [PATCH v3] QMP: add snapshot_blkdev command

[Qemu-devel] [PATCH v3] 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 |   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

[Luiz Capitulino]

On Mon, 11 Jul 2011 20:01:09 +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 |   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.Sorensen@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@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

[Jes Sorensen]

On 07/11/11 22:24, Luiz Capitulino wrote:
> On Mon, 11 Jul 2011 20:01:09 +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 |   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

[Luiz Capitulino]

On Mon, 11 Jul 2011 22:28:57 +0200
Jes Sorensen <Jes.Sorensen@redhat.com> wrote:

> On 07/11/11 22:24, Luiz Capitulino wrote:
> > On Mon, 11 Jul 2011 20:01:09 +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 |   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...

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

[Jes Sorensen]

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

[Luiz Capitulino]

On Tue, 12 Jul 2011 11:26:09 +0200
Jes Sorensen <Jes.Sorensen@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

[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 may be added with the name _async/-async.

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