Re: [Qemu-devel] [PATCH 09/14] qdev: connect some links and move type to object

[Anthony Liguori]

On 04/19/2012 09:46 AM, Andreas Färber wrote:
> Am 19.04.2012 00:38, schrieb Anthony Liguori:
> > On 04/18/2012 04:25 PM, Andreas Färber wrote:
> > > Am 18.04.2012 22:56, schrieb Anthony Liguori:
> > > > This makes sysbus part of the root hierarchy and all busses children
> > > > of their
> > > > respective parent DeviceState.
> > > >
> > > > Signed-off-by: Anthony Liguori<address@hidden>
> > > > ---
> > > >    hw/qdev.c    |   11 +++++------
> > > >    qom/object.c |   12 ++++++++++++
> > > >    2 files changed, 17 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/hw/qdev.c b/hw/qdev.c
> > > > index 26e6f09..b5eef22 100644
> > > > --- a/hw/qdev.c
> > > > +++ b/hw/qdev.c
> > > > @@ -427,6 +427,7 @@ static void do_qbus_create_inplace(BusState *bus,
> > > > const char *typename,
> > > >        if (parent) {
> > > >            QLIST_INSERT_HEAD(&parent->child_bus, bus, sibling);
> > > >            parent->num_child_bus++;
> > > > +        object_property_add_child(OBJECT(parent), bus->name,
> > > > OBJECT(bus), NULL);
> > > >        } else if (bus != main_system_bus) {
> > > >            /* TODO: once all bus devices are qdevified,
> > > >               only reset handler for main_system_bus should be
> > > > registered here. */
> > > > @@ -456,6 +457,8 @@ static void main_system_bus_create(void)
> > > >        /* assign main_system_bus before qbus_create_inplace()
> > > >         * in order to make "if (bus != main_system_bus)" work */
> > > >        main_system_bus = qbus_create(TYPE_SYSTEM_BUS, NULL,
> > > > "main-system-bus");
> > > > +    object_property_add_child(object_get_root(), "sysbus",
> > > > +                              OBJECT(main_system_bus), NULL);
> > >
> > > So this is adding /sysbus. Shouldn't this rather go into
> > > /unassigned/sysbus?
> >
> > What would sysbus be assigned too?  I think sysbus is a really special
> > case and belongs in the /root directory.
>
> I suggested /unassigned because of no explicit parenting.

Yes, but I view /machine/unattached as a TODO list.  I'd like to completely 
eliminate that hierarchy for PC in 1.2.

But if we stick sysbus in there, it needs to exist forever because there's no 
right place for sysbus to exist.

Maybe we can do something like make i440fx has-a sysbus?  I'm not sure if that 
would break anything but I assume it's a fixable problem.

> Either the
> unstable /unassigned tree or qdev_get_machine() for the real device
> hierarchy. It's a device bus so it should be in one of the device
> containers, not form yet a new root node.
> All devices trace back to some SysBus device currently (e.g., PReP PCI
> host controller is-a SysBusDevice, has-a PCIBus has-a child<PCIDevice>
> i82378 that in turn has-a ISABus etc. etc.), so if we argue that sysbus
> were special and needs to be in /, then *all* devices will end up in
> that special location.
>
> Just wondering, can a node have two parents?

'parent_bus' is just a link.  I'd like to refactor parent_bus out of DeviceState 
and into the appropriate subclasses so that we can introduce stronger typing.

You could certainly have a 'second_parent_bus' link too if you so desired. 
There's really nothing special about it.

> If you add "child[%d]"
> nodes to the busses and we then add i440fx explicitly somewhere, like we
> do, will that remove it from the bus' child[] list or will we have it in
> both locations with an indeterministic canonical path?

The 'child[%d]' properties are links so they don't factor into the canonical 
path.  IOW, the canonical path has nothing to do with the qbus hierarchy.

Regards,

Anthony Liguori

> Andreas