[Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Anthony Liguori]

As I've been waiting for QAPI review, I've been working on the design of 
a new mechanism to replace our current command line option handling 
(QemuOpts) with something that reuses the QAPI infrastructure.

The 'QemuOpts' syntax is just a way to encode complex data structures.  
'nic,model=virtio,macaddress=00:01:02:03:04:05' can be mapped directly 
to a C data structure.  This is exactly what QCFG does using the same 
JSON schema mechanism that QMP uses.

The effect is that you describe a command line argument in JSON like so:

{ 'type': 'VncConfig',
   'data': { 'address': 'str', '*password': 'bool', '*reverse': 'bool',
             '*no-lock-key-sync': 'bool', '*sasl': 'bool', '*tls': 'bool',
             '*x509': 'str', '*x509verify': 'str', '*acl': 'bool',
             '*lossy': 'bool' } }


You then just implement a C function that gets called for each -vnc 
option specified:

void qcfg_handle_vnc(VncConfig *option, Error **errp)
{
}

And that's it.  You can squirrel away the option such that they all can 
be processed later, you can perform additional validation and return an 
error, or you can implement the appropriate logic.

The VncConfig structure is a proper C data structure.  The advantages of 
this approach compared to QemuOpts are similar to QAPI:

1) Strong typing means less bugs with lack of command line validation.  
In many cases, a bad command line results in a SEGV today.

2) Every option is formally specified and documented in a way that is 
both rigorous and machine readable.  This means we can generate high 
quality documentation in a variety of formats.

3) The command line parameters support full introspection.  This should 
provide the same functionality as Dan's earlier introspection patches.

4) The 'VncConfig' structure also has JSON marshallers and the 
qcfg_handle_vnc() function can be trivially bridged to QMP.  This means 
command line oriented interfaces (like device_add) are better integrated 
with QMP.

5) Very complex data types can be implemented.  We had some discussion 
of supporting nested structures with -blockdev.  This wouldn't work with 
QemuOpts but I've already implemented it with QCFG (blockdev syntax is 
my test case right now).  The syntax I'm currently using is -blockdev 
cache=none,id=foo,format.qcow.protocol.nbd.hostname=localhost where '.' 
is used to reference sub structures.

6) Unions are supported which means complex options like -net can be 
specified in the schema and don't require hand validation.

I've got a spec written up at http://wiki.qemu.org/Features/QCFG.  
Initial code is in my QAPI tree.

I'm not going to start converting things until we get closer to the end 
of 0.15 and QAPI is fully merged.  My plan is to focus on this for 0.16 
and do a full conversion for the 0.16 time frame using the same approach 
as QAPI.  That means that for 0.16, we would be able to set all command 
line options via QMP in a programmatic fashion with full support for 
introspection.

I'm haven't yet closed on how to bridge this to qdev.  qdev is a big 
consumer of QemuOpts today.  I have some general ideas about what I'd 
like to do but so far, I haven't written anything down.

I wanted to share these plans early hoping to get some feedback and also 
to maybe interest some folks in helping out.

Regards,

Anthony Liguori

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Lluís]

Anthony Liguori writes:

> I've got a spec written up at http://wiki.qemu.org/Features/QCFG.  Initial code
> is in my QAPI tree.

What about moving the documentation to a 'doc' attribute?

Thus, instead of the example vnconfig:

{ 'type': 'VncConfig',
  'doc': 'Configuration options for the built-in VNC server.',
  'data': {'address': 'str',
           ...} }

Still, it's not clear to me how attribute documentation shoul dbe
provided:

  'data': {'address': {'type': 'str',
                       'doc': 'The hostname to bind the VNC server to...'},
            ...
          }

Or maybe:

  'data': {'address': 'str',
           'address.doc': 'The hostname to bind the VNC server to...'},
            ...
          }

But as I suppose these documentation comments are automatically
processes, this might just prove too verbose for no benefit at all, as
introspecting down to the documentation might be already doable with the
format on the example.

You could also have a 'since' attribute, in case dynamic interface
checks are necessary (e.g., the "Since: 0.14.0" in the example).


Lluis

--
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Anthony Liguori]

On 03/14/2011 02:52 PM, Lluís wrote:
> Anthony Liguori writes:
>
>> I've got a spec written up at http://wiki.qemu.org/Features/QCFG.  Initial code
>> is in my QAPI tree.
> What about moving the documentation to a 'doc' attribute?
>
> Thus, instead of the example vnconfig:
>
> { 'type': 'VncConfig',
>    'doc': 'Configuration options for the built-in VNC server.',
>    'data': {'address': 'str',
>             ...} }
>
> Still, it's not clear to me how attribute documentation shoul dbe
> provided:
>
>    'data': {'address': {'type': 'str',
>                         'doc': 'The hostname to bind the VNC server to...'},
>              ...
>            }
>
> Or maybe:
>
>    'data': {'address': 'str',
>             'address.doc': 'The hostname to bind the VNC server to...'},
>              ...
>            }
>
> But as I suppose these documentation comments are automatically
> processes, this might just prove too verbose for no benefit at all, as
> introspecting down to the documentation might be already doable with the
> format on the example.

Exactly.  This is the intention--to have the documentation be just as 
well structured as the JSON.

> You could also have a 'since' attribute, in case dynamic interface
> checks are necessary (e.g., the "Since: 0.14.0" in the example).

Indeed.

Regards,

Anthony Liguori

> Lluis
>
> --
>   "And it's much the same thing with knowledge, for whenever you learn
>   something new, the whole world becomes that much richer."
>   -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
>   Tollbooth
>

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Kevin Wolf]

Am 14.03.2011 18:48, schrieb Anthony Liguori:
> As I've been waiting for QAPI review, I've been working on the design of 
> a new mechanism to replace our current command line option handling 
> (QemuOpts) with something that reuses the QAPI infrastructure.
> 
> The 'QemuOpts' syntax is just a way to encode complex data structures.  
> 'nic,model=virtio,macaddress=00:01:02:03:04:05' can be mapped directly 
> to a C data structure.  This is exactly what QCFG does using the same 
> JSON schema mechanism that QMP uses.
> 
> The effect is that you describe a command line argument in JSON like so:
> 
> { 'type': 'VncConfig',
>    'data': { 'address': 'str', '*password': 'bool', '*reverse': 'bool',
>              '*no-lock-key-sync': 'bool', '*sasl': 'bool', '*tls': 'bool',
>              '*x509': 'str', '*x509verify': 'str', '*acl': 'bool',
>              '*lossy': 'bool' } }
> 
> 
> You then just implement a C function that gets called for each -vnc 
> option specified:
> 
> void qcfg_handle_vnc(VncConfig *option, Error **errp)
> {
> }
> 
> And that's it.  You can squirrel away the option such that they all can 
> be processed later, you can perform additional validation and return an 
> error, or you can implement the appropriate logic.
> 
> The VncConfig structure is a proper C data structure.  The advantages of 
> this approach compared to QemuOpts are similar to QAPI:
> 
> 1) Strong typing means less bugs with lack of command line validation.  
> In many cases, a bad command line results in a SEGV today.
> 
> 2) Every option is formally specified and documented in a way that is 
> both rigorous and machine readable.  This means we can generate high 
> quality documentation in a variety of formats.
> 
> 3) The command line parameters support full introspection.  This should 
> provide the same functionality as Dan's earlier introspection patches.
> 
> 4) The 'VncConfig' structure also has JSON marshallers and the 
> qcfg_handle_vnc() function can be trivially bridged to QMP.  This means 
> command line oriented interfaces (like device_add) are better integrated 
> with QMP.
> 
> 5) Very complex data types can be implemented.  We had some discussion 
> of supporting nested structures with -blockdev.  This wouldn't work with 
> QemuOpts but I've already implemented it with QCFG (blockdev syntax is 
> my test case right now).  The syntax I'm currently using is -blockdev 
> cache=none,id=foo,format.qcow.protocol.nbd.hostname=localhost where '.' 
> is used to reference sub structures.

Do you have an example from your implementation for this?

I think the tricky part is that the valid fields depend on the block
driver. qcow2 wants another BlockDriverState as its image file; file
wants a file name; vvfat wants a directory name, FAT type and disk type;
and NBD wants a host name and a port, except if it uses a UNIX socket.

This is probably the most complex thing you can get, so I think it would
make a better example than a VNC configuration.

Kevin

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Anthony Liguori]

On 03/15/2011 05:09 AM, Kevin Wolf wrote:
>> 5) Very complex data types can be implemented.  We had some discussion
>> of supporting nested structures with -blockdev.  This wouldn't work with
>> QemuOpts but I've already implemented it with QCFG (blockdev syntax is
>> my test case right now).  The syntax I'm currently using is -blockdev
>> cache=none,id=foo,format.qcow.protocol.nbd.hostname=localhost where '.'
>> is used to reference sub structures.
> Do you have an example from your implementation for this?

It's not exhaustive as I'm only using this for testing but here's what 
I've been working with:

{ 'type': 'ProbeProtocol', 'data': { 'unsafe': 'bool', 'filename': 'str' } }

{ 'type': 'FileProtocol', 'data': { 'filename': 'str' } }

{ 'type': 'HostDeviceProtocol', 'data': { 'device': 'str' } }

{ 'type': 'NbdProtocol', 'data': { 'hostname': 'str', 'port': 'int' } }

{ 'union': 'BlockdevProtocol',
   'data': { 'probe': 'ProbeProtocol', 'file': 'FileProtocol',
             'host-dev': 'HostDeviceProtocol', 'nbd': 'NbdProtocol' } }

{ 'type': 'ProbeFormat', 'data': { '*unsafe': 'bool', 'protocol': 
'BlockdevProtocol' } }

{ 'type': 'RawFormat', 'data': { 'protocol': 'BlockdevProtocol' } }

{ 'type': 'Qcow2Format',
   'data': { 'protocol': 'BlockdevProtocol',
             '*backing-file': 'BlockdevFormat' } }

{ 'type': 'QedFormat',
   'data': { 'protocol': 'BlockdevProtocol',
             '*backing-file': 'BlockdevFormat',
             '*copy-on-read': 'bool' } }

{ 'union': 'BlockdevFormat',
   'data': { 'probe': 'ProbeFormat', 'raw': 'RawFormat',
             'qcow2': 'Qcow2Format', 'qed': 'QedFormat' } }

{ 'enum': 'BlockdevCacheSetting',
   'data': [ 'none', 'writethrough', 'writeback' ] }

{ 'type': 'BlockdevConfig',
   'data': { 'id': 'str',
             'format': 'BlockdevFormat',
             '*cache': 'BlockdevCacheSetting',
             '*device': 'str' } }

{ 'option': 'blockdev', 'data': 'BlockdevConfig', 'implicit': 'id' }

Choosing a union is implicit in selecting the union value.  This was 
done to simplify the command line.  Here are some examples:

# create a blockdev using probing
-blockdev my-image.qcow2,id=ide0-hd0

# create a blockdev using probing without relying on implicit keys and 
allowing unsafe probing
-blockdev 
format.probe.unsafe=on,format.probe.protocol.file.filename=my-image.qcow2,id=ide0-hd0

# create a blockdev using qcow2 over NBD with a qed backing file
-blockdev format.qcow2.protocol.nbd={hostname=localhost,port=1025},\
                 
format.qcow2.backing-file.format.qed.protocol.nbd={hostname=localhost,port=1026},\
                 id=ide0-hd0

It looks less awkward in config file format:

[blockdev]
id = "ide0-hd0"
format.qcow2.protocol.nbd.hostname = localhost
format.qcow2.protocol.nbd.port = 1025
format.qcow2.backing-file.format.qed.protocol.nbd.hostname = localhost
format.qcow2.backing-file.format.qed.protocol.nbd.port = 1026

And with a syntax this complex, errors are important.  Here are some 
examples of Error messages:

#./test-qcfg format.qcow2.file.filename="image.img",id=ide0-hd0
-blockdev: Parameter 'format.qcow2.protocol' is missing

# ./test-qcfg 
format.qcow2.protocol.file.filename="image.img",format.qcow3.backing-file.format.qcow2.protocol.file.filename="foo.img",id=ide0-hd0
-blockdev: Invalid parameter 
'format.qcow3.backing-file.format.qcow2.protocol.file.filename'

#./test-qcfg 
format.qcow2.protocol.file.filename="image.img",id=ide0-hd0,cache=no-thank-you
-blockdev: Enum 'cache' with value 'no-thank-you' is invalid for type 
'BlockdevCacheSetting'

> I think the tricky part is that the valid fields depend on the block
> driver. qcow2 wants another BlockDriverState as its image file; file
> wants a file name; vvfat wants a directory name, FAT type and disk type;
> and NBD wants a host name and a port, except if it uses a UNIX socket.

Yes, it's all handled with a new union type.

> This is probably the most complex thing you can get, so I think it would
> make a better example than a VNC configuration.

Yup, that's been what I've been using to prototype all of this.  I 
didn't it in the mail because it's rather complex :-)

Regards,

Anthony Liguori

> Kevin
>

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Kevin Wolf]

Am 15.03.2011 14:27, schrieb Anthony Liguori:
> On 03/15/2011 05:09 AM, Kevin Wolf wrote:
>>> 5) Very complex data types can be implemented.  We had some discussion
>>> of supporting nested structures with -blockdev.  This wouldn't work with
>>> QemuOpts but I've already implemented it with QCFG (blockdev syntax is
>>> my test case right now).  The syntax I'm currently using is -blockdev
>>> cache=none,id=foo,format.qcow.protocol.nbd.hostname=localhost where '.'
>>> is used to reference sub structures.
>> Do you have an example from your implementation for this?
> 
> It's not exhaustive as I'm only using this for testing but here's what 
> I've been working with:
> 
> { 'type': 'ProbeProtocol', 'data': { 'unsafe': 'bool', 'filename': 'str' } }
> 
> { 'type': 'FileProtocol', 'data': { 'filename': 'str' } }
> 
> { 'type': 'HostDeviceProtocol', 'data': { 'device': 'str' } }
> 
> { 'type': 'NbdProtocol', 'data': { 'hostname': 'str', 'port': 'int' } }
> 
> { 'union': 'BlockdevProtocol',
>    'data': { 'probe': 'ProbeProtocol', 'file': 'FileProtocol',
>              'host-dev': 'HostDeviceProtocol', 'nbd': 'NbdProtocol' } }

What would this look like in the generated C code? A union of
differently typed pointers?

Are format drivers still contained in a single C file in block/ that is
enabled just by compiling it in or does the block layer now have to know
about all available drivers and the options they provide?

>> This is probably the most complex thing you can get, so I think it would
>> make a better example than a VNC configuration.
> 
> Yup, that's been what I've been using to prototype all of this.  I 
> didn't it in the mail because it's rather complex :-)

This is exactly what makes it interesting. :-)

Kevin

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Anthony Liguori]

On 03/15/2011 08:45 AM, Kevin Wolf wrote:
> Am 15.03.2011 14:27, schrieb Anthony Liguori:
>> On 03/15/2011 05:09 AM, Kevin Wolf wrote:
>>>> 5) Very complex data types can be implemented.  We had some discussion
>>>> of supporting nested structures with -blockdev.  This wouldn't work with
>>>> QemuOpts but I've already implemented it with QCFG (blockdev syntax is
>>>> my test case right now).  The syntax I'm currently using is -blockdev
>>>> cache=none,id=foo,format.qcow.protocol.nbd.hostname=localhost where '.'
>>>> is used to reference sub structures.
>>> Do you have an example from your implementation for this?
>> It's not exhaustive as I'm only using this for testing but here's what
>> I've been working with:
>>
>> { 'type': 'ProbeProtocol', 'data': { 'unsafe': 'bool', 'filename': 'str' } }
>>
>> { 'type': 'FileProtocol', 'data': { 'filename': 'str' } }
>>
>> { 'type': 'HostDeviceProtocol', 'data': { 'device': 'str' } }
>>
>> { 'type': 'NbdProtocol', 'data': { 'hostname': 'str', 'port': 'int' } }
>>
>> { 'union': 'BlockdevProtocol',
>>     'data': { 'probe': 'ProbeProtocol', 'file': 'FileProtocol',
>>               'host-dev': 'HostDeviceProtocol', 'nbd': 'NbdProtocol' } }
> What would this look like in the generated C code? A union of
> differently typed pointers?

Yes:

typedef enum BlockdevFormatKind {
     BFK_PROBE = 0,
     BFK_RAW = 1,
     BFK_QCOW2 = 2,
     BFK_QED = 3,
} BlockdevFormatKind;

typedef struct BlockdevFormat {
     BlockdevFormatKind kind;
     union {
         struct ProbeFormat * probe;
         struct RawFormat * raw;
         struct Qcow2Format * qcow2;
         struct QedFormat * qed;
     };
     struct BlockdevFormat * next;
} BlockdevFormat;

> Are format drivers still contained in a single C file in block/ that is
> enabled just by compiling it in or does the block layer now have to know
> about all available drivers and the options they provide?

Yes, everything is contained within a single file.  In terms of build 
dependencies, it's really just a call about what matters to you.  You 
can have the block open take a BlockdevFormat which means the block 
layer doesn't need to know about specific formats.

Regards,

Anthony Liguori

>>> This is probably the most complex thing you can get, so I think it would
>>> make a better example than a VNC configuration.
>> Yup, that's been what I've been using to prototype all of this.  I
>> didn't it in the mail because it's rather complex :-)
> This is exactly what makes it interesting. :-)
>
> Kevin
>

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Stefan Hajnoczi]

On Tue, Mar 15, 2011 at 1:27 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> # create a blockdev using probing without relying on implicit keys and
> allowing unsafe probing
> -blockdev
> format.probe.unsafe=on,format.probe.protocol.file.filename=my-image.qcow2,id=ide0-hd0

This is a programmer's user interface :).  I don't think typing
format.probe.protocol.file.filename will make sense to users.  I don't
want to have to learn about 4 sub-objects in order to set the image
filename.

I would much prefer:
-blockdev filename=my-image.qcow2,id=id0-hd0,format=probe

> It looks less awkward in config file format:
>
> [blockdev]
> id = "ide0-hd0"
> format.qcow2.protocol.nbd.hostname = localhost
> format.qcow2.protocol.nbd.port = 1025
> format.qcow2.backing-file.format.qed.protocol.nbd.hostname = localhost
> format.qcow2.backing-file.format.qed.protocol.nbd.port = 1026

Here is the alternative if we don't support composition:

[blockdev]
id = "nbd-1025"
protocol = "nbd"
hostname = "localhost"
port = 1025

[blockdev]
id = "nbd-1026"
protocol = "nbd"
hostname = "localhost"
port = 1026

[blockdev]
id = "qed-nbd-1026"
format = "qed"
image = "nbd-1026"

[blockdev]
id = "ide0-hd0"
format = "qcow2"
image = "nbd-1025"
backing_file = "qed-nbd-1026"

I like the simplicity but it becomes so verbose that it is hard to see
what is going on.  Actually I think it's INI syntax's fault:

blockdev id=nbd-1025,protocol=nbd,hostname=localhost,port=1025
blockdev id=nbd-1026,protocol=nbd,hostname=localhost,port=1026
blockdev id=qed-nbd-1026,format=qed,image=nbd-1026
blockdev id=ide0-hd0,format=qcow2,image=nbd-1025,backing_file=qed-nbd-1026

Perhaps the best compromise is to keep composition but implement
shortcuts as suggested by Markus.

Stefan

[Qemu-devel] Re: [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Kevin Wolf]

Am 14.03.2011 18:48, schrieb Anthony Liguori:
> I've got a spec written up at http://wiki.qemu.org/Features/QCFG.  
> Initial code is in my QAPI tree.

One question about a small detail on this wiki page:

> typedef struct BlockdevConfig {
>     char * file;
>     struct BlockdevConfig * backing_file;
> 
>     struct BlockdevConfig * next;
> } BlockdevConfig;

What is the 'next' pointer used for, are you going to store a list of
all -blockdev options used? And why isn't it a QLIST or something?

Kevin

Re: [Qemu-devel] Re: [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Anthony Liguori]

On 03/15/2011 06:21 AM, Kevin Wolf wrote:
> Am 14.03.2011 18:48, schrieb Anthony Liguori:
>> I've got a spec written up at http://wiki.qemu.org/Features/QCFG.
>> Initial code is in my QAPI tree.
> One question about a small detail on this wiki page:
>
>> typedef struct BlockdevConfig {
>>      char * file;
>>      struct BlockdevConfig * backing_file;
>>
>>      struct BlockdevConfig * next;
>> } BlockdevConfig;
> What is the 'next' pointer used for,

This is a standard part of QAPI.  All types get a next pointer added 
such that we can support lists of complex types.

>   are you going to store a list of
> all -blockdev options used? And why isn't it a QLIST or something?

Two reasons.  QLIST requires another type for the head of the list which 
would complicate things overall.  Second is that these types are part of 
the libqmp interface and I didn't want to force qemu-queue on any 
consumer of libqmp.

Regards,

Anthony Liguori

> Kevin
>

Re: [Qemu-devel] Re: [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Kevin Wolf]

Am 15.03.2011 14:37, schrieb Anthony Liguori:
> On 03/15/2011 06:21 AM, Kevin Wolf wrote:
>> Am 14.03.2011 18:48, schrieb Anthony Liguori:
>>> I've got a spec written up at http://wiki.qemu.org/Features/QCFG.
>>> Initial code is in my QAPI tree.
>> One question about a small detail on this wiki page:
>>
>>> typedef struct BlockdevConfig {
>>>      char * file;
>>>      struct BlockdevConfig * backing_file;
>>>
>>>      struct BlockdevConfig * next;
>>> } BlockdevConfig;
>> What is the 'next' pointer used for,
> 
> This is a standard part of QAPI.  All types get a next pointer added 
> such that we can support lists of complex types.

Only a single list for each object.

>>   are you going to store a list of
>> all -blockdev options used? And why isn't it a QLIST or something?
> 
> Two reasons.  QLIST requires another type for the head of the list which 
> would complicate things overall.  Second is that these types are part of 
> the libqmp interface and I didn't want to force qemu-queue on any 
> consumer of libqmp.

And now you force existing qemu code to go back to open coded lists,
which is arguably a step backwards. I don't think this is any better
than forcing the (non-existent) users of libqmp to include one
additional header file.

Kevin

Re: [Qemu-devel] Re: [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 15.03.2011 14:37, schrieb Anthony Liguori:
>> On 03/15/2011 06:21 AM, Kevin Wolf wrote:
>>> Am 14.03.2011 18:48, schrieb Anthony Liguori:
>>>> I've got a spec written up at http://wiki.qemu.org/Features/QCFG.
>>>> Initial code is in my QAPI tree.
>>> One question about a small detail on this wiki page:
>>>
>>>> typedef struct BlockdevConfig {
>>>>      char * file;
>>>>      struct BlockdevConfig * backing_file;
>>>>
>>>>      struct BlockdevConfig * next;
>>>> } BlockdevConfig;
>>> What is the 'next' pointer used for,
>> 
>> This is a standard part of QAPI.  All types get a next pointer added 
>> such that we can support lists of complex types.
>
> Only a single list for each object.

Don't even think of trees.  Yuck.

>>>   are you going to store a list of
>>> all -blockdev options used? And why isn't it a QLIST or something?
>> 
>> Two reasons.  QLIST requires another type for the head of the list which 
>> would complicate things overall.  Second is that these types are part of 
>> the libqmp interface and I didn't want to force qemu-queue on any 
>> consumer of libqmp.
>
> And now you force existing qemu code to go back to open coded lists,
> which is arguably a step backwards. I don't think this is any better
> than forcing the (non-existent) users of libqmp to include one
> additional header file.

Agree.  Let's not screw up our internal interfaces for the sake of an
external interface that may or may not turn out to be viable.

Re: [Qemu-devel] Re: [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Anthony Liguori]

On 03/17/2011 10:26 AM, Markus Armbruster wrote:
> Kevin Wolf<kwolf@redhat.com>  writes:
>
>> Am 15.03.2011 14:37, schrieb Anthony Liguori:
>>> On 03/15/2011 06:21 AM, Kevin Wolf wrote:
>>>> Am 14.03.2011 18:48, schrieb Anthony Liguori:
>>>>> I've got a spec written up at http://wiki.qemu.org/Features/QCFG.
>>>>> Initial code is in my QAPI tree.
>>>> One question about a small detail on this wiki page:
>>>>
>>>>> typedef struct BlockdevConfig {
>>>>>       char * file;
>>>>>       struct BlockdevConfig * backing_file;
>>>>>
>>>>>       struct BlockdevConfig * next;
>>>>> } BlockdevConfig;
>>>> What is the 'next' pointer used for,
>>> This is a standard part of QAPI.  All types get a next pointer added
>>> such that we can support lists of complex types.
>> Only a single list for each object.
> Don't even think of trees.  Yuck.

Sorry, don't fully understand.  The above data structure is a tree.

I haven't looked yet at converting the code generator to use the 
qemu-queue structures.  I'm not sure I agree it's the right thing to do 
but I don't think it's all that hard.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 03/17/2011 10:26 AM, Markus Armbruster wrote:
>> Kevin Wolf<kwolf@redhat.com>  writes:
>>
>>> Am 15.03.2011 14:37, schrieb Anthony Liguori:
>>>> On 03/15/2011 06:21 AM, Kevin Wolf wrote:
>>>>> Am 14.03.2011 18:48, schrieb Anthony Liguori:
>>>>>> I've got a spec written up at http://wiki.qemu.org/Features/QCFG.
>>>>>> Initial code is in my QAPI tree.
>>>>> One question about a small detail on this wiki page:
>>>>>
>>>>>> typedef struct BlockdevConfig {
>>>>>>       char * file;
>>>>>>       struct BlockdevConfig * backing_file;
>>>>>>
>>>>>>       struct BlockdevConfig * next;
>>>>>> } BlockdevConfig;
>>>>> What is the 'next' pointer used for,
>>>> This is a standard part of QAPI.  All types get a next pointer added
>>>> such that we can support lists of complex types.
>>> Only a single list for each object.
>> Don't even think of trees.  Yuck.
>
> Sorry, don't fully understand.  The above data structure is a tree.

The next pointer that is "a standard part of QAPI" provides for a single
list membership, no more.  The criticism isn't that you can't build
trees in QAPI (you can).  It's that there's one special list.  Doesn't
feel right for a general purpose interface.  But it really depends on
how it's used.  Anyway, this belongs into the QAPI thread, not here.

> I haven't looked yet at converting the code generator to use the
> qemu-queue structures.  I'm not sure I agree it's the right thing to
> do but I don't think it's all that hard.

Depends on how the list is used.  If all uses are known (say because
they're all behind the interface), and limited to trivial pointer
chasing, then qemu-queue.h may well be more trouble than it's worth.

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> As I've been waiting for QAPI review, I've been working on the design
> of a new mechanism to replace our current command line option handling
> (QemuOpts) with something that reuses the QAPI infrastructure.

I'm ignoring the connection to QAPI, because I'm still ignorant there.

> The 'QemuOpts' syntax is just a way to encode complex data structures.
> nic,model=virtio,macaddress=00:01:02:03:04:05' can be mapped directly
> to a C data structure.  This is exactly what QCFG does using the same
> JSON schema mechanism that QMP uses.
>
> The effect is that you describe a command line argument in JSON like so:
>
> { 'type': 'VncConfig',
>   'data': { 'address': 'str', '*password': 'bool', '*reverse': 'bool',
>             '*no-lock-key-sync': 'bool', '*sasl': 'bool', '*tls': 'bool',
>             '*x509': 'str', '*x509verify': 'str', '*acl': 'bool',
>             '*lossy': 'bool' } }
>
>
> You then just implement a C function that gets called for each -vnc
> option specified:
>
> void qcfg_handle_vnc(VncConfig *option, Error **errp)
> {
> }
>
> And that's it.  You can squirrel away the option such that they all
> can be processed later, you can perform additional validation and
> return an error, or you can implement the appropriate logic.
>
> The VncConfig structure is a proper C data structure.  The advantages
> of this approach compared to QemuOpts are similar to QAPI:
>
> 1) Strong typing means less bugs with lack of command line validation.
> In many cases, a bad command line results in a SEGV today.

Static typing, you mean.

> 2) Every option is formally specified and documented in a way that is
> both rigorous and machine readable.  This means we can generate high
> quality documentation in a variety of formats.

You make it sound like QemuOpts wouldn't specify options.  It does.
Where your proposal differs from QemuOpts, in my opinion, is that it
uses C types rather than QemuOpts' very own ad hoc type system, and is
more expressive, see your 6) below.

> 3) The command line parameters support full introspection.  This
> should provide the same functionality as Dan's earlier introspection
> patches.

Again, this isn't something QemuOpts could not do.  We just neglected to
make its option specification available outside QEMU, mostly because for
lack of consensus on what we want to expose there.

> 4) The 'VncConfig' structure also has JSON marshallers and the
> qcfg_handle_vnc() function can be trivially bridged to QMP.  This
> means command line oriented interfaces (like device_add) are better
> integrated with QMP.

Yes.

> 5) Very complex data types can be implemented.  We had some discussion
> of supporting nested structures with -blockdev.  This wouldn't work
> with QemuOpts but I've already implemented it with QCFG (blockdev
> syntax is my test case right now).  The syntax I'm currently using is
> -blockdev
> cache=none,id=foo,format.qcow.protocol.nbd.hostname=localhost where
> .' is used to reference sub structures.

We might want some syntactic sugar so that users don't have to repeat
format.qcow.protocol.nbd.FOO for every FOO they want to configure.

> 6) Unions are supported which means complex options like -net can be
> specified in the schema and don't require hand validation.
>
> I've got a spec written up at http://wiki.qemu.org/Features/QCFG.

Not reachable right now.

> Initial code is in my QAPI tree.
>
> I'm not going to start converting things until we get closer to the
> end of 0.15 and QAPI is fully merged.  My plan is to focus on this for
> 0.16 and do a full conversion for the 0.16 time frame using the same
> approach as QAPI.  That means that for 0.16, we would be able to set
> all command line options via QMP in a programmatic fashion with full
> support for introspection.
>
> I'm haven't yet closed on how to bridge this to qdev.  qdev is a big
> consumer of QemuOpts today.  I have some general ideas about what I'd
> like to do but so far, I haven't written anything down.

Glad you mention qdev.

qdev properties describe the configurable parts of qdev state objects.
A state object is a C struct.  The description is C data.  Poor man's
reflection.

qdev needs to parse and print a device's properties.  It uses QemuOpts
to parse NAME=VALUE,... option strings into a list of (NAME, VALUE), and
callbacks to parse the VALUEs.  Awkward, especially when you go QMP ->
option string -> qdev struct.

Yet another one: vmstate, which describes migratable parts of qdev state
objects.

Unlike these two, QCFG doesn't describe C structs, it generates them
from JSON specs.  If I understand your proposal correctly.  Hmm.

Can we avoid defining our data types in JSON rather than C?

Can we adopt *one* reflection mechanism?

> I wanted to share these plans early hoping to get some feedback and
> also to maybe interest some folks in helping out.

There's also QEMUOptionParameter, thankfully limited to block code.  Any
plans to cover that as well?


Hmm, my nitpicking and questioning may sound like I don't like the idea
of replacing QemuOpts.  That's actually not the case.

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Anthony Liguori]

On 03/17/2011 10:22 AM, Markus Armbruster wrote:
>
>> void qcfg_handle_vnc(VncConfig *option, Error **errp)
>> {
>> }
>>
>> And that's it.  You can squirrel away the option such that they all
>> can be processed later, you can perform additional validation and
>> return an error, or you can implement the appropriate logic.
>>
>> The VncConfig structure is a proper C data structure.  The advantages
>> of this approach compared to QemuOpts are similar to QAPI:
>>
>> 1) Strong typing means less bugs with lack of command line validation.
>> In many cases, a bad command line results in a SEGV today.
> Static typing, you mean.

Both.  We don't consistently check error returns when dealing with 
QemuOpts so while it does implement dynamic typing, the dynamic typing 
is not consistently enforced.

>> 2) Every option is formally specified and documented in a way that is
>> both rigorous and machine readable.  This means we can generate high
>> quality documentation in a variety of formats.
> You make it sound like QemuOpts wouldn't specify options.  It does.
> Where your proposal differs from QemuOpts, in my opinion, is that it
> uses C types rather than QemuOpts' very own ad hoc type system, and is
> more expressive, see your 6) below.

My point in that bullet is that the documentation is machine readable 
and highly structured.

>> 3) The command line parameters support full introspection.  This
>> should provide the same functionality as Dan's earlier introspection
>> patches.
> Again, this isn't something QemuOpts could not do.  We just neglected to
> make its option specification available outside QEMU, mostly because for
> lack of consensus on what we want to expose there.

It's all just a matter of bits, right :-)

But QemuOpts is just about parsing a string, it doesn't really 
explicitly tie to a command line.  Another layer would be needed to 
clearly map that option foo is associated with this QemuOpts.

So yeah, it could grow in that direction but there's a lot more to it 
then just returning a list of names and ties given a qemu opts definition.


>> 5) Very complex data types can be implemented.  We had some discussion
>> of supporting nested structures with -blockdev.  This wouldn't work
>> with QemuOpts but I've already implemented it with QCFG (blockdev
>> syntax is my test case right now).  The syntax I'm currently using is
>> -blockdev
>> cache=none,id=foo,format.qcow.protocol.nbd.hostname=localhost where
>> .' is used to reference sub structures.
> We might want some syntactic sugar so that users don't have to repeat
> format.qcow.protocol.nbd.FOO for every FOO they want to configure.

-set will still function in this model as key/value parsing and 
translation to a C structure are two separate steps.

But yeah, I think we ought to brain storm ways to simplify blockdev 
because the structure is very complex.  But that's a separate discussion.

>> Initial code is in my QAPI tree.
>>
>> I'm not going to start converting things until we get closer to the
>> end of 0.15 and QAPI is fully merged.  My plan is to focus on this for
>> 0.16 and do a full conversion for the 0.16 time frame using the same
>> approach as QAPI.  That means that for 0.16, we would be able to set
>> all command line options via QMP in a programmatic fashion with full
>> support for introspection.
>>
>> I'm haven't yet closed on how to bridge this to qdev.  qdev is a big
>> consumer of QemuOpts today.  I have some general ideas about what I'd
>> like to do but so far, I haven't written anything down.
> Glad you mention qdev.
>
> qdev properties describe the configurable parts of qdev state objects.
> A state object is a C struct.  The description is C data.  Poor man's
> reflection.

Right.  The problem is it's very hard to reflect C even if you parse it 
without additional annotations.  For instance:

typedef struct Foo {
     Bar *bar;
} Foo;

What the type of bar is is ambigious.  It could be a pointer to a list 
of Bar's (if bar has an embedded pointer), it could be an array of Bars 
that is terminated using a field within Bar, it could be a pointer to a 
fixed size array of Bars, or it could be just a pointer to a single Bar 
object.

So you end up needing additional annotations like:

typedef struct Foo {
    size_t n_bar;
    Bar *bar sizeis(n_bar);
} Foo;

This is what most IDLs that use C style syntax do.

> qdev needs to parse and print a device's properties.  It uses QemuOpts
> to parse NAME=VALUE,... option strings into a list of (NAME, VALUE), and
> callbacks to parse the VALUEs.  Awkward, especially when you go QMP ->
> option string ->  qdev struct.

Indeed.  You can do very weird things like pass a float through qdev and 
it appears as a (valid) string type with QemuOpts.

But qdev properties are a little odder semantically.  qdev properties 
are construct-only and read-only.   They are construct-only because they 
are set implicitly in the object before its created as a sort of default 
value.

It's easy to bridge QCFG/QMP to this but I've been thinking of going a 
step further.  I've been thinking of taking the current qdev properties 
and making them proper construction properties and removing the magic 
involved in setting their default value.  This would require the code 
generator to create a construction function that is called such as:

DeviceState *isa_serial_init(int iobase, int irq, CharDriverState *chr)
{
      ISADeviceState *dev = qdev_alloc(&isa_serial_desc);

      dev->iobase = iobase;
      dev->irq = irq;
      dev->chr = chr;
      // ...
}

There would be a separate interface for getting/setting properties.  It 
might even be something as simple as, you have to implement:

int isa_serial_get_iobase(ISADeviceState *dev);
int isa_serial_get_irq(ISADeviceState *dev);
...

This ends up being a powerful interface because you can easily get these 
properties within QEMU, but you also (transparently) map these to the 
wire.  It also extends really nicely for setting properties which 
becomes an interesting way to implement dynamic device logic (like 
setting the link status of a network card).

> Yet another one: vmstate, which describes migratable parts of qdev state
> objects.

Yes, for now, I'm ignoring vmstate.  To really solve vmstate, I think 
you need to describe the complete object instead of just it's properties.

> Unlike these two, QCFG doesn't describe C structs, it generates them
> from JSON specs.  If I understand your proposal correctly.  Hmm.

Correct.

> Can we avoid defining our data types in JSON rather than C?

I didn't start with describing them in JSON.  I started by describing 
them in Python with the idea that I'd write a IDL parser using a C style 
syntax that would then generate the Python structures.  Using the fixed 
Python data structures, I could play with code generation without all 
the trouble of writing a full blown IDL parser.

But I never got around to writing that parser and have actually become 
fond of the Python/JSON style syntax.

> Can we adopt *one* reflection mechanism?

Yes, I think we should start with that being the JSON syntax I'm using 
for QAPI/QCFG and then down the road, we can introduce a totally new 
syntax if need be.  But I think we need a central schema that describes 
all externally visible interfaces.  I think this is really the key idea 
here.

>> I wanted to share these plans early hoping to get some feedback and
>> also to maybe interest some folks in helping out.
> There's also QEMUOptionParameter, thankfully limited to block code.  Any
> plans to cover that as well?

QCFG will replace this because the BlockdevConfig structure can be 
passed throughout the block layer to enable block format specific 
options to be implemented.

QEMUOptionParameter is just used for bdrv_create today right?  Do we 
want create options to be different than run time options or would a 
superset of the two be appropriate?

Regards,

Anthony Liguori

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Kevin Wolf]

Am 17.03.2011 19:28, schrieb Anthony Liguori:
> On 03/17/2011 10:22 AM, Markus Armbruster wrote:
>>> I wanted to share these plans early hoping to get some feedback and
>>> also to maybe interest some folks in helping out.
>> There's also QEMUOptionParameter, thankfully limited to block code.  Any
>> plans to cover that as well?
> 
> QCFG will replace this because the BlockdevConfig structure can be 
> passed throughout the block layer to enable block format specific 
> options to be implemented.
> 
> QEMUOptionParameter is just used for bdrv_create today right?

I think so.

I created it for bdrv_create, and when Gerd needed something similar not
much later, it turned out that we wanted to add some more features. So
Gerd turned it into QemuOpts, but left the old interface instead of
replacing it in the existing users. Everything newer than that should be
using QemuOpts.

> Do we 
> want create options to be different than run time options or would a 
> superset of the two be appropriate?

Run time options would be things like cache mode etc.? I don't think we
have an image format that can store these, so it doesn't make a lot of
sense to provide them in bdrv_create.

For opening an image, I think we have three levels of options, in this
order: Defaults that are set by the block driver, options that are read
from an image, options specified on the command line. It seems to make
sense to allow this for any option.

Kevin

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 03/17/2011 10:22 AM, Markus Armbruster wrote:
>>
>>> void qcfg_handle_vnc(VncConfig *option, Error **errp)
>>> {
>>> }
>>>
>>> And that's it.  You can squirrel away the option such that they all
>>> can be processed later, you can perform additional validation and
>>> return an error, or you can implement the appropriate logic.
>>>
>>> The VncConfig structure is a proper C data structure.  The advantages
>>> of this approach compared to QemuOpts are similar to QAPI:
>>>
>>> 1) Strong typing means less bugs with lack of command line validation.
>>> In many cases, a bad command line results in a SEGV today.
>> Static typing, you mean.
>
> Both.  We don't consistently check error returns when dealing with
> QemuOpts so while it does implement dynamic typing, the dynamic typing
> is not consistently enforced.

Yes.  Dynamic typing in C takes a lot of discipline.

>>> 2) Every option is formally specified and documented in a way that is
>>> both rigorous and machine readable.  This means we can generate high
>>> quality documentation in a variety of formats.
>> You make it sound like QemuOpts wouldn't specify options.  It does.
>> Where your proposal differs from QemuOpts, in my opinion, is that it
>> uses C types rather than QemuOpts' very own ad hoc type system, and is
>> more expressive, see your 6) below.
>
> My point in that bullet is that the documentation is machine readable
> and highly structured.
>
>>> 3) The command line parameters support full introspection.  This
>>> should provide the same functionality as Dan's earlier introspection
>>> patches.
>> Again, this isn't something QemuOpts could not do.  We just neglected to
>> make its option specification available outside QEMU, mostly because for
>> lack of consensus on what we want to expose there.
>
> It's all just a matter of bits, right :-)
>
> But QemuOpts is just about parsing a string, it doesn't really
> explicitly tie to a command line.  Another layer would be needed to
> clearly map that option foo is associated with this QemuOpts.
>
> So yeah, it could grow in that direction but there's a lot more to it
> then just returning a list of names and ties given a qemu opts
> definition.
>
>
>>> 5) Very complex data types can be implemented.  We had some discussion
>>> of supporting nested structures with -blockdev.  This wouldn't work
>>> with QemuOpts but I've already implemented it with QCFG (blockdev
>>> syntax is my test case right now).  The syntax I'm currently using is
>>> -blockdev
>>> cache=none,id=foo,format.qcow.protocol.nbd.hostname=localhost where
>>> .' is used to reference sub structures.
>> We might want some syntactic sugar so that users don't have to repeat
>> format.qcow.protocol.nbd.FOO for every FOO they want to configure.
>
> -set will still function in this model as key/value parsing and
> translation to a C structure are two separate steps.
>
> But yeah, I think we ought to brain storm ways to simplify blockdev
> because the structure is very complex.  But that's a separate
> discussion.

Fair enough.

>>> Initial code is in my QAPI tree.
>>>
>>> I'm not going to start converting things until we get closer to the
>>> end of 0.15 and QAPI is fully merged.  My plan is to focus on this for
>>> 0.16 and do a full conversion for the 0.16 time frame using the same
>>> approach as QAPI.  That means that for 0.16, we would be able to set
>>> all command line options via QMP in a programmatic fashion with full
>>> support for introspection.
>>>
>>> I'm haven't yet closed on how to bridge this to qdev.  qdev is a big
>>> consumer of QemuOpts today.  I have some general ideas about what I'd
>>> like to do but so far, I haven't written anything down.
>> Glad you mention qdev.
>>
>> qdev properties describe the configurable parts of qdev state objects.
>> A state object is a C struct.  The description is C data.  Poor man's
>> reflection.
>
> Right.  The problem is it's very hard to reflect C even if you parse
> it without additional annotations.  For instance:
>
> typedef struct Foo {
>     Bar *bar;
> } Foo;
>
> What the type of bar is is ambigious.  It could be a pointer to a list
> of Bar's (if bar has an embedded pointer), it could be an array of
> Bars that is terminated using a field within Bar, it could be a
> pointer to a fixed size array of Bars, or it could be just a pointer
> to a single Bar object.
>
> So you end up needing additional annotations like:
>
> typedef struct Foo {
>    size_t n_bar;
>    Bar *bar sizeis(n_bar);
> } Foo;
>
> This is what most IDLs that use C style syntax do.

We currently use a more low-tech approach: define the struct in plain C,
and the data describing the struct in plain C as well.

Information about the type is in two places and in two formats (C type
declaration and C data intializer).  There's a bit of redundancy.
Ensuring consistency requires preprocessor hackery.

The data doesn't have to describe all struct members.  I'm inclined to
regard that as a feature.

Avoids generating source code.

>> qdev needs to parse and print a device's properties.  It uses QemuOpts
>> to parse NAME=VALUE,... option strings into a list of (NAME, VALUE), and
>> callbacks to parse the VALUEs.  Awkward, especially when you go QMP ->
>> option string ->  qdev struct.
>
> Indeed.  You can do very weird things like pass a float through qdev
> and it appears as a (valid) string type with QemuOpts.
>
> But qdev properties are a little odder semantically.  qdev properties
> are construct-only and read-only.   They are construct-only because
> they are set implicitly in the object before its created as a sort of
> default value.
>
> It's easy to bridge QCFG/QMP to this but I've been thinking of going a
> step further.  I've been thinking of taking the current qdev
> properties and making them proper construction properties and removing
> the magic involved in setting their default value.  This would require
> the code generator to create a construction function that is called
> such as:
>
> DeviceState *isa_serial_init(int iobase, int irq, CharDriverState *chr)
> {
>      ISADeviceState *dev = qdev_alloc(&isa_serial_desc);
>
>      dev->iobase = iobase;
>      dev->irq = irq;
>      dev->chr = chr;
>      // ...
> }
>
> There would be a separate interface for getting/setting properties.
> It might even be something as simple as, you have to implement:
>
> int isa_serial_get_iobase(ISADeviceState *dev);
> int isa_serial_get_irq(ISADeviceState *dev);
> ...

To be honest, I'm wary of generating lots of C source.  Prone to become
a pain to debug and change.  Generated code tends to be much worse than
data in that regard.

If you generate significant parts of your program with other programs,
chances are that you're not using the right language for the job.

> This ends up being a powerful interface because you can easily get
> these properties within QEMU, but you also (transparently) map these
> to the wire.  It also extends really nicely for setting properties
> which becomes an interesting way to implement dynamic device logic
> (like setting the link status of a network card).

Well, the traditional way to "get properties" within QEMU is the ol' ->
operator.

I'm not sure I get the rest of the paragraph.

>> Yet another one: vmstate, which describes migratable parts of qdev state
>> objects.
>
> Yes, for now, I'm ignoring vmstate.  To really solve vmstate, I think
> you need to describe the complete object instead of just it's
> properties.

Would be nice to have a solution that can cover all our reflection
needs, including vmstate.  But we need to watch out not to get bogged
down in the Generality Swamps.

>> Unlike these two, QCFG doesn't describe C structs, it generates them
>> from JSON specs.  If I understand your proposal correctly.  Hmm.
>
> Correct.
>
>> Can we avoid defining our data types in JSON rather than C?
>
> I didn't start with describing them in JSON.  I started by describing
> them in Python with the idea that I'd write a IDL parser using a C
> style syntax that would then generate the Python structures.  Using
> the fixed Python data structures, I could play with code generation
> without all the trouble of writing a full blown IDL parser.
>
> But I never got around to writing that parser and have actually become
> fond of the Python/JSON style syntax.

Well, let's say "fondness" isn't what I'd expect ordinary people to feel
when confonted with the idea to define half our structs in Python rather
than C ;)

>> Can we adopt *one* reflection mechanism?
>
> Yes, I think we should start with that being the JSON syntax I'm using
> for QAPI/QCFG and then down the road, we can introduce a totally new
> syntax if need be.  But I think we need a central schema that
> describes all externally visible interfaces.  I think this is really
> the key idea here.

Yes, self-describing external interfaces are desirable.

Speaking of external interfaces: we need to be careful when exposing
internal data types externally.  If we couple our external interfaces
too closely to internals, we risk calcifying our internal interfaces.

qdev has gotten that partly right: you can change the state structs
pretty freely without affecting the externally visible properties.

[...]

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Anthony Liguori]

On 03/18/2011 09:04 AM, Markus Armbruster wrote:
>>>> Initial code is in my QAPI tree.
>>>>
>>>> I'm not going to start converting things until we get closer to the
>>>> end of 0.15 and QAPI is fully merged.  My plan is to focus on this for
>>>> 0.16 and do a full conversion for the 0.16 time frame using the same
>>>> approach as QAPI.  That means that for 0.16, we would be able to set
>>>> all command line options via QMP in a programmatic fashion with full
>>>> support for introspection.
>>>>
>>>> I'm haven't yet closed on how to bridge this to qdev.  qdev is a big
>>>> consumer of QemuOpts today.  I have some general ideas about what I'd
>>>> like to do but so far, I haven't written anything down.
>>> Glad you mention qdev.
>>>
>>> qdev properties describe the configurable parts of qdev state objects.
>>> A state object is a C struct.  The description is C data.  Poor man's
>>> reflection.
>> Right.  The problem is it's very hard to reflect C even if you parse
>> it without additional annotations.  For instance:
>>
>> typedef struct Foo {
>>      Bar *bar;
>> } Foo;
>>
>> What the type of bar is is ambigious.  It could be a pointer to a list
>> of Bar's (if bar has an embedded pointer), it could be an array of
>> Bars that is terminated using a field within Bar, it could be a
>> pointer to a fixed size array of Bars, or it could be just a pointer
>> to a single Bar object.
>>
>> So you end up needing additional annotations like:
>>
>> typedef struct Foo {
>>     size_t n_bar;
>>     Bar *bar sizeis(n_bar);
>> } Foo;
>>
>> This is what most IDLs that use C style syntax do.
> We currently use a more low-tech approach: define the struct in plain C,
> and the data describing the struct in plain C as well.
>
> Information about the type is in two places and in two formats (C type
> declaration and C data intializer).  There's a bit of redundancy.
> Ensuring consistency requires preprocessor hackery.

I've explored this to what I believe it's limit without crossing into 
extreme non-portability.

I think the best you can possibly do is a scheme like the following:

typedef struct MyStruct {
     int x;
     int y;
     MyOtherStruct *mo_list;
     MyStruct *next;
} MyStruct;

TypeInfo type_into_MyStruct = {
      .name = "MyStruct",
      .params = {
          DEFINE_PARAM(MyStruct, int, x),
          DEFINE_PARAM(MyStruct, int, y),
          DEFINE_LIST(MyStruct, MyOther, mo_list),
          DEFINE_NEXT(MyStruct, next),
          DEFINE_EOL(),
       },
};

But there is absolutely no type safety here.  You can confuse the type 
of mo_list as an int and you'll get no errors.

To get type safety, you need to have macros for each type.  However, 
this makes it very difficult to support things like lists of lists or 
anything else that would basically require a non-concrete type.

The basic problem here is that we're mixing how to transverse a data 
structure with how to describe a data structure.  If you separate those 
two things, it becomes cleaner:

void marshal_MyStruct(MyStruct *obj, const char *name)
{
      marshal_start_struct("MyStruct", name);
      marshal_int(&obj->x, "x");
      marshal_int(&obj->y, "y");
      marshal_start_list("mo_list");
      for (MyOtherStruct *i = obj->mo_list; i; i = i->next) {
          marshal_MyOtherStruct(i, "");
      }
      marshal_end_list();
      marshal_end_struct();
}

This generalizes very well to almost arbitrarily complicated data 
structures and is really almost as compact as the data structure 
version.  It also has very strong type safety.   But it's not good 
enough because C doesn't have exceptions so if a marshal error occurs, 
you need to propagate it.  So then you get:

void marshal_MyStruct(MyStruct *obj, const char *name, Error **errp)
{
      marshal_start_struct("MyStruct", name, errp);
      if (error_is_set(errp)) {
          return;
      }
      marshal_int(&obj->x, "x", errp);
       ...
}

Unwinding can be a bit challenging too without destructors assuming that 
some of the marshalling routines allocate state (they will for lists).

But this provides an extremely nice marshaller/unmarshaller.  You get 
high quality errors and you can support arbitrarily complex objects.

This is what qmp-gen.py creates for the qmp-marshal-types module.

But it creates quite a bit more.  Even if you write the above by hand 
(or use struct to describe it), these types are complex and cannot be 
free'd with a simple free call.  So you also need a function to free 
each type.

If you plan to expose these types in a library, you need to either 
explicitly pad each structure and make sure that the padding is updated 
correctly each time a new member is added.  Alternatively, you can add 
an allocation function that automatically pads each structure transparently.

qmp-gen.py creates qmp-types.[ch] to do exactly the above and also 
generates the type declaration so that you don't have to duplicate the 
type marshalling code and the type declaration.  Today, this is close to 
2k LOCs so it's actually a significant amount of code code.

There is also the code that takes the input (via QCFG or QMP) and calls 
an appropriate C function with a strongly typed argument.  I've played 
with libffi here to try to do this dynamically but you still lose strong 
typing because there's no (obvious) way to check the types of a function 
pointer against a dynamic type description at compile time.  I've tried 
to do some serious preprocessor-fu here to make it work but have failed.

Finally, on the libqmp side, you need to code that takes C arguments and 
calls the appropriate marshalling routines to build the variant types.  
This may not seem relevant with QCFG but as we make command line options 
configurable via QMP, it becomes important.

> The data doesn't have to describe all struct members.  I'm inclined to
> regard that as a feature.
>
> Avoids generating source code.

I don't think we really can and still preserve type safety.  All the 
techniques I see to do it without code generation also mean we still 
have to write a fair bit of code by hand.

We could actually avoid it if we were using C++ because templates 
(particularly with specialization) are pretty much what the code 
generator is making up for.  But yeah, I'm not going to even try to make 
that suggestion :-)

It's still hard to do a proper libqmp even with C++ FWIW.

>> There would be a separate interface for getting/setting properties.
>> It might even be something as simple as, you have to implement:
>>
>> int isa_serial_get_iobase(ISADeviceState *dev);
>> int isa_serial_get_irq(ISADeviceState *dev);
>> ...
> To be honest, I'm wary of generating lots of C source.  Prone to become
> a pain to debug and change.  Generated code tends to be much worse than
> data in that regard.
>
> If you generate significant parts of your program with other programs,
> chances are that you're not using the right language for the job.

Yeah, C really sucks when it comes to working with types in an abstract 
way.  But the real question is, would we rather have C++ with fancy 
template code or C with some code generation to make up for C's 
incredibly limited type system?

>> This ends up being a powerful interface because you can easily get
>> these properties within QEMU, but you also (transparently) map these
>> to the wire.  It also extends really nicely for setting properties
>> which becomes an interesting way to implement dynamic device logic
>> (like setting the link status of a network card).
> Well, the traditional way to "get properties" within QEMU is the ol' ->
> operator.
>
> I'm not sure I get the rest of the paragraph.

I mean:

(qemu) device_get virtio-pci.0 link_status
True
(qemu) device_set virtio-pci.0 link_status False
(qemu) device_get virtio-pci.0
link_status: False
mac-address: 00:12:32:43:54:92
vectors: 1
...

We already have read-only and construct-only properties.  A writable 
property is an obvious next step.

>>> Yet another one: vmstate, which describes migratable parts of qdev state
>>> objects.
>> Yes, for now, I'm ignoring vmstate.  To really solve vmstate, I think
>> you need to describe the complete object instead of just it's
>> properties.
> Would be nice to have a solution that can cover all our reflection
> needs, including vmstate.  But we need to watch out not to get bogged
> down in the Generality Swamps.

The mechanism I described using the visitor pattern is really the right 
solution for vmstate.  The hard problems to solve for vmstate are:

1) How to we support old versions in a robust way.  There are fancy 
things we could do once we have a proper visitor mechanism.  We could 
have special marshallers for old versions, we could treat the output of 
the visitor as an in memory tree and do XSLT style translations, etc.

2) How do we generate the visitor for each device.  I don't think it's 
practical to describe devices in JSON.  It certainly simplifies the 
problem but it seems ugly to me.  I think we realistically need a C 
style IDL and adopt a style of treating it as a header.

I think (1) is pretty straight forward but requires careful auditing of 
all of the weirdness we currently have.  (2) isn't so hard but we need 
to make sure the syntax is right from the start.

>>> Unlike these two, QCFG doesn't describe C structs, it generates them
>>> from JSON specs.  If I understand your proposal correctly.  Hmm.
>> Correct.
>>
>>> Can we avoid defining our data types in JSON rather than C?
>> I didn't start with describing them in JSON.  I started by describing
>> them in Python with the idea that I'd write a IDL parser using a C
>> style syntax that would then generate the Python structures.  Using
>> the fixed Python data structures, I could play with code generation
>> without all the trouble of writing a full blown IDL parser.
>>
>> But I never got around to writing that parser and have actually become
>> fond of the Python/JSON style syntax.
> Well, let's say "fondness" isn't what I'd expect ordinary people to feel
> when confonted with the idea to define half our structs in Python rather
> than C ;)

The thing I really like is that it limits you.  With a C based approach, 
there is almost an uncanny valley effect where there's an expectation 
that since my type looks mostly like a C structure, I should be able to 
do anything that I would normally do in C.  This is the real thing that 
I think frustrates most people with C style IDLs.

But with a JSON IDL, you approach it differently.  It's obviously not C 
and you start with the assumption that you can't do crazy things (like 
bitfields).

>>> Can we adopt *one* reflection mechanism?
>> Yes, I think we should start with that being the JSON syntax I'm using
>> for QAPI/QCFG and then down the road, we can introduce a totally new
>> syntax if need be.  But I think we need a central schema that
>> describes all externally visible interfaces.  I think this is really
>> the key idea here.
> Yes, self-describing external interfaces are desirable.
>
> Speaking of external interfaces: we need to be careful when exposing
> internal data types externally.  If we couple our external interfaces
> too closely to internals, we risk calcifying our internal interfaces.
>
> qdev has gotten that partly right: you can change the state structs
> pretty freely without affecting the externally visible properties.

Yeah, this is one of the big challenges with vmstate.  There needs to be 
a clearer line between internal types and object models vs. the 
externally visible interfaces.

Regards,

Anthony Liguori

> [...]
>

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 03/18/2011 09:04 AM, Markus Armbruster wrote:
>>>>> Initial code is in my QAPI tree.
>>>>>
>>>>> I'm not going to start converting things until we get closer to the
>>>>> end of 0.15 and QAPI is fully merged.  My plan is to focus on this for
>>>>> 0.16 and do a full conversion for the 0.16 time frame using the same
>>>>> approach as QAPI.  That means that for 0.16, we would be able to set
>>>>> all command line options via QMP in a programmatic fashion with full
>>>>> support for introspection.
>>>>>
>>>>> I'm haven't yet closed on how to bridge this to qdev.  qdev is a big
>>>>> consumer of QemuOpts today.  I have some general ideas about what I'd
>>>>> like to do but so far, I haven't written anything down.
>>>> Glad you mention qdev.
>>>>
>>>> qdev properties describe the configurable parts of qdev state objects.
>>>> A state object is a C struct.  The description is C data.  Poor man's
>>>> reflection.
>>> Right.  The problem is it's very hard to reflect C even if you parse
>>> it without additional annotations.  For instance:
>>>
>>> typedef struct Foo {
>>>      Bar *bar;
>>> } Foo;
>>>
>>> What the type of bar is is ambigious.  It could be a pointer to a list
>>> of Bar's (if bar has an embedded pointer), it could be an array of
>>> Bars that is terminated using a field within Bar, it could be a
>>> pointer to a fixed size array of Bars, or it could be just a pointer
>>> to a single Bar object.
>>>
>>> So you end up needing additional annotations like:
>>>
>>> typedef struct Foo {
>>>     size_t n_bar;
>>>     Bar *bar sizeis(n_bar);
>>> } Foo;
>>>
>>> This is what most IDLs that use C style syntax do.
>> We currently use a more low-tech approach: define the struct in plain C,
>> and the data describing the struct in plain C as well.
>>
>> Information about the type is in two places and in two formats (C type
>> declaration and C data intializer).  There's a bit of redundancy.
>> Ensuring consistency requires preprocessor hackery.
>
> I've explored this to what I believe it's limit without crossing into
> extreme non-portability.
>
> I think the best you can possibly do is a scheme like the following:
>
> typedef struct MyStruct {
>     int x;
>     int y;
>     MyOtherStruct *mo_list;
>     MyStruct *next;
> } MyStruct;
>
> TypeInfo type_into_MyStruct = {
>      .name = "MyStruct",
>      .params = {
>          DEFINE_PARAM(MyStruct, int, x),
>          DEFINE_PARAM(MyStruct, int, y),
>          DEFINE_LIST(MyStruct, MyOther, mo_list),
>          DEFINE_NEXT(MyStruct, next),
>          DEFINE_EOL(),
>       },
> };
>
> But there is absolutely no type safety here.  You can confuse the type
> of mo_list as an int and you'll get no errors.
>
> To get type safety, you need to have macros for each type.

Type checking macros are feasible (see [*] for an existence proof), but
things do get hairy, and the resulting error messages can be less than
clear at times.

>                                                             However,
> this makes it very difficult to support things like lists of lists or
> anything else that would basically require a non-concrete type.

Sounds like you want a more expressive type system than C's, and propose
to get it by building your own DSL.  I'm not sure that's wise.

> The basic problem here is that we're mixing how to transverse a data
> structure with how to describe a data structure.  If you separate
> those two things, it becomes cleaner:
>
> void marshal_MyStruct(MyStruct *obj, const char *name)
> {
>      marshal_start_struct("MyStruct", name);
>      marshal_int(&obj->x, "x");
>      marshal_int(&obj->y, "y");
>      marshal_start_list("mo_list");
>      for (MyOtherStruct *i = obj->mo_list; i; i = i->next) {
>          marshal_MyOtherStruct(i, "");
>      }
>      marshal_end_list();
>      marshal_end_struct();
> }
>
> This generalizes very well to almost arbitrarily complicated data
> structures and is really almost as compact as the data structure
> version.  It also has very strong type safety.   But it's not good
> enough because C doesn't have exceptions so if a marshal error occurs,
> you need to propagate it.  So then you get:
>
> void marshal_MyStruct(MyStruct *obj, const char *name, Error **errp)
> {
>      marshal_start_struct("MyStruct", name, errp);
>      if (error_is_set(errp)) {
>          return;
>      }
>      marshal_int(&obj->x, "x", errp);
>       ...
> }
>
> Unwinding can be a bit challenging too without destructors assuming
> that some of the marshalling routines allocate state (they will for
> lists).
>
> But this provides an extremely nice marshaller/unmarshaller.  You get
> high quality errors and you can support arbitrarily complex objects.
>
> This is what qmp-gen.py creates for the qmp-marshal-types module.
>
> But it creates quite a bit more.  Even if you write the above by hand
> (or use struct to describe it), these types are complex and cannot be
> free'd with a simple free call.  So you also need a function to free
> each type.

I'm not advocating hand-written marshallers.  Data description should be
data, not code.  If you need a marshaller, generate it from the
descriptive data, or write a generic one that interprets the descriptive
data.

> If you plan to expose these types in a library, you need to either
> explicitly pad each structure and make sure that the padding is
> updated correctly each time a new member is added.

As long as the data description is data, writing a program to check that
a new version is compatible with the old one shouldn't be hard.

>                                                     Alternatively, you
> can add an allocation function that automatically pads each structure
> transparently.

Weaker than a comprehensive check, but could be good enough.

> qmp-gen.py creates qmp-types.[ch] to do exactly the above and also
> generates the type declaration so that you don't have to duplicate the
> type marshalling code and the type declaration.  Today, this is close
> to 2k LOCs so it's actually a significant amount of code code.
>
> There is also the code that takes the input (via QCFG or QMP) and
> calls an appropriate C function with a strongly typed argument.  I've

Not sure I got you here.  Perhaps an example could enlighten me :)

> played with libffi here to try to do this dynamically but you still
> lose strong typing because there's no (obvious) way to check the types
> of a function pointer against a dynamic type description at compile
> time.  I've tried to do some serious preprocessor-fu here to make it
> work but have failed.
>
> Finally, on the libqmp side, you need to code that takes C arguments
> and calls the appropriate marshalling routines to build the variant
> types.  This may not seem relevant with QCFG but as we make command
> line options configurable via QMP, it becomes important.
>
>> The data doesn't have to describe all struct members.  I'm inclined to
>> regard that as a feature.
>>
>> Avoids generating source code.
>
> I don't think we really can and still preserve type safety.  All the
> techniques I see to do it without code generation also mean we still
> have to write a fair bit of code by hand.
>
> We could actually avoid it if we were using C++ because templates
> (particularly with specialization) are pretty much what the code
> generator is making up for.  But yeah, I'm not going to even try to
> make that suggestion :-)
>
> It's still hard to do a proper libqmp even with C++ FWIW.
>
>>> There would be a separate interface for getting/setting properties.
>>> It might even be something as simple as, you have to implement:
>>>
>>> int isa_serial_get_iobase(ISADeviceState *dev);
>>> int isa_serial_get_irq(ISADeviceState *dev);
>>> ...
>> To be honest, I'm wary of generating lots of C source.  Prone to become
>> a pain to debug and change.  Generated code tends to be much worse than
>> data in that regard.
>>
>> If you generate significant parts of your program with other programs,
>> chances are that you're not using the right language for the job.
>
> Yeah, C really sucks when it comes to working with types in an
> abstract way.  But the real question is, would we rather have C++ with
> fancy template code or C with some code generation to make up for C's
> incredibly limited type system?

Yes, C is limited.  It makes you keep things simple & stupid, because as
you stray away from that, pain increases sharply.

Maybe what you try to do here simply isn't in C's application domain
anymore.

But I don't think C++ with fancy templating is a viable solution to the
problem either.  The set of people understanding fancy templating may
not be empty, but it's certainly small.  Much smaller than the set of
people who think they understand fancy templating.

>>> This ends up being a powerful interface because you can easily get
>>> these properties within QEMU, but you also (transparently) map these
>>> to the wire.  It also extends really nicely for setting properties
>>> which becomes an interesting way to implement dynamic device logic
>>> (like setting the link status of a network card).
>> Well, the traditional way to "get properties" within QEMU is the ol' ->
>> operator.
>>
>> I'm not sure I get the rest of the paragraph.
>
> I mean:
>
> (qemu) device_get virtio-pci.0 link_status
> True
> (qemu) device_set virtio-pci.0 link_status False
> (qemu) device_get virtio-pci.0
> link_status: False
> mac-address: 00:12:32:43:54:92
> vectors: 1
> ...
>
> We already have read-only and construct-only properties.  A writable
> property is an obvious next step.

Got it.

>>>> Yet another one: vmstate, which describes migratable parts of qdev state
>>>> objects.
>>> Yes, for now, I'm ignoring vmstate.  To really solve vmstate, I think
>>> you need to describe the complete object instead of just it's
>>> properties.
>> Would be nice to have a solution that can cover all our reflection
>> needs, including vmstate.  But we need to watch out not to get bogged
>> down in the Generality Swamps.
>
> The mechanism I described using the visitor pattern is really the
> right solution for vmstate.  The hard problems to solve for vmstate
> are:
>
> 1) How to we support old versions in a robust way.  There are fancy
> things we could do once we have a proper visitor mechanism.  We could
> have special marshallers for old versions, we could treat the output
> of the visitor as an in memory tree and do XSLT style translations,
> etc.
>
> 2) How do we generate the visitor for each device.  I don't think it's
> practical to describe devices in JSON.  It certainly simplifies the
> problem but it seems ugly to me.  I think we realistically need a C
> style IDL and adopt a style of treating it as a header.

Now I'm confused.  Do you mean your JSON-based DSL won't cut it for
vmstate?

If yes, why is it wise to go down that route now?

> I think (1) is pretty straight forward but requires careful auditing
> of all of the weirdness we currently have.  (2) isn't so hard but we
> need to make sure the syntax is right from the start.
>
>>>> Unlike these two, QCFG doesn't describe C structs, it generates them
>>>> from JSON specs.  If I understand your proposal correctly.  Hmm.
>>> Correct.
>>>
>>>> Can we avoid defining our data types in JSON rather than C?
>>> I didn't start with describing them in JSON.  I started by describing
>>> them in Python with the idea that I'd write a IDL parser using a C
>>> style syntax that would then generate the Python structures.  Using
>>> the fixed Python data structures, I could play with code generation
>>> without all the trouble of writing a full blown IDL parser.
>>>
>>> But I never got around to writing that parser and have actually become
>>> fond of the Python/JSON style syntax.
>> Well, let's say "fondness" isn't what I'd expect ordinary people to feel
>> when confonted with the idea to define half our structs in Python rather
>> than C ;)
>
> The thing I really like is that it limits you.  With a C based
> approach, there is almost an uncanny valley effect where there's an
> expectation that since my type looks mostly like a C structure, I
> should be able to do anything that I would normally do in C.  This is
> the real thing that I think frustrates most people with C style IDLs.
>
> But with a JSON IDL, you approach it differently.  It's obviously not
> C and you start with the assumption that you can't do crazy things
> (like bitfields).
>
>>>> Can we adopt *one* reflection mechanism?
>>> Yes, I think we should start with that being the JSON syntax I'm using
>>> for QAPI/QCFG and then down the road, we can introduce a totally new
>>> syntax if need be.  But I think we need a central schema that
>>> describes all externally visible interfaces.  I think this is really
>>> the key idea here.
>> Yes, self-describing external interfaces are desirable.
>>
>> Speaking of external interfaces: we need to be careful when exposing
>> internal data types externally.  If we couple our external interfaces
>> too closely to internals, we risk calcifying our internal interfaces.
>>
>> qdev has gotten that partly right: you can change the state structs
>> pretty freely without affecting the externally visible properties.
>
> Yeah, this is one of the big challenges with vmstate.  There needs to
> be a clearer line between internal types and object models vs. the
> externally visible interfaces.

Not only with vmstate.  If we couple QMP closely to internal interfaces,
I'm afraid we'll end up in the same unhappy place we're now with
vmstate.

> Regards,
>
> Anthony Liguori
>
>> [...]
>>

[*] http://ccan.ozlabs.org/info/check_type.html 

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Anthony Liguori]

On 03/22/2011 08:01 AM, Markus Armbruster wrote:
> Type checking macros are feasible (see [*] for an existence proof), but
> things do get hairy, and the resulting error messages can be less than
> clear at times.

That just gives you a warning.  You can do much better things with 
__builtin_types_compatible_p.

But neither really solves the problem I'm talking about.  I can go into 
it in depth but hopefully we both can agree that trying to build 
introspection macros is pretty difficult if not impossible :-)

>>                                                              However,
>> this makes it very difficult to support things like lists of lists or
>> anything else that would basically require a non-concrete type.
> Sounds like you want a more expressive type system than C's, and propose
> to get it by building your own DSL.  I'm not sure that's wise.

It's an IDL.  IDL based RPCs are pretty common with C.  The IDL is 
purely declarative.

>> If you plan to expose these types in a library, you need to either
>> explicitly pad each structure and make sure that the padding is
>> updated correctly each time a new member is added.
> As long as the data description is data, writing a program to check that
> a new version is compatible with the old one shouldn't be hard.

If you define the structs on your own, you need to either have a data 
description of the padding or be very careful doing it yourself.

>>                                                      Alternatively, you
>> can add an allocation function that automatically pads each structure
>> transparently.
> Weaker than a comprehensive check, but could be good enough.
>
>> qmp-gen.py creates qmp-types.[ch] to do exactly the above and also
>> generates the type declaration so that you don't have to duplicate the
>> type marshalling code and the type declaration.  Today, this is close
>> to 2k LOCs so it's actually a significant amount of code code.
>>
>> There is also the code that takes the input (via QCFG or QMP) and
>> calls an appropriate C function with a strongly typed argument.  I've
> Not sure I got you here.  Perhaps an example could enlighten me :)

void qapi_free_vnc_info(VncInfo *obj)
{
     if (!obj) {
         return;
     }
     if (obj->has_host) {
         qemu_free(obj->host);
     }
     if (obj->has_family) {
         qemu_free(obj->family);
     }
     if (obj->has_service) {
         qemu_free(obj->service);
     }
     if (obj->has_auth) {
         qemu_free(obj->auth);
     }
     if (obj->has_clients) {
         qapi_free_vnc_client_info(obj->clients);
     }


     qapi_free_vnc_info(obj->next);
     qemu_free(obj);
}

It's pretty basic boiler plate code that could be written by hand, but 
why not generate it.  It actually all adds up pretty quickly in terms of 
SLOCs.

>> The mechanism I described using the visitor pattern is really the
>> right solution for vmstate.  The hard problems to solve for vmstate
>> are:
>>
>> 1) How to we support old versions in a robust way.  There are fancy
>> things we could do once we have a proper visitor mechanism.  We could
>> have special marshallers for old versions, we could treat the output
>> of the visitor as an in memory tree and do XSLT style translations,
>> etc.
>>
>> 2) How do we generate the visitor for each device.  I don't think it's
>> practical to describe devices in JSON.  It certainly simplifies the
>> problem but it seems ugly to me.  I think we realistically need a C
>> style IDL and adopt a style of treating it as a header.
> Now I'm confused.  Do you mean your JSON-based DSL won't cut it for
> vmstate?
>
> If yes, why is it wise to go down that route now?

There are a few paths we could go.  We can describe devices in JSON.  
This makes VMState introspectable with all of the nice properties of 
everything else.  But the question is, do we describe the full device 
state and then use a separate mechanism to cull out the bits that can be 
recomputed.

To we only describe the guest visible state and treat that as a separate 
structure?  Is that embedded in the main state object or do we 
explicitly translate the main state object to this new type?

We can pretty easily have a C-like IDL so I'm not terribly concerned 
about describing devices in JSON.  It's about finding the right way to 
structure device marshalling in the long term.

So yes, I think JSON is the right approach, but that doesn't mean I've 
figured out how to do VMState.

>> Yeah, this is one of the big challenges with vmstate.  There needs to
>> be a clearer line between internal types and object models vs. the
>> externally visible interfaces.
> Not only with vmstate.  If we couple QMP closely to internal interfaces,
> I'm afraid we'll end up in the same unhappy place we're now with
> vmstate.

Yeah, it's a tough balance to strike.  If you expose too much detail 
about internals, it's very difficult to maintain compatibility in the 
long term.  If you have a pure external interface, it's difficult to 
make sure that the external interfaces are actually useful because 
there's no immediate feedback mechanism.

Regards,

Anthony Liguori

>> Regards,
>>
>> Anthony Liguori
>>
>>> [...]
>>>
> [*] http://ccan.ozlabs.org/info/check_type.html
>

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 03/22/2011 08:01 AM, Markus Armbruster wrote:
>> Type checking macros are feasible (see [*] for an existence proof), but
>> things do get hairy, and the resulting error messages can be less than
>> clear at times.
>
> That just gives you a warning.  You can do much better things with
> __builtin_types_compatible_p.

Quote of existence proof != this is the best way to do it :)

> But neither really solves the problem I'm talking about.  I can go
> into it in depth but hopefully we both can agree that trying to build
> introspection macros is pretty difficult if not impossible :-)

Depends on what you need from them.

>>>                                                              However,
>>> this makes it very difficult to support things like lists of lists or
>>> anything else that would basically require a non-concrete type.
>> Sounds like you want a more expressive type system than C's, and propose
>> to get it by building your own DSL.  I'm not sure that's wise.
>
> It's an IDL.  IDL based RPCs are pretty common with C.  The IDL is
> purely declarative.

Still sounds like you want a more expressive type system than C's, and
propose to build it ourselves.

First step would be to define the type system rigorously.

>>> If you plan to expose these types in a library, you need to either
>>> explicitly pad each structure and make sure that the padding is
>>> updated correctly each time a new member is added.
>> As long as the data description is data, writing a program to check that
>> a new version is compatible with the old one shouldn't be hard.
>
> If you define the structs on your own, you need to either have a data
> description of the padding or be very careful doing it yourself.

With a program to check compatibility, you can easily make the build
fail when you break compatibility.

>>>                                                      Alternatively, you
>>> can add an allocation function that automatically pads each structure
>>> transparently.
>> Weaker than a comprehensive check, but could be good enough.
>>
>>> qmp-gen.py creates qmp-types.[ch] to do exactly the above and also
>>> generates the type declaration so that you don't have to duplicate the
>>> type marshalling code and the type declaration.  Today, this is close
>>> to 2k LOCs so it's actually a significant amount of code code.
>>>
>>> There is also the code that takes the input (via QCFG or QMP) and
>>> calls an appropriate C function with a strongly typed argument.  I've
>> Not sure I got you here.  Perhaps an example could enlighten me :)
>
> void qapi_free_vnc_info(VncInfo *obj)
> {
>     if (!obj) {
>         return;
>     }
>     if (obj->has_host) {
>         qemu_free(obj->host);
>     }
>     if (obj->has_family) {
>         qemu_free(obj->family);
>     }
>     if (obj->has_service) {
>         qemu_free(obj->service);
>     }
>     if (obj->has_auth) {
>         qemu_free(obj->auth);
>     }
>     if (obj->has_clients) {
>         qapi_free_vnc_client_info(obj->clients);
>     }
>
>
>     qapi_free_vnc_info(obj->next);
>     qemu_free(obj);
> }
>
> It's pretty basic boiler plate code that could be written by hand, but
> why not generate it.  It actually all adds up pretty quickly in terms
> of SLOCs.

Since we have the data description anyway, why not

    for all members
        if member needs destruction
            destroy it

Just because we can generate boiler-plate code doesn't mean we should
have boiler-plate code.

>>> The mechanism I described using the visitor pattern is really the
>>> right solution for vmstate.  The hard problems to solve for vmstate
>>> are:
>>>
>>> 1) How to we support old versions in a robust way.  There are fancy
>>> things we could do once we have a proper visitor mechanism.  We could
>>> have special marshallers for old versions, we could treat the output
>>> of the visitor as an in memory tree and do XSLT style translations,
>>> etc.
>>>
>>> 2) How do we generate the visitor for each device.  I don't think it's
>>> practical to describe devices in JSON.  It certainly simplifies the
>>> problem but it seems ugly to me.  I think we realistically need a C
>>> style IDL and adopt a style of treating it as a header.
>> Now I'm confused.  Do you mean your JSON-based DSL won't cut it for
>> vmstate?
>>
>> If yes, why is it wise to go down that route now?
>
> There are a few paths we could go.  We can describe devices in JSON.
> This makes VMState introspectable with all of the nice properties of
> everything else.  But the question is, do we describe the full device
> state and then use a separate mechanism to cull out the bits that can
> be recomputed.
>
> To we only describe the guest visible state and treat that as a
> separate structure?  Is that embedded in the main state object or do
> we explicitly translate the main state object to this new type?

Requiring fields of the externally visible vmstate wire format to be
backed by members of the device state struct (whether they are right in
the outermost struct or not) creates a potentially troublesome tight
coupling between wire format and internal representation.

You can avoid such coupling by permitting wire format fields to be
backed by functions rather than struct members.

> We can pretty easily have a C-like IDL so I'm not terribly concerned
> about describing devices in JSON.  It's about finding the right way to
> structure device marshalling in the long term.
>
> So yes, I think JSON is the right approach, but that doesn't mean I've
> figured out how to do VMState.
>
>>> Yeah, this is one of the big challenges with vmstate.  There needs to
>>> be a clearer line between internal types and object models vs. the
>>> externally visible interfaces.
>> Not only with vmstate.  If we couple QMP closely to internal interfaces,
>> I'm afraid we'll end up in the same unhappy place we're now with
>> vmstate.
>
> Yeah, it's a tough balance to strike.  If you expose too much detail
> about internals, it's very difficult to maintain compatibility in the
> long term.  If you have a pure external interface, it's difficult to
> make sure that the external interfaces are actually useful because
> there's no immediate feedback mechanism.

The only valid judges on the usefulness of external interfaces are
external users.

Coupling them tightly to internal interfaces just to shortcut that
judgement doesn't strike me as a smart idea.  Please note the "tightly"
there.

>
> Regards,
>
> Anthony Liguori
>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>> [...]
>>>>
>> [*] http://ccan.ozlabs.org/info/check_type.html