Re: [Qemu-devel] [PATCH for-3.2 v4 25/28] machine: add compat-props interface

[Eduardo Habkost]

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 <address@hidden>
[...]
> 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(...)
  }

Most people don't understand QOM interfaces and their
initialization ordering rules.  Everybody understands C function
calls.

> [...]

-- 
Eduardo