Re: [Qemu-devel] [PATCH 5/7] qdev: Protect device-list-properties against broken devices

[David Hildenbrand]

> No knowledge should be required for object_new(). Classes' instance_init
> functions should have no side-effects outside the object itself.

While this should theoretically be true, I can guarantee to you that this is
not the case for all devices :) (especially as there are too many unwritten
laws related to that whole qmp/qom thing flying around).

E.g. we can save ourselves from the creation of other devices (init + realize)
by setting a device to !hotpluggable in the device class. This code simply
bypasses such checks and assumes that QEMUs internal structure can deal with
that. We saw that this doesn't work this far.

Other interfaces (object_add) seem to rely on TYPE_USER_CREATABLE, so we can't
trigger "everything" from outside QEMU.

One can argue that we simply have to fix the devices - nevertheless I think this
is the wrong approach to the problem.

> 
> > 
> > Feels a little like hacking an interface to a java program, which allows to
> > create any object of a special kind dynamically (constructor arguments?),
> > reading some member variable (names) via reflections and then throwing that
> > object away. Which sounds very wrong to me.
> 
> I wouldn't call it wrong. Confusing, maybe. We use a mix of class-based
> and prototype-based approaches to inheritance and property
> introspection.
> 
> > 
> > Wonder if it wouldn't make more sense to query only the static properties 
> > of a
> > device. I mean if the dynamic properties are dynamic by definition (and can
> > change during runtime). So looking up the static properties via the typename
> > feels more KIS-style to me. Of course, the relevant properties would have 
> > to be
> > defined statically then, which shouldn't be a problem.
> 
> It depends on your definition of "shouldn't be a problem". :)

Well, it might require some internal rework of that property thingy, hehe ;)

> 
> The static property system is more limited than the QOM dynamic property
> system, today. We already depend on introspection of dynamic properties
> registered by instance_init functions, so we would need to move
> everything to a better static property system if we want to stop doing
> object_new() during class introspection without regressions.
> 
> > 
> > Dynamic properties that are actually created could depend on almost
> > everything in the system - why fake something that we don't know.
> 
> Properties registered on instance_init shouldn't depend on anything else
> on the system. Properties registered later in the object lifetime (e.g.
> on realize) can.
> 

Okay, so if the properties in instance_init should depend on nothing else in
the system, they are always the same for one QEMU version + device.

So maybe it would make sense to define all these "fixed properties" on a device
class level (e.g. via dc->props) and fill them with life afterwards (if we can't
completely specify them at that point). Of course this would need some
rework/careful thinking (e.g. concept of uninitialized properties).

Looking at the errors we get with that approach tells me that it will easily
break again in the future (we need tests for all devices - do we already have
it?) and makes me wonder if a definition on a device class level wouldn't be the
right thing to do - a clean interface description that is valid for each device
instance.

David