On 08/15/2012 01:41 AM, Pavel Hrdina wrote:
HMP command "savevm" now takes extra optional force parameter to specifi whether
s/specifi/specify/
replace existing snapshot or not.
QMP command "vm-snapshot-save" has also extra optional force parameter and
name parameter isn't optional anymore.
When HMP omits the name, what name are you using in it's place? Either
a nameless snapshot is okay (and QMP must expose it), or it is an error
(and HMP must now be passing a reasonable name).
/me reads patch
Oh, there was always a name; the default name was a timestamp, and you
moved the timestamp creation into HMP.
+++ b/hmp-commands.hx
@@ -264,19 +264,19 @@ ETEXI
{
.name = "savevm",
- .args_type = "name:s?",
- .params = "[tag|id]",
- .help = "save a VM snapshot. If no tag or id are provided, a new
snapshot is created",
+ .args_type = "name:s?,force:b?",
+ .params = "[tag|id] [on|off]",
Rather than sticking 'force' as an optional parameter at the end,
instead you want to add '-f' as a flag at the beginning.
.args_type = "force:-b,name:s?",
.params = "[-f] name"
By doing this, you no longer need 17/18.
+ .help = "save a VM snapshot. To replace existing snapshot use force
parameter.",
.mhandler.cmd = hmp_vm_snapshot_save,
},
STEXI
@item savevm address@hidden|@var{id}]
@findex savevm
-Create a snapshot of the whole virtual machine. If @var{tag} is
-provided, it is used as human readable identifier. If there is already
-a snapshot with the same tag or ID, it is replaced. More info at
address@hidden
+Create a snapshot of the whole virtual machine. Parameter "name" is optional.
+If @var{tag} is provided, it is used as human readable identifier. If there is
+already a snapshot with the same tag, force argument need to be "yes" to
s/force argument need/the force argument needs/
+++ b/qapi-schema.json
@@ -2394,21 +2394,23 @@
##
# @vm-snapshot-save:
#
-# Create a snapshot of the whole virtual machine. If 'tag' is provided,
-# it is used as human readable identifier. If there is already a snapshot
-# with the same tag or ID, it is replaced.
+# Create a snapshot of the whole virtual machine. Provided 'tag' is used as
+# human readable identifier. If there is already a snapshot with the same tag,
+# force argument need to be "yes" to replace it.
s/need to be "yes"/needs to be 'true'/
#
# The VM is automatically stopped and resumed and saving a snapshot can take
# a long time.
#
-# @name: tag or id of new or existing snapshot
+# @name: tag of new or existing snapshot
Given your new semantics, @name must be tag when @force is false; but
the old HMP semantics allowed 'savevm 1' to rewrite snapshot id 1
regardless of the name, all while keeping the old tag. Does the new HMP
code do an id lookup, and pass in the correct @name to the QMP code?
Should the QMP code allow the same semantics of being able to pass
@force=true and @name=id instead of the normal creation of @name=tag?
+#
+# @force: specify whether existing snapshot is replaced or not
Based on the JSON below, this needs an #optional marking, as well as
mentioning that the default is false.
#
# Returns: Nothing on success
# If an error occurs, GenericError with error message
Knowing that a creation failed due to a snapshot already existing by the
same name or id might be worth a distinct error class (do we already
have an error class that we could reuse?)