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

2011-07-12 Thread 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

2011-07-12 Thread Luiz Capitulino
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

2011-07-11 Thread Jes . Sorensen
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

2011-07-11 Thread Luiz Capitulino
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

2011-07-11 Thread Jes Sorensen
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

2011-07-11 Thread Luiz Capitulino
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

2011-04-28 Thread Jes . Sorensen
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