Re: [Qemu-devel] [PATCH] qdev: Skip non-existing properties when setting globals

[Igor Mammedov]

On Fri, 6 Jun 2014 17:14:29 -0300
Eduardo Habkost <address@hidden> wrote:

> This avoids QEMU from aborting on cases like this:
> 
>   $ ./install/bin/qemu-system-x86_64 -global cpu.foobar=5
>   qemu-system-x86_64: Property '.foobar' not found
>   Aborted (core dumped)
That is expected behavior.

> 
> The code sets dev->not_used if the property is not found as an effort to
> to allow errors to be reported even if the device is hotpluggable, but
> it won't catch all errors. We can't know the property is not going to be
> available for hotpluggable devices, unless we actually try to create the
> device.
Instead of ignoring users errors, DeviceState should have async_error
field which could be set by device_post_init() instead of aborting and
later device_add could gracefully fail hotadd operation if error is set.

PS:
initfn-s could also reuse this, instead of ignoring errors as they do now.

> 
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
>  hw/core/qdev-properties.c      | 10 +++++++++-
>  tests/test-qdev-global-props.c | 14 ++++++++++++--
>  2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 3d12560..8cd7c2a 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -976,6 +976,7 @@ void qdev_prop_set_globals_for_type(DeviceState *dev, 
> const char *typename,
>                                      Error **errp)
>  {
>      GlobalProperty *prop;
> +    Object *obj = OBJECT(dev);
>  
>      QTAILQ_FOREACH(prop, &global_props, next) {
>          Error *err = NULL;
> @@ -983,8 +984,15 @@ void qdev_prop_set_globals_for_type(DeviceState *dev, 
> const char *typename,
>          if (strcmp(typename, prop->driver) != 0) {
>              continue;
>          }
> +        if (!object_property_find(obj, prop->property, &err)) {
> +            /* not_used doesn't default to true for hotpluggable devices,
> +             * but in this case we know the property wasn't used when it 
> could.
> +             */
> +            prop->not_used = true;
> +            continue;
> +        }
>          prop->not_used = false;
> -        object_property_parse(OBJECT(dev), prop->value, prop->property, 
> &err);
> +        object_property_parse(obj, prop->value, prop->property, &err);
>          if (err != NULL) {
>              error_propagate(errp, err);
>              return;
> diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
> index 2bef04c..1ccc3e5 100644
> --- a/tests/test-qdev-global-props.c
> +++ b/tests/test-qdev-global-props.c
> @@ -148,8 +148,9 @@ static void test_dynamic_globalprop(void)
>  {
>      MyType *mt;
>      static GlobalProperty props[] = {
> -        { TYPE_DYNAMIC_PROPS, "prop1", "101" },
> -        { TYPE_DYNAMIC_PROPS, "prop2", "102" },
> +        { TYPE_DYNAMIC_PROPS, "prop1", "101", true },
> +        { TYPE_DYNAMIC_PROPS, "prop2", "102", true },
> +        { TYPE_DYNAMIC_PROPS, "prop3", "103", true },
>          { TYPE_DYNAMIC_PROPS"-bad", "prop3", "103", true },
>          {}
>      };
> @@ -164,6 +165,15 @@ static void test_dynamic_globalprop(void)
>      g_assert_cmpuint(mt->prop2, ==, 102);
>      all_used = qdev_prop_check_global();
>      g_assert_cmpuint(all_used, ==, 1);
> +
> +    /* prop1 */
> +    g_assert(!props[0].not_used);
> +    /* prop2 */
> +    g_assert(!props[1].not_used);
> +    /* TYPE_DYNAMIC_PROPS.prop3: non-existing property */
> +    g_assert(props[2].not_used);
> +    /* TYPE_DYNAMIC_PROPS-bad.prop3: non-existing class */
> +    g_assert(props[3].not_used);
>  }
>  
>  int main(int argc, char **argv)
> -- 
> 1.9.0
> 
> 


-- 
Regards,
  Igor