On Thu, 29 Nov 2018 15:49:00 -0200
Eduardo Habkost <ehabk...@redhat.com> wrote:
> On Tue, Nov 27, 2018 at 01:27:58PM +0400, Marc-André Lureau wrote:
> > Let's make compatiblity properties an interface, so that objects other
> > than QDev can benefit from having machine compatiblity properties.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> [...]
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index 3b31b2c025..b0ee05f837 100644
> > --- a/hw/core/qdev.c
> > +++ b/hw/core/qdev.c
> > @@ -970,28 +970,8 @@ static void device_initfn(Object *obj)
> > QLIST_INIT(&dev->gpios);
> > }
> >
> > -static const GPtrArray *ac_compat_props;
> > -static const GPtrArray *mc_compat_props;
> > -
> > -void accel_register_compat_props(const GPtrArray *props)
> > -{
> > - ac_compat_props = props;
> > -}
> > -
> > -void machine_register_compat_props(const GPtrArray *props)
> > -{
> > - mc_compat_props = props;
> > -}
> > -
> > static void device_post_init(Object *obj)
> > {
> > - if (ac_compat_props) {
> > - object_apply_global_props(obj, ac_compat_props, &error_abort);
> > - }
> > - if (mc_compat_props) {
> > - object_apply_global_props(obj, mc_compat_props, &error_abort);
> > - }
> > -
> > qdev_prop_set_globals(DEVICE(obj));
> > }
> >
> > @@ -1124,6 +1104,10 @@ static const TypeInfo device_type_info = {
> > .class_init = device_class_init,
> > .abstract = true,
> > .class_size = sizeof(DeviceClass),
> > + .interfaces = (InterfaceInfo[]) {
> > + { TYPE_COMPAT_PROPS },
> > + { }
> > + }
>
> At first I thought TYPE_COMPAT_PROPS was a practical way to
> implement this feature, but now I'm worried: the ordering between
> compat_props_post_init() qdev_prop_set_globals() is very
> important (user-provided globals must always be set after compat
> props), and here the ordering is implicit and easy to break
> accidentally.
>
> What if instead of a QOM interface we just provide a simple
> object_apply_compat_props() function? e.g.:
>
> qdev.c:
>
> static void device_post_init(Object *obj)
> {
> object_apply_compat_props(obj);
> apply_user_provided_globals(obj);
> }
>
> object_interface.c:
>
> void user_creatable_complete(Object *obj, Error **errp)
> {
> object_apply_compat_props(obj);
> ...
> ucc->complete(...)
> }
this would work as long as nothing is happening between
object_new() ... user_creatable_complete() but look at
current users of user_creatable_complete() so it's
fragile too.
Reasons for compat props interface are the same as for
instance_post_init/device_post_init.
the thing we can do here is getting rid of device_post_init
and making device override TYPE_COMPAT_PROPS::instance_post_init
to make explicit ordering like we do everywhere else:
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6b3cc55..46ad6f5 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -1033,11 +1033,19 @@ static void device_unparent(Object *obj)
}
}
+device_compat_props()
+{
+ dc->parent_compat_props()
+ apply_global_props()
+}
+
static void device_class_init(ObjectClass *class, void *data)
{
DeviceClass *dc = DEVICE_CLASS(class);
class->unparent = device_unparent;
+ dc->parent_compat_props = COMPAT_PROPS_GET_CLASS(class)->instance_post_init
+ COMPAT_PROPS_GET_CLASS(class)->instance_post_init = device_compat_props()
/* by default all devices were considered as hotpluggable,
* so with intent to check it in generic qdev_unplug() /
> Most people don't understand QOM interfaces and their
> initialization ordering rules. Everybody understands C function
> calls.
>
> > [...]
>
