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

[Markus Armbruster]

Anthony Liguori <address@hidden> 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