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

[Anthony Liguori]

On 04/19/2012 10:06 AM, Andreas Färber wrote:
> Am 19.04.2012 16:53, schrieb 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 [qdev_get_machine()]/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.
>
> I thought SysBus would go away for 1.2 as well?

For PC devices...

But yeah, okay, as long as we all agree on nuking SysBus in the medium term, 
I'll change the patch to put it in /machine/unattached.

Regards,

Anthony Liguori

 AFAIU SysBus has no real
> bus state and is just a container for devices because qdev requires
> devices to be attached to a bus. SysBusDevices could then become direct
> childs of /machine, and we'd need no special sysbus node at all long-term.
>
> Andreas