Re: Integrating QOM into QAPI

[Christophe de Dinechin]

> On 21 Jan 2020, at 16:11, Marc-André Lureau <address@hidden> wrote:
> 
> Hi
> 
> On Tue, Jan 21, 2020 at 7:01 PM Markus Armbruster <address@hidden> wrote:
>> 
>> Daniel P. Berrangé <address@hidden> writes:
>> 
>>> On Tue, Jan 21, 2020 at 02:36:17PM +0100, Markus Armbruster wrote:
>>>> Marc-André Lureau <address@hidden> writes:
>>>> 
>>>>> Hi
>>>>> 
>>>>> On Tue, Jan 21, 2020 at 3:32 PM Stefan Hajnoczi <address@hidden> wrote:
>>>>>> 
>>>>>> On Tue, Jan 21, 2020 at 06:42:47AM +0100, Markus Armbruster wrote:
>>>>>>> Stefan Hajnoczi <address@hidden> writes:
>>>>>>> 
>>>>>>>> On Wed, Jan 15, 2020 at 01:15:17PM +0100, Markus Armbruster wrote:
>>>>>>>>> Christophe de Dinechin <address@hidden> writes:
>>>>>>>>>>> On 15 Jan 2020, at 10:20, Markus Armbruster <address@hidden> wrote:
>>>>>>>>> * qemuMonitorJSONSetIOThread() uses it to control iothread's 
>>>>>>>>> properties
>>>>>>>>>  poll-max-ns, poll-grow, poll-shrink.  Their use with -object is
>>>>>>>>>  documented (in qemu-options.hx), their use with qom-set is not.
>>>>>>>> 
>>>>>>>> I'm happy to use a different interface.
>>>>>>>> 
>>>>>>>> Writing a boilerplate "iothread-set-poll-params" QMP command in C would
>>>>>>>> be a step backwards.
>>>>>>> 
>>>>>>> No argument.
>>>>>>> 
>>>>>>>> Maybe the QAPI code generator could map something like this:
>>>>>>>> 
>>>>>>>>  { 'command': 'iothread-set-poll-params',
>>>>>>>>    'data': {
>>>>>>>>        'id': 'str',
>>>>>>>>    '*max-ns': 'uint64',
>>>>>>>>    '*grow': 'uint64',
>>>>>>>>    '*shrink': 'uint64'
>>>>>>>>    },
>>>>>>>>    'map-to-qom-set': 'IOThread'
>>>>>>>>  }
>>>>>>>> 
>>>>>>>> And turn it into QOM accessors on the IOThread object.
>>>>>>> 
>>>>>>> I think a generic "set this configuration to that value" command is just
>>>>>>> fine.  qom-set fails on several counts, though:
>>>>>>> 
>>>>>>> * Tolerable: qom-set is not actually generic, it applies only to QOM.
>>>>>>> 
>>>>>>> * qom-set lets you set tons of stuff that is not meant to be changed at
>>>>>>>  run time.  If it breaks your guest, you get to keep the pieces.
>>>>>>> 
>>>>>>> * There is virtually no documentation on what can be set to what values,
>>>>>>>  and their semantics.
>>>>>>> 
>>>>>>> In its current state, QOM is a user interface superfund site.
>>>>>> 
>>>>>> Thoughts about a solution:
>>>>>> 
>>>>>> Static QOM properties should be declared via QAPI instead of
>>>>>> imperatively via QOM APIs.  That way they are introspectable and type
>>>>>> information is present in the schema.
>>>>>> 
>>>>>> The QAPI code generator could emit a function that is callable from
>>>>>> .class_init().  This eliminates the need to manually call
>>>>>> object_class_property_add().
>>>> 
>>>> We need to make up our minds what exactly we want generated.  Then we
>>>> can design the QAPI language, and code up the generator.
>>>> 
>>>> Skeleton QOM type, to help with the discussion:
>>>> 
>>>>    #define TYPE_FOO "foo"
>>>> 
>>>>    #define FOO(obj) OBJECT_CHECK(Foo, (obj), TYPE_FOO)
>>>>    #define FOO_CLASS(klass) \
>>>>        OBJECT_CLASS_CHECK(FooClass, (klass), TYPE_FOO)
>>>>    #define FOO_GET_CLASS(obj) \
>>>>        OBJECT_GET_CLASS(FooClass, (obj), TYPE_FOO)
>>>> 
>>>>    typedef FooClass {
>>>>        ParentClass parent_class;
>>>>        ... // hand-written per-class state
>>>>    }
>>>> 
>>>>    struct Chardev {
>>>>        ParentObject parent_obj;
>>>>        ... // hand-written instance (per-object) state
>>>>    };
>>>> 
>>>>    static const TypeInfo char_type_info = {
>>>>        .name = TYPE_FOO,
>>>>        .parent = TYPE_OBJECT,
>>>>        .instance_size = sizeof(Foo),
>>>>        .instance_init = ...,                   // methods to initialize
>>>>        .instance_post_init = ...,              // and finalize instances,
>>>>        .instance_finalize = ...,               // all optional
>>>>        .abstract = ...,                        // true or false (d'oh)
>>>>        .class_size = sizeof(FooClass),
>>>>        .class_init = ...,                      // methods to initialize
>>>>        .class_base_init = ...,                 // classes, optional
>>>>        .class_data = ...,                      // extra argument for them
>>>>        .interfaces = ...
>>>>    };
>>>> 
>>>> There's substantial boilerplate, with plenty of hand-written code in the
>>>> gaps.  What of the boilerplate do we plan to generate?  How do we plan
>>>> to fill the gaps, if any?
>>> 
>>> FWIW, even without a QOM generator, we can do waaaaaaay better on the
>>> amount of boilerplate needed for QOM without very much work. It just
>>> needs a few convenience macros writing.
>>> 
>>> QOM is not GObject, but is heavily inspired by it and so looking at
>>> GObject gives us a design pattern we can aim to match in terms of
>>> amount of boilerplate.
>>> 
>>> What we do manually with TypeInfo struct there has essentially always
>>> been done by a 1 line macro in GObject:
>>> 
>>>  G_DEFINE_TYPE(virIdentity, vir_identity, G_TYPE_OBJECT)
>>> 
>>> If implementing interfaces, there's 1 extra line needed per interface
>>> to associate them.
>>> 
>>>  
>>> https://developer.gnome.org/gobject/stable/gobject-Type-Information.html#G-DEFINE-TYPE:CAPS
>>> 
>>> 
>>> And what we do in the header file to add the 4 or more FOO_XXX macros,
>>> and the class struct and the object struct has recently been turned
>>> into a 2-liner:
>>> 
>>>  #define VIR_TYPE_IDENTITY vir_identity_get_type()
>>>  G_DECLARE_FINAL_TYPE(virIdentity, vir_identity, VIR, IDENTITY, GObject);
>>> 
>>>  
>>> https://developer.gnome.org/gobject/stable/gobject-Type-Information.html#G-DECLARE-FINAL-TYPE:CAPS
>>> 
>>> Or
>>> 
>>>  #define VIR_TYPE_IDENTITY vir_identity_get_type()
>>>  G_DECLARE_DERIVABLE_TYPE(virIdentity, vir_identity, VIR, IDENTITY, 
>>> GObject);
>>> 
>>>  
>>> https://developer.gnome.org/gobject/stable/gobject-Type-Information.html#G-DECLARE-DERIVABLE-TYPE:CAPS
>>> 
>>> 
>>> It would be nice to have a QOM code generator so that we can statically
>>> declare properties & parent/child/interface relationships, but for an
>>> immediate low cost win, better macros would be very useful IMHO.
>> 
>> Volunteers?
>> 
> 
> Actually, we are not that far off from being able to use GObject
> altogether (I hacked something like that to play with), but I
> disgress...
> 
> So introducing GObject-like macros? sure!

I’m still puzzled as to why anybody would switch to something like
GObject when there is C++.

I’m serious.

> 
> There are plenty of refactoring to do. The problem when touching the
> whole code-base, imho, is review time. It may take a couple of
> hours/days to come up with a cocci/spatch, and make various patches
> here and there. But it takes often weeks and a lot of constant push to
> various folks to get all the reviews (as seens by the qdev prop-ptr
> series earlier for example). How can we better address whole code-base
> changes?
> 
> 
> 
> -- 
> Marc-André Lureau
>