Re: [Qemu-devel] [PATCH 5/5] qmp: create QMP implementation of loadvm command

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2234 bytes --]

On 12/04/2015 07:44 AM, Denis V. Lunev wrote:
> Unfortunately load_vmstate has a return code (int) and this code is checked
> in the other places. Thus we could not just rename it to qmp_loadvm as
> returns void.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Amit Shah <amit.shah@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> ---
>  migration/savevm.c | 12 ++++++++++++
>  monitor.c          | 12 ++++++------
>  qapi-schema.json   | 13 +++++++++++++
>  qmp-commands.hx    | 23 +++++++++++++++++++++++
>  4 files changed, 54 insertions(+), 6 deletions(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 7846437..07b0bf4 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2103,6 +2103,18 @@ int load_vmstate(const char *name, Error **errp)
>      return 0;
>  }
>  
> +void qmp_loadvm(const char *name, Error **errp)
> +{
> +    int saved_vm_running  = runstate_is_running();
> +    vm_stop(RUN_STATE_RESTORE_VM);
> +
> +    load_vmstate(name, errp);
> +
> +    if (saved_vm_running) {
> +        vm_start();

Do you really mean to be calling vm_start() even if an error was
reported?  But to properly short-circuit, you need a local 'Error *err =
NULL; load_vmstate(name, &err);', rather than reusing errp (since the
caller may pass in NULL).

> +++ b/monitor.c
> @@ -1737,18 +1737,18 @@ void qmp_closefd(const char *fdname, Error **errp)
>  
>  static void hmp_loadvm(Monitor *mon, const QDict *qdict)
>  {
> -    int saved_vm_running  = runstate_is_running();
>      const char *name = qdict_get_str(qdict, "name");
>      Error *local_err = NULL;
>  
> -    vm_stop(RUN_STATE_RESTORE_VM);
> +    if (name == NULL) {

Dead code.  qdict_get_str() crashes rather than returning NULL, if
"name" is not present.

> +++ b/qmp-commands.hx
> @@ -4787,3 +4787,26 @@ EQMP
>          .args_type  = "name:s",
>          .mhandler.cmd_new = qmp_marshal_delvm,
>      },
> +
> +SQMP
> +loadvm
> +------------------

Line too long.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]