[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] QMP: add snapshot_blkdev_sync command

From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH] QMP: add snapshot_blkdev_sync command
Date: Wed, 09 Mar 2011 09:46:50 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20110223 Lightning/1.0b2 Thunderbird/3.1.8

On 03/09/2011 09:37 AM, address@hidden wrote:
From: Jes Sorensen<address@hidden>

Add QMP bits for snapshot_blkdev_sync command. This is the same as
snapshot_blkdev in the human monitor, but added _sync to the name to
make it explicit that the command is synchronous and leave space for a
future async version.

Signed-off-by: Jes Sorensen<address@hidden>
  qmp-commands.hx |   19 +++++++++++++++++++
  1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9d3cc31..e32187e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -667,6 +667,25 @@ Example:

+        .name       = "snapshot_blkdev_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, 
+                      "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,
+    },
+Synchronous snapshot of block device, using snapshot file as target
+if provided.

Please document the error semantics.

The documentation in .help is discarded for QMP. You should put the docs in the SQMP section.

Also, QMP should use '-' instead of '_'. We should also try to follow the form 'noun'-'verb' so the name would be better as 'blkdev-snapshot-sync'

I'm not sure blkdev is the right prefix. Kevin, what are your thoughts here? Does 'blkdev' make sense for any command operating on a block device (that is, a qdev device that happens to have a block drive, not the same thing as -blockdev that we've discussed in the past).


Anthony Liguori

+    {
          .name       = "balloon",
          .args_type  = "value:M",
          .params     = "target",

reply via email to

[Prev in Thread] Current Thread [Next in Thread]