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

[Don Slutz <dslutz@verizon.com>]

On 06/06/14 16:14, Eduardo Habkost 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)
>
> 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.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>   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 },

I think the test would be better if this started out as false.

     -Don Slutz

>           { 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)