On 04/27/2011 10:05 AM, Luiz Capitulino wrote:
On Mon, 18 Apr 2011 16:27:01 +0200
jes.soren...@redhat.com wrote:
From: Jes Sorensen<jes.soren...@redhat.com>
This is quivalent to snapshot_blkdev in the human monitor, with _sync
added to the command name to make it explicit that the command is
synchronous and leave space for a future async version.
I'm not sure appending "_sync" is such a good convention, most commands
are sync today and they don't have it. I'd prefer to call it snapshot_blkdev
and note in the documentation how it works.
It probably should be called snapshot_blkdev_broken because that's what
it really is.
The '_sync' is there to indicate that this command doesn't properly
implement asynchronous logic and can break a guest.
I'd actually prefer that we not expose this command through QMP at all
and instead implement a proper snapshot command.
Regards,
Anthony Liguori
On the other hand, I'm not sure how Anthony is going to model async
commands, so maybe he has a better suggestion.
Signed-off-by: Jes Sorensen<jes.soren...@redhat.com>
---
qmp-commands.hx | 26 ++++++++++++++++++++++++++
1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/qmp-commands.hx b/qmp-commands.hx
index fbd98ee..b8f537c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -667,6 +667,32 @@ 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
This doesn't follow QMP doc convention, which is:
command name
------------
(Explain how the command works, like you do below)
Arguments
Example
+Synchronous snapshot of block device, using snapshot file as target
+if provided.
It's not optional in HMP:
(qemu) snapshot_blkdev ide0-hd0
Parameter 'snapshot_file' is missing
(qemu)
And the command argument is called "snapshot_file" not "new-image-file"
as written in the HMP help text.
+
+If a new image file is specified, the new image file will become the
+new root image. If format is specified, the snapshot file will be
+created in that format. Otherwise the snapshot will be internal!
+(currently unsupported).
Sorry for the stupid question, but what's a "new root image"? Also, all
these assumptions seem human features to me, as it can save some typing
and I can poke around to see where the snapshots are stored.
All arguments should be mandatory in QMP, IMO.
Finally, what's the expect behavior when -snapshot is used? I'm getting
this:
(qemu) snapshot_blkdev ide0-hd0 snap-test
Could not open '/tmp/vl.6w8YXA'
(qemu)
At first, I don't see why we shouldn't generate the live snapshot, but anyway,
any special behavior like this should be noted in the section called Notes
in the command's documentation.
+
+Errors:
+If creating the new snapshot image fails, QEMU will continue running
+on the original image. If switching to the newly created image fails,
+it will be attempted to fall back to the original image and return
+QERR_OPEN_FILE_FAILED with the snapshot filename. If re-opening
+the original image fails, QERR_OPEN_FILE_FAILED will be returned with
+the original image filename.
+EQMP
+
+ {
.name = "balloon",
.args_type = "value:M",
.params = "target",
