From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44644) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SDL1D-0007b5-5E for qemu-devel@nongnu.org; Thu, 29 Mar 2012 15:28:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SDL1B-0003qP-6y for qemu-devel@nongnu.org; Thu, 29 Mar 2012 15:28:22 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:56205) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SDL1A-0003q1-VI for qemu-devel@nongnu.org; Thu, 29 Mar 2012 15:28:21 -0400 Received: by yenr5 with SMTP id r5so2185687yen.4 for ; Thu, 29 Mar 2012 12:28:18 -0700 (PDT) Sender: fluxion Date: Thu, 29 Mar 2012 14:28:13 -0500 From: Michael Roth Message-ID: <20120329192813.GA14158@illuin> References: <1333042003-15490-1-git-send-email-lcapitulino@redhat.com> <1333042003-15490-13-git-send-email-lcapitulino@redhat.com> <4F74A95A.7050403@us.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4F74A95A.7050403@us.ibm.com> Subject: Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: armbru@redhat.com, kraxel@redhat.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com, Luiz Capitulino 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 > > 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): >