Re: [Qemu-devel] [PATCH 15/18] qapi: implement support for variable argument list

[Paolo Bonzini]

Il 17/04/2012 22:42, Luiz Capitulino ha scritto:
> On Tue, 17 Apr 2012 22:26:55 +0200
> Paolo Bonzini <address@hidden> wrote:
> 
>> Il 17/04/2012 21:36, Luiz Capitulino ha scritto:
>>> +            switch(qobject_type(obj)) {
>>> +            case QTYPE_QSTRING:
>>> +                qstring_append(arglist,
>>> +                               qstring_get_str(qobject_to_qstring(obj)));
>>> +                break;
>>
>> Does this escape commas correctly?
> 
> No, but does it have to? Does QemuOpts accept an option with a coma in it?

Yes, ",," is parsed as ",".

>> It seems much easier to use no_gen and qemu_opts_from_qdict...  Then
>> cmd_netdev_add can be
> 
> netdev_add/del is expected to be a stable interface, so we can't use no_gen.

You can have hmp_netdev_add and the no_gen qmp_netdev_add as front-ends
for the QAPI cmd_netdev_add.  I think it's fair when we have to take
into account backwards-compatibility.  The conversion gives correct
error propagation, so even though QemuOpts still leaks it's a step in
the right direction.

>>   void cmd_foo(QemuOpts *arglist, Error **errp);
> 
> Until now we're treating hmp.c like an external QMP C client, using QemuOpts
> this way will leak qemu internals to hmp.c...

True, but on the other hand it sounds strange to have QAPI clients
encoding options manually and escaping commas.

A KeyValueList (list of string->string associations) could be an
alternative, but I do think that ultimately we want to have a visitor
and remove QemuOpts altogether from net.c.  I can write a proof of
concept in a couple of weeks.  Again, we can proceed in steps.

Paolo