Re: [Qemu-devel] [PATCH 5/9] qom/globals: generalize object_property_set_globals()

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 12 Sep 2018 16:55:27 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> Handle calls of object_property_set_globals() with any object type,
> but only apply globals to TYPE_DEVICE & TYPE_USER_CREATABLE.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qom/globals.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/qom/globals.c b/qom/globals.c
> index 587f4a1b5c..8664baebe0 100644
> --- a/qom/globals.c
> +++ b/qom/globals.c
> @@ -15,22 +15,28 @@ void object_property_register_global(GlobalProperty *prop)
>  
>  void object_property_set_globals(Object *obj)
>  {
> -    DeviceState *dev = DEVICE(obj);
>      GList *l;
> +    DeviceState *dev = (DeviceState *)object_dynamic_cast(obj, TYPE_DEVICE);
> +
> +    if (!dev && !IS_USER_CREATABLE(obj)) {
> +        /* only TYPE_DEVICE and TYPE_USER_CREATABLE support globals */
> +        return;
> +    }
more dynamic casts but now imposed on every create object :(

Maybe we should add ObjectClass::check/set_globals hook?
It would be cheap to check and only objects we intend to work with
it would be affected. On top of that hooks could be different so
that device/user_creatable specifics won't be in generic code
(like it's implemented here).

>  
>      for (l = global_props; l; l = l->next) {
>          GlobalProperty *prop = l->data;
>          Error *err = NULL;
>  
> -        if (object_dynamic_cast(OBJECT(dev), prop->driver) == NULL) {
> +        if (object_dynamic_cast(obj, prop->driver) == NULL) {
>              continue;
>          }
>          prop->used = true;
> -        object_property_parse(OBJECT(dev), prop->value, prop->property, &err);
> +        object_property_parse(obj, prop->value, prop->property, &err);
>          if (err != NULL) {
>              error_prepend(&err, "can't apply global %s.%s=%s: ",
>                            prop->driver, prop->property, prop->value);
> -            if (!dev->hotplugged && prop->errp) {
> +
> +            if (dev && !dev->hotplugged && prop->errp) {
>                  error_propagate(prop->errp, err);
>              } else {
>                  assert(prop->user_provided);
> @@ -56,15 +62,15 @@ int object_property_check_globals(void)
>              continue;
>          }
>          oc = object_class_by_name(prop->driver);
> -        oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
> -        if (!oc) {
> +        dc = (DeviceClass *)object_class_dynamic_cast(oc, TYPE_DEVICE);
> +        if (!IS_USER_CREATABLE_CLASS(oc) && !dc) {
>              warn_report("global %s.%s has invalid class name",
>                          prop->driver, prop->property);
>              ret = 1;
>              continue;
>          }
> -        dc = DEVICE_CLASS(oc);
> -        if (!dc->hotpluggable && !prop->used) {
> +
> +        if (dc && !dc->hotpluggable) {
>              warn_report("global %s.%s=%s not used",
>                          prop->driver, prop->property, prop->value);
>              ret = 1;