Re: [Qemu-devel] [PATCH 04/14] qapi: convert query-name

[Luiz Capitulino]

On Wed, 24 Aug 2011 13:42:59 -0500
Anthony Liguori <address@hidden> wrote:

> A simple example conversion 'info name'.  This also adds the new files for
> QMP and HMP.

The QAPI makes QMP commands real good!

(more comments below).

> 
> Signed-off-by: Anthony Liguori <address@hidden>
> ---
>  Makefile.objs    |    1 +
>  hmp.c            |   26 ++++++++++++++++++++++++++
>  hmp.h            |   22 ++++++++++++++++++++++
>  monitor.c        |   27 +++++----------------------
>  qapi-schema.json |   23 +++++++++++++++++++++++
>  qmp.c            |   28 ++++++++++++++++++++++++++++
>  6 files changed, 105 insertions(+), 22 deletions(-)
>  create mode 100644 hmp.c
>  create mode 100644 hmp.h
>  create mode 100644 qmp.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index c02431f..570dda7 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -397,6 +397,7 @@ qapi-nested-y += qmp-registry.o qmp-dispatch.o
>  qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
>  
>  common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o $(qapi-obj-y)
> +common-obj-y += qmp.o hmp.o
>  
>  ######################################################################
>  # guest agent
> diff --git a/hmp.c b/hmp.c
> new file mode 100644
> index 0000000..47e1ff7
> --- /dev/null
> +++ b/hmp.c
> @@ -0,0 +1,26 @@
> +/*
> + * Human Monitor Interface
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Anthony Liguori   <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hmp.h"
> +#include "qmp-commands.h"
> +
> +void hmp_info_name(Monitor *mon)
> +{
> +    NameInfo *info;
> +
> +    info = qmp_query_name(NULL);
> +    if (info->has_name) {
> +        monitor_printf(mon, "%s\n", info->name);
> +    }
> +    qapi_free_NameInfo(info);
> +}
> diff --git a/hmp.h b/hmp.h
> new file mode 100644
> index 0000000..5fe73f1
> --- /dev/null
> +++ b/hmp.h
> @@ -0,0 +1,22 @@
> +/*
> + * Human Monitor Interface
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Anthony Liguori   <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef HMP_H
> +#define HMP_H
> +
> +#include "qemu-common.h"
> +#include "qapi-types.h"
> +
> +void hmp_info_name(Monitor *mon);
> +
> +#endif
> diff --git a/monitor.c b/monitor.c
> index ef204c0..6a3a3d2 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -61,6 +61,8 @@
>  #include "trace.h"
>  #endif
>  #include "ui/qemu-spice.h"
> +#include "qmp-commands.h"
> +#include "hmp.h"
>  
>  //#define DEBUG
>  //#define DEBUG_COMPLETION
> @@ -757,24 +759,6 @@ static void do_info_version(Monitor *mon, QObject 
> **ret_data)
>          'micro': %d }, 'package': %s }", major, minor, micro, 
> QEMU_PKGVERSION);
>  }
>  
> -static void do_info_name_print(Monitor *mon, const QObject *data)
> -{
> -    QDict *qdict;
> -
> -    qdict = qobject_to_qdict(data);
> -    if (qdict_size(qdict) == 0) {
> -        return;
> -    }
> -
> -    monitor_printf(mon, "%s\n", qdict_get_str(qdict, "name"));
> -}
> -
> -static void do_info_name(Monitor *mon, QObject **ret_data)
> -{
> -    *ret_data = qemu_name ? qobject_from_jsonf("{'name': %s }", qemu_name) :
> -                            qobject_from_jsonf("{}");
> -}
> -
>  static QObject *get_cmd_dict(const char *name)
>  {
>      const char *p;
> @@ -3069,8 +3053,7 @@ static const mon_cmd_t info_cmds[] = {
>          .args_type  = "",
>          .params     = "",
>          .help       = "show the current VM name",
> -        .user_print = do_info_name_print,
> -        .mhandler.info_new = do_info_name,
> +        .mhandler.info = hmp_info_name,
>      },
>      {
>          .name       = "uuid",
> @@ -3266,8 +3249,8 @@ static const mon_cmd_t qmp_query_cmds[] = {
>          .args_type  = "",
>          .params     = "",
>          .help       = "show the current VM name",
> -        .user_print = do_info_name_print,
> -        .mhandler.info_new = do_info_name,
> +        .mhandler.cmd_new = qmp_marshal_input_query_name,
> +        .qapi       = true,
>      },
>      {
>          .name       = "uuid",
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 7fcefdb..654409b 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1,3 +1,26 @@
>  # -*- Mode: Python -*-
>  #
>  # QAPI Schema
> +
> +##
> +# @NameInfo:
> +#
> +# Guest name information.
> +#
> +# @name: #optional The name of the guest

Isn't it the VM name?

> +#
> +# Since 0.14.0
> +##
> +{ 'type': 'NameInfo', 'data': {'*name': 'str'} }

Is the type name ('NameInfo' in this case) going to be public in libqmp? If yes,
Isn't VmNameInfo better?

> +
> +##
> +# @query-name:
> +#
> +# Return the name information of a guest.
> +#
> +# Returns: @NameInfo of the guest
> +#
> +# Since 0.14.0
> +##
> +{ 'command': 'query-name', 'returns': 'NameInfo' }
> +
> diff --git a/qmp.c b/qmp.c
> new file mode 100644
> index 0000000..8aa9c66
> --- /dev/null
> +++ b/qmp.c
> @@ -0,0 +1,28 @@
> +/*
> + * QEMU Management Protocol
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Anthony Liguori   <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu-common.h"
> +#include "sysemu.h"
> +#include "qmp-commands.h"
> +
> +NameInfo *qmp_query_name(Error **errp)
> +{
> +    NameInfo *info = g_malloc0(sizeof(*info));
> +
> +    if (qemu_name) {
> +        info->has_name = true;
> +        info->name = g_strdup(qemu_name);
> +    }

Isn't it better to keep using qemu_malloc() & friends but change its
current implementation to use the glib malloc functions?



> +
> +    return info;
> +}