Patchwork [14/19] Add a query-argv command to QMP

login
register
mail settings
Submitter Daniel P. Berrange
Date June 7, 2010, 2:42 p.m.
Message ID <1275921752-29420-15-git-send-email-berrange@redhat.com>
Download mbox | patch
Permalink /patch/54866/
State New
Headers show

Comments

Daniel P. Berrange - June 7, 2010, 2:42 p.m.
Add a new QMP command called 'query-argv' to information about the command
line arguments supported by the QEMU binary. This is intended to remove the
need for apps to parse '-help' output.

    [
        {
            "name": "help",
        },
        {
            "name": "M",
            "parameters": [
                {
                }
            ]
        },
    ]

NB, command line args which accept parameters have a non-zero length
parameters list. The element of the list is currently empty though
since the parameter names are not easily available in QEMU source in
a format suitable for exposing as a stable ABI.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 monitor.c |    8 ++++++
 monitor.h |    2 +
 vl.c      |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 84 insertions(+), 0 deletions(-)
Anthony Liguori - June 7, 2010, 3:01 p.m.
On 06/07/2010 09:42 AM, Daniel P. Berrange wrote:
> Add a new QMP command called 'query-argv' to information about the command
> line arguments supported by the QEMU binary. This is intended to remove the
> need for apps to parse '-help' output.
>    

This is just as bad as parsing -help output IMHO.

The problem with something like this is that it discourages people from 
using proper APIs to get at capabilities information.

Regards,

Anthony Liguori

>      [
>          {
>              "name": "help",
>          },
>          {
>              "name": "M",
>              "parameters": [
>                  {
>                  }
>              ]
>          },
>      ]
>
> NB, command line args which accept parameters have a non-zero length
> parameters list. The element of the list is currently empty though
> since the parameter names are not easily available in QEMU source in
> a format suitable for exposing as a stable ABI.
>
> Signed-off-by: Daniel P. Berrange<berrange@redhat.com>
> ---
>   monitor.c |    8 ++++++
>   monitor.h |    2 +
>   vl.c      |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 84 insertions(+), 0 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index d55b27b..1c5157d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2544,6 +2544,14 @@ static const mon_cmd_t info_cmds[] = {
>           .mhandler.info_new = do_info_commands,
>       },
>       {
> +        .name       = "argv",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "list QEMU command line argv",
> +        .user_print = monitor_user_noop,
> +        .mhandler.info_new = do_info_argv,
> +    },
> +    {
>           .name       = "network",
>           .args_type  = "",
>           .params     = "",
> diff --git a/monitor.h b/monitor.h
> index ea15469..46b7a0e 100644
> --- a/monitor.h
> +++ b/monitor.h
> @@ -55,4 +55,6 @@ typedef void (MonitorCompletion)(void *opaque, QObject *ret_data);
>
>   void monitor_set_error(Monitor *mon, QError *qerror);
>
> +void do_info_argv(Monitor *mon, QObject **data);
> +
>   #endif /* !MONITOR_H */
> diff --git a/vl.c b/vl.c
> index 8043fac..a76c673 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1987,6 +1987,80 @@ static void version(void)
>       printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
>   }
>
> +
> +/**
> + * do_info_argv():
> + *
> + * Provide info about the command line arguments
> + * supported by the QEMU binary. The returned
> + * data is a QList with one QDict entry for  each named
> + * argument. Each entry's QDict contains the following
> + * keys
> + *
> + *  'name': the command argument name (eg 'drive' for -drive arg)
> + *  'parameters': list of parameter values (if any)
> + *
> + * NB, the 'parameters' key is omitted completely if
> + * the argument has no associated value.
> + *
> + * XXXX details of the parameters are not yet filled in
> + * since this info is not easily available
> + *
> + *     [
> + *         {
> + *             "name": "help",
> + *             "parameters": [
> + *             ]
> + *         },
> + *         {
> + *             "name": "M",
> + *             "parameters": [
> + *                 {
> + *                 }
> + *             ]
> + *         },
> + *     ]
> + */
> +void do_info_argv(Monitor *mon, QObject **data)
> +{
> +    QList *args = qlist_new();
> +    struct {
> +       const char *name;
> +       int has_arg;
> +       const char *help;
> +    } options_help[] = {
> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
> +        { option, opt_arg },
> +#define DEFHEADING(text)
> +#define HAS_ARG 1
> +#include "qemu-options.h"
> +#undef DEF
> +#undef DEFHEADING
> +#undef HAS_ARG
> +       { NULL, 0 },
> +    };
> +    int i;
> +
> +    for (i = 0 ; options_help[i].name != NULL ; i++) {
> +	QObject *opt;
> +
> +	if (options_help[i].has_arg) {
> +	    /* XXX actually fill in the parameter details */
> +	    opt = qobject_from_jsonf("{ 'name': %s, 'parameters': [] }",
> +				     options_help[i].name);
> +	} else {
> +	    opt = qobject_from_jsonf("{ 'name': %s }",
> +				     options_help[i].name);
> +	}
> +
> +
> +	qlist_append_obj(args, opt);
> +    }
> +
> +    *data = QOBJECT(args);
> +}
> +
> +
>   static void help(int exitcode)
>   {
>       const char *options_help =
>
Paolo Bonzini - June 10, 2010, 3:34 p.m.
On 06/07/2010 05:01 PM, Anthony Liguori wrote:
> On 06/07/2010 09:42 AM, Daniel P. Berrange wrote:
>> Add a new QMP command called 'query-argv' to information about the
>> command
>> line arguments supported by the QEMU binary. This is intended to
>> remove the
>> need for apps to parse '-help' output.
>
> This is just as bad as parsing -help output IMHO.
>
> The problem with something like this is that it discourages people from
> using proper APIs to get at capabilities information.

What about a query-qemuopts instead?  This has a well-defined schema 
and, while it won't let you get all arguments, going forward libvirt is 
going to try and use more qemuopts options and only the bare minimum 
legacy options (-incoming, -S).

Paolo
Markus Armbruster - June 26, 2010, 7:30 a.m.
Anthony Liguori <anthony@codemonkey.ws> writes:

> On 06/07/2010 09:42 AM, Daniel P. Berrange wrote:
>> Add a new QMP command called 'query-argv' to information about the command
>> line arguments supported by the QEMU binary. This is intended to remove the
>> need for apps to parse '-help' output.
>>    
>
> This is just as bad as parsing -help output IMHO.
>
> The problem with something like this is that it discourages people
> from using proper APIs to get at capabilities information.

The interfaces to be used by management software should be
self-documenting, i.e. the client should be able to examine the
available interface over that interface.

This applies to QMP.  It equally applies to those parts of command line
we want management software to use.

Example: We don't need to provide machine-readable descriptions for all
the various options to configure devices, because management software
should use -device.  We do need to provide a description for -device.
Note: if we make device_add fully cold-plug capable, we could point
management software to QMP instead.

Such command-line self-documentation would also guide management
software to "proper" use of the command line.

Patch

diff --git a/monitor.c b/monitor.c
index d55b27b..1c5157d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2544,6 +2544,14 @@  static const mon_cmd_t info_cmds[] = {
         .mhandler.info_new = do_info_commands,
     },
     {
+        .name       = "argv",
+        .args_type  = "",
+        .params     = "",
+        .help       = "list QEMU command line argv",
+        .user_print = monitor_user_noop,
+        .mhandler.info_new = do_info_argv,
+    },
+    {
         .name       = "network",
         .args_type  = "",
         .params     = "",
diff --git a/monitor.h b/monitor.h
index ea15469..46b7a0e 100644
--- a/monitor.h
+++ b/monitor.h
@@ -55,4 +55,6 @@  typedef void (MonitorCompletion)(void *opaque, QObject *ret_data);
 
 void monitor_set_error(Monitor *mon, QError *qerror);
 
+void do_info_argv(Monitor *mon, QObject **data);
+
 #endif /* !MONITOR_H */
diff --git a/vl.c b/vl.c
index 8043fac..a76c673 100644
--- a/vl.c
+++ b/vl.c
@@ -1987,6 +1987,80 @@  static void version(void)
     printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n");
 }
 
+
+/**
+ * do_info_argv():
+ *
+ * Provide info about the command line arguments
+ * supported by the QEMU binary. The returned
+ * data is a QList with one QDict entry for  each named
+ * argument. Each entry's QDict contains the following
+ * keys
+ *
+ *  'name': the command argument name (eg 'drive' for -drive arg)
+ *  'parameters': list of parameter values (if any)
+ *
+ * NB, the 'parameters' key is omitted completely if
+ * the argument has no associated value. 
+ *
+ * XXXX details of the parameters are not yet filled in
+ * since this info is not easily available
+ *
+ *     [
+ *         {
+ *             "name": "help",
+ *             "parameters": [
+ *             ]
+ *         },
+ *         {
+ *             "name": "M",
+ *             "parameters": [
+ *                 {
+ *                 }
+ *             ]
+ *         },
+ *     ]
+ */
+void do_info_argv(Monitor *mon, QObject **data)
+{
+    QList *args = qlist_new();
+    struct {
+       const char *name;
+       int has_arg;
+       const char *help;
+    } options_help[] = {
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
+        { option, opt_arg },
+#define DEFHEADING(text)
+#define HAS_ARG 1
+#include "qemu-options.h"
+#undef DEF
+#undef DEFHEADING
+#undef HAS_ARG
+       { NULL, 0 },
+    };
+    int i;
+
+    for (i = 0 ; options_help[i].name != NULL ; i++) {
+	QObject *opt;
+
+	if (options_help[i].has_arg) {
+	    /* XXX actually fill in the parameter details */
+	    opt = qobject_from_jsonf("{ 'name': %s, 'parameters': [] }",
+				     options_help[i].name);
+	} else {
+	    opt = qobject_from_jsonf("{ 'name': %s }",
+				     options_help[i].name);
+	}
+
+
+	qlist_append_obj(args, opt);
+    }
+
+    *data = QOBJECT(args);
+}
+
+
 static void help(int exitcode)
 {
     const char *options_help =