[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] test-qdev-global-props: Test global propert
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] test-qdev-global-props: Test global property ordering |
Date: |
Wed, 12 Jul 2017 15:48:05 -0300 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
On Wed, Jul 12, 2017 at 08:06:14PM +0200, Halil Pasic wrote:
>
>
> 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?
-global documentation is already very poor, as you have noticed:
>
> I've checked out the user manual. There we talk about 'driver' but
> I could not find a spot where 'driver' is explained.
Yes. For that reason, it looks like there's nothing to be
updated on the existing (poor) documentation. I will re-read it
to be sure.
>
> 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.
We could, but this wouldn't save us from having to decide what to
do if people are already using those non-abstract superclasses
with -global. e.g.: "-global spapr-pci-host-bridge.FOO=BAR"
(spapr-pci-host-bridge is the parent of spapr-pci-vfio-host-bridge).
This means a new rule like this would necessarily break
command-line compatibility. Probably not a blocker in the
spapr-pci-host-bridge case, but to me it looks like the rule
would cause more problems than it would solve.
>
> Another question is -global with an abstract class seems to be
> accepted on the command line. Is that an undocumented feature?
Considering that we never documented that "driver" could be a
superclass, that's true: it is an undocumented feature.
However, it is a feature people are likely to be relying upon, to
configure a feature on all devices of a given type.
>
> Sorry for expanding the front. I also trying to figure out the design
> for my own benefit.
Your comments and suggestions are very welcome, thanks!
--
Eduardo
- Re: [Qemu-devel] [PATCH 1/3] qdev: fix the order compat and global properties are applied, (continued)
[Qemu-devel] [PATCH 2/3] test-qdev-global-props: Test global property ordering, Eduardo Habkost, 2017/07/10
[Qemu-devel] [PATCH 3/3] Revert "machine: Convert abstract typename on compat_props to subclass names", Eduardo Habkost, 2017/07/10