Re: [Qemu-devel] [PATCH 2/5] qmp: create qmp_savevm command

[Markus Armbruster]

"Denis V. Lunev" <address@hidden> writes:

> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Juan Quintela <address@hidden>
> CC: Amit Shah <address@hidden>
> CC: Markus Armbruster <address@hidden>
> CC: Eric Blake <address@hidden>
> ---
>  migration/savevm.c |  5 +++++
>  qapi-schema.json   | 13 +++++++++++++
>  qmp-commands.hx    | 25 +++++++++++++++++++++++++
>  3 files changed, 43 insertions(+)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index f83ffd0..565b10a 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2010,6 +2010,11 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
>      }
>  }
>  
> +void qmp_savevm(bool has_name, const char *name, Error **errp)
> +{
> +    do_savevm(has_name ? name : NULL, errp);
> +}
> +

Please name do_savevm() qmp_savevm() and drop this wrapper.

We're working on omitting has_FOO for pointer-valued FOO.

>  void qmp_xen_save_devices_state(const char *filename, Error **errp)
>  {
>      QEMUFile *f;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index b65905f..8cc8b44 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3962,3 +3962,16 @@
>  ##
>  { 'enum': 'ReplayMode',
>    'data': [ 'none', 'record', 'play' ] }
> +
> +##
> +# @savevm
> +#
> +# Save a VM snapshot. Without a name new snapshot is created",
> +#
> +# @name: identifier of a snapshot to be created

Missing #optional tag.

What happens when @name is missing?

What happens when @name names an existing snapshot?

Do we want @name to be optional in QMP?  I dimly remember ambiguity
problems between names and IDs.  Perhaps it'll become clear later in the
series.

The QMP interface needs to make sense on its own, even if that means
deviating from HMP.  Juan, Amit, do you have opinions on the proper QMP
interface for internal snapshots?

> +#
> +# Returns: Nothing on success
> +#
> +# Since 2.6
> +##
> +{ 'command': 'savevm', 'data': {'*name': 'str'} }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 9d8b42f..cd895f6 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -4739,3 +4739,28 @@ Example:
>                   {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
>                    "pop-vlan": 1, "id": 251658240}
>     ]}
> +
> +EQMP
> +
> +SQMP
> +savevm
> +------------------
> +
> +Save a VM snapshot. If no tag or id are provided, a new snapshot is created
> +
> +Arguments:
> +
> +- "name": (optional) snapshot name
> +
> +Example:
> +
> +-> { "execute": "savevm", "arguments": { "name": "snapshot1" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
> +        .name       = "savevm",
> +        .args_type  = "name:s?",
> +        .mhandler.cmd_new = qmp_marshal_savevm,
> +    },