On Mon, Sep 21, 2015 at 10:30:50AM +0200, David Hildenbrand wrote:
> > Am 18.09.2015 um 14:00 schrieb Markus Armbruster:
> > > Several devices don't survive object_unref(object_new(T)): they crash
> > > or hang during cleanup, or they leave dangling pointers behind.
> > >
> > > This breaks at least device-list-properties, because
> > > qmp_device_list_properties() needs to create a device to find its
> > > properties. Broken in commit f4eb32b "qmp: show QOM properties in
> > > device-list-properties", v2.1. Example reproducer:
> > >
> > > $ qemu-system-aarch64 -nodefaults -display none -machine none -S -qmp
> > > stdio
> > > {"QMP": {"version": {"qemu": {"micro": 50, "minor": 4, "major": 2},
> > > "package": ""}, "capabilities": []}}
> > > { "execute": "qmp_capabilities" }
> > > {"return": {}}
> > > { "execute": "device-list-properties", "arguments": { "typename":
> > > "pxa2xx-pcmcia" } }
> > > qemu-system-aarch64: /home/armbru/work/qemu/memory.c:1307:
> > > memory_region_finalize: Assertion `((&mr->subregions)->tqh_first ==
> > > ((void *)0))' failed.
> > > Aborted (core dumped)
> > > [Exit 134 (SIGABRT)]
> > >
> > > Unfortunately, I can't fix the problems in these devices right now.
> > > Instead, add DeviceClass member cannot_even_create_with_object_new_yet
> > > to mark them:
> > >
> > > * Crash or hang during cleanup (didn't debug them, so I can't say
> > > why): "pxa2xx-pcmcia", "realview_pci", "versatile_pci",
> > > "s390-sclp-event-facility", "sclp"
> >
> > Ack for the sclp things. Theses devices are created by the machine and
> > sclp creates the event-facility, so not having a way to query properties
> > for these devices is better than a hang.
> >
> > David, can you have a look on why these devices fail as outlined?
> >
>
> The problem was that the quiesce and cpu hotplug sclp event devices had no
> parent (onoly a parent bus). So when the bus (inside the event facility) was
> removed, it looped forever trying remove/unparent it's "bus childs" (which had
> no parent).
>
> sclp (parent=machine)
> -> even-facility (parent=sclp)
> -> bus (parent=event-facility)
> -> quiesce (parent=null)
> -> cpu hotplug (parent=null)
>
> Maybe that helps others struggling with the same symptoms.
>
>
> Just a quick comment on the introspection:
>
> I don't think it is a good idea to expose properties that way. Temporarily
> creating devices for the sake of querying property names sounds very wrong to
> me, because certain devices require certain knowledge about how and when they
> can be created.
No knowledge should be required for object_new(). Classes' instance_init
functions should have no side-effects outside the object itself.
>
> 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". :)
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.
--
Eduardo
