qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] monitor: introduce query-config-schema


From: Osier Yang
Subject: Re: [Qemu-devel] [PATCH] monitor: introduce query-config-schema
Date: Fri, 19 Apr 2013 21:44:08 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4

On 19/04/13 20:02, Osier Yang wrote:
On 19/04/13 17:52, Amos Kong wrote:
Libvirt doesn't have a stable way to know option support

Actually no way now.

Libvirt swtiched to use qmp to collect the qemu capabilities
for qemu newer than 1.2.0,  thus there is no way to probe either
if a option or a property is supported or not by QEMU. Any
support for the new options/properties in libvirt are blocked.

Except we go back to probe using the help output, but it's not
right way to go.

Consider to change the sentence to:

Libvirt has no way to probe if an option or property is supported,

detail. This patch introdued a new qmp command to query

s/introdued/introduces/

configuration schema information. hmp command isn't added.

Not quite sure if QEMU wants the hmp command, though I think no,
per all the monitor commands are converted (or going to be converted)
to qmp. Anyway, libvirt won't want it.


Signed-off-by: Amos Kong <address@hidden>
CC: Osier Yang <address@hidden>
CC: Anthony Liguori <address@hidden>
---
Reposted this patch: https://www.redhat.com/archives/libvir-list/2013-January/msg01656.html
---
  qapi-schema.json       |   26 ++++++++++++++++++++++++++
  qemu-options-wrapper.h |   16 ++++++++++++++++
  qmp-commands.hx        |   32 ++++++++++++++++++++++++++++++++
  qmp.c                  |   36 ++++++++++++++++++++++++++++++++++++
  4 files changed, 110 insertions(+), 0 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 751d3c2..f781372 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3505,3 +3505,29 @@
      '*asl_compiler_rev':  'uint32',
      '*file':              'str',
      '*data':              'str' }}
+
+##
+# @ConfigSchemaInfo:
+#
+# Configration schema information.
+#
+# @option: option name
+#
+# @config-schema: configuration schema string of one option

Since if no option name is specified, all the config schema is returned, you
might want to change this comment a bit.

+#
+# Since 1.5
+##
+{ 'type': 'ConfigSchemaInfo', 'data': {'option': 'str', 'config-schema': 'str'} }
+
+##
+# @query-config-schema
+#
+# Query configuration schema information of options

And this.

+#
+# @option: #optional option name
+#
+# Returns: @ConfigSchemaInfo list
+#
+# Since 1.5
+##
+{'command': 'query-config-schema', 'data': {'*option': 'str'}, 'returns': ['ConfigSchemaInfo']}
diff --git a/qemu-options-wrapper.h b/qemu-options-wrapper.h
index 13bfea0..6449268 100644
--- a/qemu-options-wrapper.h
+++ b/qemu-options-wrapper.h
@@ -18,6 +18,22 @@
    #define DEFHEADING(text) ARCHHEADING(text, QEMU_ARCH_ALL)
  +#elif defined(QEMU_OPTIONS_GENERATE_CONFIG)
+
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask) \
+    opt_help,
+
+#define DEFHEADING(text)
+#define ARCHHEADING(text, arch_mask)
+
+#elif defined(QEMU_OPTIONS_GENERATE_NAME)

How about QEMU_OPTIONS_GENERATE_OPTION_NAME?

+
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask) \
+    option,
+
+#define DEFHEADING(text)
+#define ARCHHEADING(text, arch_mask)
+
  #elif defined(QEMU_OPTIONS_GENERATE_OPTIONS)
    #define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4d65422..1bb691e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2414,7 +2414,39 @@ EQMP
          .args_type  = "",
          .mhandler.cmd_new = qmp_marshal_input_query_uuid,
      },
+SQMP
+query-config-schema
+------------
+
+Show configuration schema.
+
+Return configuration schema of one option, if no option is assigned,
+will return configuration schema of all options.
+
+- "option": option name
+- "config-schema": configuration schema string of one option

And this.

+
+Example:
+-> {"execute": "query-config-schema", "arguments" : {"option": "boot"}}
+<- {"return": [
+ {"config-schema": "-boot [order=drives][,once=drives][,menu=on|off]\n + [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time][,strict=on|off]\n
+    'drives': floppy (a), hard disk (c), CD-ROM (d), network (n)\n
+ 'sp_name': the file's name that would be passed to bios as logo picture, if menu=on\n + 'sp_time': the period that splash picture last if menu=on, unit is ms\n + 'rb_timeout': the timeout before guest reboot when boot failed, unit is ms\n",
+      "option": "boot"
+     }
+    ]
+}
+
+EQMP
  +    {
+        .name       = "query-config-schema",
+        .args_type  = "option:s?",
+        .mhandler.cmd_new = qmp_marshal_input_query_config_schema,
+    },

Need one blank line.

  SQMP
  query-migrate
  -------------
diff --git a/qmp.c b/qmp.c
index ed6c7ef..a3f7cc9 100644
--- a/qmp.c
+++ b/qmp.c
@@ -24,6 +24,9 @@
  #include "hw/qdev.h"
  #include "sysemu/blockdev.h"
  #include "qom/qom-qobject.h"
+#include "qemu-options.h"
+#include "net/net.h"
+#include "exec/gdbstub.h"
    NameInfo *qmp_query_name(Error **errp)
  {
@@ -78,6 +81,39 @@ UuidInfo *qmp_query_uuid(Error **errp)
      return info;
  }
  +ConfigSchemaInfoList *qmp_query_config_schema(bool has_option,
+                              const char *option, Error **errp)

Indention problem.

+{
+    ConfigSchemaInfoList *conf_list = NULL, *entry;
+    ConfigSchemaInfo *info;
+
+    char const *optionstr[] = {

s/optionstr/options/, ?

+#define QEMU_OPTIONS_GENERATE_NAME
+#include "qemu-options-wrapper.h"
+    };
+
+    char const *configstr[] = {

s/configstr/configs/, ?

+#define QEMU_OPTIONS_GENERATE_CONFIG
+#include "qemu-options-wrapper.h"
+    };
+
+    int i;
+    for (i = 0; i < sizeof(optionstr) / sizeof(char *); i++) {
+        if (!has_option || !strcmp(option, optionstr[i])) {

How about an invalid option name is passed? E.g.

{"execute": "query-config-schema", "arguments" : {"option": "foo"}}

Should we have an QMP error instead of sliently ignoring it with nothing
returned?

How about changing the logic to:

    bool found = false;

    if (has_option) {
        for (i = 0; i < sizeof(options) / sizeof(char *); i++) {
            if (!strcmp(option, option[i])) {
                found = true;
                break;
            }
        }

        if (found) {
            /* Return the found option schema */
        } else {
            /* QMP error */
        }
    } else {
        /* Return the schema of all options */
    }

+            info = g_malloc0(sizeof(*info));
+            info->option = g_strdup(option);
+            info->config_schema = g_strdup(configstr[i]);
+
+            entry = g_malloc0(sizeof(*entry));
+            entry->value = info;
+            entry->next = conf_list;
+            conf_list = entry;

No macros or helpers to manage the list?

Osier





reply via email to

[Prev in Thread] Current Thread [Next in Thread]