Peter Xu <[email protected]> writes:

> Property itself is a well defined interface to either support smooth
> conversions to Object's properties, or supports global properties.  However
> currently it's tied to DeviceClass, aka, qdev. So non-qdev cannot use
> Property list.
>
> My current observation shows Property is almost ready to be used as a
> separated exported interface, except two small things that may need touch
> up internally:
>
>         qdev_prop_allow_set

Just a thought, we could use something like this to block update of
migration parameters during migration runtime.

>         qdev_prop_check_globals
>
> The 1st one currently checks against realize state (which is part of qdev
> attributes only).  The 2nd one checks for all global property being used in
> all non-pluggable qdevs.
>
> We can loose the check in both spots, keep the check if the driver is a
> qdev, otherwise we can safely whitelist non-qdev use cases of Property.
>
> Signed-off-by: Peter Xu <[email protected]>
> ---
>  hw/core/qdev-properties.c           | 24 ++++++++++++++++++++----
>  tests/unit/test-qdev-global-props.c |  6 +++++-
>  2 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 34d7b26a73..8f870c85fc 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -31,8 +31,14 @@ void qdev_prop_set_after_realize(DeviceState *dev, const 
> char *name,
>  static bool qdev_prop_allow_set(Object *obj, const char *name,
>                                  const PropertyInfo *info, Error **errp)
>  {
> -    DeviceState *dev = DEVICE(obj);
> +    DeviceState *dev;
> +
> +    if (!object_dynamic_cast(obj, TYPE_DEVICE)) {
> +        /* Currently, non-qdev can always set Property anytime */
> +        return true;
> +    }
>  
> +    dev = DEVICE(obj);
>      if (dev->realized && !info->realized_set_allowed) {
>          qdev_prop_set_after_realize(dev, name, errp);
>          return false;
> @@ -997,6 +1003,7 @@ int qdev_prop_check_globals(void)
>  
>      for (i = 0; i < global_props()->len; i++) {
>          GlobalProperty *prop;
> +        bool hotpluggable;
>          ObjectClass *oc;
>          DeviceClass *dc;
>  
> @@ -1005,15 +1012,24 @@ int qdev_prop_check_globals(void)
>              continue;
>          }
>          oc = object_class_by_name(prop->driver);
> -        oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
>          if (!oc) {
>              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) {
> +        oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
> +        if (oc) {
> +            dc = DEVICE_CLASS(oc);
> +            hotpluggable = dc->hotpluggable;
> +        } else {
> +            /*
> +             * Currently, to be strict to assume all non-qdev are not
> +             * hotpluggable (whoever will use -global).
> +             */
> +            hotpluggable = false;
> +        }
> +        if (!hotpluggable && !prop->used) {
>              warn_report("global %s.%s=%s not used",
>                          prop->driver, prop->property, prop->value);
>              ret = 1;
> diff --git a/tests/unit/test-qdev-global-props.c 
> b/tests/unit/test-qdev-global-props.c
> index 8ea362cbb9..86bb1458d9 100644
> --- a/tests/unit/test-qdev-global-props.c
> +++ b/tests/unit/test-qdev-global-props.c
> @@ -275,8 +275,12 @@ static void test_dynamic_globalprop(void)
>      g_test_trap_assert_stderr_unmatched("*prop4*");
>      g_test_trap_assert_stderr(
>          "*warning: global nohotplug-type.prop5=105 not used*");
> +    /*
> +     * TYPE_OBJECT can opt-in for global properties, so the error is "not
> +     * used" too for nondevice-type.
> +     */
>      g_test_trap_assert_stderr(
> -        "*warning: global nondevice-type.prop6 has invalid class name*");
> +        "*warning: global nondevice-type.prop6=106 not used*");
>      g_test_trap_assert_stdout("");
>  }

Reply via email to