[Qemu-devel] Converting existing name=value,... arguments to QMP

[Qemu-devel] Converting existing name=value,... arguments to QMP

[Markus Armbruster]

The human monitor uses positional arguments.  This is fine; nobody wants
to type "info item=network" instead of "info network".

QMP uses named arguments.  Also fine.

Internally, we use named arguments: we pass them as QDict to the command
handler.  This involves associating positional arguments with names.
Works fine.

There's one little mess hiding in a corner, though: a few commands abuse
a positional argument to get named arguments.  That argument has the
form of a "parameter string": NAME=VALUE,...  For instance:

    pci_add auto nic model=e1000,macaddr=00:11:22:33:44:55

If we convert this naively, we get in QMP:

   { "execute": "pci_add",
     "arguments": { "pci_addr": "auto",
                    "type": "nic",
                    "opts": "model=e1000,macaddr=00:11:22:33:44:55" } }

which is silly.  Oops, we already did.

A more appropriate encoding would be

   { "execute": "pci_add",
     "arguments": { "pci_addr": "auto",
                    "type": "nic",
                    "model": "e1000",
                    "macaddr": "00:11:22:33:44:55" } }

or maybe

   { "execute": "pci_add",
     "arguments": { "pci_addr": "auto",
                    "type": "nic",
                    "opts": { "model": "e1000",
                              "macaddr": "00:11:22:33:44:55" } } }

Done right, this involves lifting the parsing of the parameter string
from the bowels of the handler up into the human monitor code.  Could be
somewhat messy.

Parsing should be done with qemu_opts_parse(), of course.  We need a way
to declare which QemuOptList to use.  Fortunately, we can look them up
by name.  We could create argument type 'O', followed by the name, with
some suitable delimiter, say parenthesis.  pci_add's args_type changes
from

        .args_type  = "pci_addr:s,type:s,opts:s?",

to

        .args_type  = "pci_addr:s,type:s,opts:O(net)",

Opinions?

Re: [Qemu-devel] Converting existing name=value,... arguments to QMP

[Daniel P. Berrange]

On Thu, Feb 04, 2010 at 06:43:22PM +0100, Markus Armbruster wrote:
> The human monitor uses positional arguments.  This is fine; nobody wants
> to type "info item=network" instead of "info network".
> 
> QMP uses named arguments.  Also fine.
> 
> Internally, we use named arguments: we pass them as QDict to the command
> handler.  This involves associating positional arguments with names.
> Works fine.
> 
> There's one little mess hiding in a corner, though: a few commands abuse
> a positional argument to get named arguments.  That argument has the
> form of a "parameter string": NAME=VALUE,...  For instance:
> 
>     pci_add auto nic model=e1000,macaddr=00:11:22:33:44:55
> 
> If we convert this naively, we get in QMP:
> 
>    { "execute": "pci_add",
>      "arguments": { "pci_addr": "auto",
>                     "type": "nic",
>                     "opts": "model=e1000,macaddr=00:11:22:33:44:55" } }
> 
> which is silly.  Oops, we already did.

This isn't so silly, because it allows the mgmt apps to re-use the
exact same code for generating the 'opts' string here that they 
already have written to generate the initial '-net' argument value
when launching QEMU. That is a very big benefit that I would not 
like to loose

> A more appropriate encoding would be
> 
>    { "execute": "pci_add",
>      "arguments": { "pci_addr": "auto",
>                     "type": "nic",
>                     "model": "e1000",
>                     "macaddr": "00:11:22:33:44:55" } }
> 
> or maybe
> 
>    { "execute": "pci_add",
>      "arguments": { "pci_addr": "auto",
>                     "type": "nic",
>                     "opts": { "model": "e1000",
>                               "macaddr": "00:11:22:33:44:55" } } }

Both these approaches mean extra code for client apps, by not allowing
re-use of the ARGV argument generators they must already implement.
That said I can see the value in allowing the "fully normalized" format,
so if going for the second option, then I'd say that QEMU should use its
existing key,value pair parser to automagically convert this:

>                     "opts": "model=e1000,macaddr=00:11:22:33:44:55" } }

into this

>                     "opts": { "model": "e1000",
>                               "macaddr": "00:11:22:33:44:55" } } }

in case where it saw a JSON string, instead of hash.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|