Re: [Qemu-devel] [PATCH 2/3] test-qdev-global-props: Test global property ordering

[Halil Pasic]

On 07/11/2017 02:43 AM, Eduardo Habkost wrote:
> Test case to detect the bug fixed by commit
> "qdev: fix the order compat and global properties are applied".
> 
> Signed-off-by: Eduardo Habkost <address@hidden>

Reviewed-by: Halil Pasic <address@hidden>

Please see below nevertheless.

> ---
>  tests/test-qdev-global-props.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
> index 48e5b73..ef2951f 100644
> --- a/tests/test-qdev-global-props.c
> +++ b/tests/test-qdev-global-props.c
> @@ -33,6 +33,8 @@
>  #define STATIC_TYPE(obj) \
>      OBJECT_CHECK(MyType, (obj), TYPE_STATIC_PROPS)
> 
> +#define TYPE_SUBCLASS "static_prop_subtype"
> +
>  #define PROP_DEFAULT 100
> 
>  typedef struct MyType {
> @@ -63,6 +65,11 @@ static const TypeInfo static_prop_type = {
>      .class_init = static_prop_class_init,
>  };
> 
> +static const TypeInfo subclass_type = {
> +    .name = TYPE_SUBCLASS,
> +    .parent = TYPE_STATIC_PROPS,
> +};
> +
>  /* Test simple static property setting to default value */
>  static void test_static_prop_subprocess(void)
>  {
> @@ -279,12 +286,35 @@ static void test_dynamic_globalprop_nouser(void)
>      g_test_trap_assert_stdout("");
>  }
> 
> +/* Test if global props affecting subclasses are applied in the right order 
> */

Now that I think about it this is affecting an  external and
(end-)user facing interface. You say this is the right order.
Based on what is this the right order? Do we need to update some
documentation too?

I've checked out the user manual. There we talk about 'driver' but
I could not find a spot where 'driver' is explained.

If I would have to translate between user manual terminology and
code terminology, I would say a driver is a not abstract class
inheriting from device. If that's right I wonder if it should
be allowed for a non-abstract class inheriting from device to
inherit form another non-abstract class.

Another question is -global with an abstract class seems to be
accepted on the command line. Is that an undocumented feature?

Sorry for expanding the front. I also trying to figure out the design
for my own benefit.

Regards,
Halil 


> +static void test_subclass_global_props(void)
> +{
> +    MyType *mt;
> +    /* Global properties must be applied in the order they were registered */
> +    static GlobalProperty props[] = {
> +        { TYPE_STATIC_PROPS, "prop1", "101" },
> +        { TYPE_SUBCLASS,     "prop1", "102" },
> +        { TYPE_SUBCLASS,     "prop2", "103" },
> +        { TYPE_STATIC_PROPS, "prop2", "104" },
> +        {}
> +    };
> +
> +    qdev_prop_register_global_list(props);
> +
> +    mt = STATIC_TYPE(object_new(TYPE_SUBCLASS));
> +    qdev_init_nofail(DEVICE(mt));
> +
> +    g_assert_cmpuint(mt->prop1, ==, 102);
> +    g_assert_cmpuint(mt->prop2, ==, 104);
> +}
> +
>  int main(int argc, char **argv)
>  {
>      g_test_init(&argc, &argv, NULL);
> 
>      module_call_init(MODULE_INIT_QOM);
>      type_register_static(&static_prop_type);
> +    type_register_static(&subclass_type);
>      type_register_static(&dynamic_prop_type);
>      type_register_static(&hotplug_type);
>      type_register_static(&nohotplug_type);
> @@ -310,6 +340,9 @@ int main(int argc, char **argv)
>      g_test_add_func("/qdev/properties/dynamic/global/nouser",
>                      test_dynamic_globalprop_nouser);
> 
> +    g_test_add_func("/qdev/properties/global/subclass",
> +                    test_subclass_global_props);
> +
>      g_test_run();
> 
>      return 0;
>