Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list

[Michael Roth]

On Thu, Mar 29, 2012 at 01:26:34PM -0500, Anthony Liguori wrote:
> On 03/29/2012 12:26 PM, Luiz Capitulino wrote:
> >This allows for QAPI functions to receive a variable-length argument
> >list. This is going to be used by device_add and netdev_add commands.
> >
> >In the schema, the argument list is represented by type name '**',
> >like this example:
> >
> >     { 'command': 'foo', 'data': { 'arg-list': '**' } }
> >
> >Each argument is represented by the KeyValues type and the C
> >implementation should expect a KeyValuesList, like:
> >
> >     void qmp_foo(KeyValuesList *values_list, Error **errp);
> >
> >XXX: This implementation is simple but very hacky. We just iterate
> >      through all arguments and build the KeyValuesList list to be
> >      passed to the QAPI function.
> >
> >      Maybe we could have a kwargs type, that does exactly this but
> >      through a visitor instead?
> >
> >Signed-off-by: Luiz Capitulino<address@hidden>
> 
> What about just treating '**' as "marshal remaining arguments to a
> string" and then pass that string to device_add?  qmp_device_add can
> then parse that string with QemuOpts.

Since currently we explicitly point qmp to the marshaller anyway, we
could also just treat '**' as an indicator to not generate a marshaller.
Then, we open-code the marshaller to process the QDict, rather than embedding
it in the script or passing it through to qmp_device_add().

>From the perspective of qmp_device_add() it then just looks like any
other qmp command.

> 
> It's a bit ugly, but that's how things worked.  When we introduce
> qom_add, this problem goes away because you would make multiple
> calls to qom_set to set all of the properties.
> 
> Regards,
> 
> Anthony Liguori
> 
> >---
> >  qapi-schema.json         |   15 +++++++++++++++
> >  scripts/qapi-commands.py |   31 ++++++++++++++++++++++++++++---
> >  scripts/qapi.py          |    2 ++
> >  3 files changed, 45 insertions(+), 3 deletions(-)
> >
> >diff --git a/qapi-schema.json b/qapi-schema.json
> >index 0d11d6e..25bd487 100644
> >--- a/qapi-schema.json
> >+++ b/qapi-schema.json
> >@@ -1701,3 +1701,18 @@
> >  # Since: 1.1
> >  ##
> >  { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
> >+
> >+##
> >+# @KeyValues:
> >+#
> >+# A generic representation of a key value pair.
> >+#
> >+# @key: the name of the item
> >+#
> >+# @value: the string representation of the item value.  This typically 
> >follows
> >+#         QEMU's command line parsing format.  See the man pages for more
> >+#         information.
> >+#
> >+# Since: 0.14.0
> >+##
> >+{ 'type': 'KeyValues', 'data': {'key': 'str', 'value': 'str'} }
> >diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> >index 30a24d2..75a6e81 100644
> >--- a/scripts/qapi-commands.py
> >+++ b/scripts/qapi-commands.py
> >@@ -146,19 +146,44 @@ v = qmp_input_get_visitor(mi);
> >                       obj=obj)
> >
> >      for argname, argtype, optional, structured in parse_args(args):
> >-        if optional:
> >+        if optional and not '**':
> >              ret += mcgen('''
> >  visit_start_optional(v,&has_%(c_name)s, "%(name)s", errp);
> >  if (has_%(c_name)s) {
> >  ''',
> >                           c_name=c_var(argname), name=argname)
> >              push_indent()
> >-        ret += mcgen('''
> >+        if argtype == '**':
> >+            if dealloc:
> >+                ret += mcgen('''
> >+qapi_free_KeyValuesList(%(obj)s);
> >+''',
> >+                        obj=c_var(argname))
> >+            else:
> >+                ret += mcgen('''
> >+{
> >+    const QDictEntry *entry;
> >+    v = v; /* fix me baby */
> >+
> >+    for (entry = qdict_first(args); entry; entry = qdict_next(qdict, 
> >entry)) {
> >+        KeyValuesList *item = g_malloc0(sizeof(*item));
> >+        item->value = g_malloc0(sizeof(*item->value));
> >+        item->value->key = g_strdup(qdict_entry_key(entry));
> >+        item->value->value = 
> >g_strdup(qstring_get_str(qobject_to_qstring(qdict_entry_value(entry))));
> >+
> >+        item->next = %(obj)s;
> >+        %(obj)s = item;
> >+    }
> >+}
> >+''',
> >+                        obj=c_var(argname))
> >+        else:
> >+            ret += mcgen('''
> >  %(visitor)s(v,&%(c_name)s, "%(name)s", errp);
> >  ''',
> >                       c_name=c_var(argname), name=argname, argtype=argtype,
> >                       visitor=type_visitor(argtype))
> >-        if optional:
> >+        if optional and not '**':
> >              pop_indent()
> >              ret += mcgen('''
> >  }
> >diff --git a/scripts/qapi.py b/scripts/qapi.py
> >index e062336..87b9ee6 100644
> >--- a/scripts/qapi.py
> >+++ b/scripts/qapi.py
> >@@ -163,6 +163,8 @@ def c_type(name):
> >          return 'bool'
> >      elif name == 'number':
> >          return 'double'
> >+    elif name == '**':
> >+        return 'KeyValuesList *'
> >      elif type(name) == list:
> >          return '%s *' % c_list_type(name[0])
> >      elif is_enum(name):
>