Re: [Qemu-devel] [PATCH 08/14] qdev: convert busses to QEMU Object Model

[Anthony Liguori]

On 05/01/2012 02:31 PM, Andreas Färber wrote:
> Am 01.05.2012 20:18, schrieb Anthony Liguori:
> > This is far less interesting than it sounds.  We simply add an Object to each
> > BusInfo and then register the types appropriately.  Most of the interesting
> > refactoring will follow in the next patches.
> >
> > Since we're changing fundamental type names (BusInfo ->  BusClass), it all needs
> > to convert at once.  Fortunately, not a lot of code is affected.
> >
> > Signed-off-by: Anthony Liguori<address@hidden>
> > ---
> [...]
> > diff --git a/hw/intel-hda.c b/hw/intel-hda.c
> > index 9405f54..58497e5 100644
> > --- a/hw/intel-hda.c
> > +++ b/hw/intel-hda.c
> > @@ -29,16 +29,19 @@
> >   /* --------------------------------------------------------------------- */
> >   /* hda bus                                                               */
> >
> > -static struct BusInfo hda_codec_bus_info = {
> > -    .name      = "HDA",
> > -    .size      = sizeof(HDACodecBus),
> > +#define TYPE_HDA_BUS "HDA"
>
> I stumbled over this being a pretty generic term for a type name.
> "HDA-codec-bus" maybe?

It's unfortunately part of the ABI, so it cannot be changed :-(

> > diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> > index f5ee166..97673dd 100644
> > --- a/hw/qdev-monitor.c
> > +++ b/hw/qdev-monitor.c
> [...]
> > @@ -432,16 +433,16 @@ DeviceState *qdev_device_add(QemuOpts *opts)
> >           if (!bus) {
> >               return NULL;
> >           }
> > -        if (bus->info != k->bus_info) {
> > +        if (strcmp(object_get_typename(OBJECT(bus)), k->bus_type) != 0){
>
> Space before {.

Ack.

> >   struct BusState {
> > +    Object obj;
> >       DeviceState *parent;
> > -    BusInfo *info;
> >       const char *name;
> >       int allow_hotplug;
> >       int qdev_allocated;
>
> Indicate what is /*<  private>*/ and what not?

Ack.

> > @@ -321,7 +323,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, 
> > DeviceState *dev,
> >   char *qdev_get_fw_dev_path(DeviceState *dev);
> >
> >   /* This is a nasty hack to allow passing a NULL bus to qdev_create.  */
> > -extern struct BusInfo system_bus_info;
> > +#define TYPE_SYSTEM_BUS "System"
>
> "System-bus"?
>
> Regarding the bus names, these have been merely moved here but I'm
> raising the topic because they are now in the global name space where
> they will potentially conflict with devices and other types.

The problem is they are already part of our existing ABI.  If we change it, it 
will break existing bus ids and qdev paths.

Regards,

Anthony Liguori