[Qemu-devel] [PATCH 0/14] qom: convert busses to QOM (v2)

[Qemu-devel] [PATCH 0/14] qom: convert busses to QOM (v2)

[Anthony Liguori]

This is the last of the core QOM series.  This series converts busses to QOM
using a model where busses are proper objects that inherit from Object directly.
Devices have a has-a relationship with any bus they implement.

This series also creates link associated with the device/bus relationships.  All
devices have a parent_bus link that can be (optionally) set to their parent_bus
property.  This link is typed as BusState.  Over time, I'd like to refactor
this to a subclass added property with a stronger type.  For instance, PCIDevice
would have a parent:link<PCIBus> property.

Busses also have links to their children.  These are anonymous/unstable names.
Long term, I'd like to move to having stable names based on bus specific
information.  For instance, PCI busses ought to use a 'slot[0.0]' naming
convention.

I've tested this series pretty extensively.  It should be clean except for the
one patch that temporarily breaks and then fixes info qdm/qtree.
---
v1 -> v2
 - Move sysbus to /machine/unattached/sysbus (Andreas)
 - Rebase

 exec.c                        |    4 
 hw/acpi_piix4.c               |   10 +
 hw/i2c.c                      |   34 +++--
 hw/ide/qdev.c                 |   55 +++++----
 hw/intel-hda.c                |   44 ++++---
 hw/isa-bus.c                  |   75 ++++++------
 hw/lsi53c895a.c               |    5 
 hw/pci-hotplug.c              |    6 -
 hw/pci.c                      |  221 +++++++++++++++++++------------------
 hw/pci_bridge.c               |    2 
 hw/pci_internals.h            |    2 
 hw/qdev-monitor.c             |  177 +++++++++++++++++-------------
 hw/qdev-properties.c          |   33 +----
 hw/qdev.c                     |  247 ++++++++++++++++++++++++++++--------------
 hw/qdev.h                     |   53 +++++----
 hw/s390-virtio-bus.c          |   39 +++---
 hw/scsi-bus.c                 |   80 +++++++------
 hw/scsi.h                     |    4 
 hw/spapr_pci.c                |    7 -
 hw/spapr_vio.c                |   56 +++++----
 hw/spapr_vty.c                |    6 -
 hw/ssi.c                      |   28 ++--
 hw/sysbus.c                   |   81 ++++++-------
 hw/usb/bus.c                  |  158 ++++++++++++++------------
 hw/usb/dev-smartcard-reader.c |   29 +++-
 hw/virtio-scsi.c              |    6 -
 hw/virtio-serial-bus.c        |   55 +++++----
 include/qemu/object.h         |   26 ++++
 qom/object.c                  |   33 +++++
 savevm.c                      |   12 +-
 30 files changed, 933 insertions(+), 655 deletions(-)

[Qemu-devel] [PATCH 01/14] qdev: fix adding of ptr properties

[Anthony Liguori]

ptr properties have neither a get/set or a print/parse which means that when
they're added they aren't treated as static or legacy properties.

Just assume properties like this are legacy properties and treat them as such.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 0bcde20..6a8f6bd 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -576,9 +576,12 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop,
 {
     gchar *name, *type;
 
-    if (!prop->info->print && !prop->info->parse) {
+    /* Register pointer properties as legacy properties */
+    if (!prop->info->print && !prop->info->parse &&
+        (prop->info->set || prop->info->get)) {
         return;
     }
+
     name = g_strdup_printf("legacy-%s", prop->name);
     type = g_strdup_printf("legacy<%s>",
                            prop->info->legacy_name ?: prop->info->name);
-- 
1.7.5.4

[Qemu-devel] [PATCH 02/14] object: add object_property_foreach

[Anthony Liguori]

Provide a mechanism to walk through each property for an object.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 include/qemu/object.h |   26 ++++++++++++++++++++++++++
 qom/object.c          |   10 ++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/include/qemu/object.h b/include/qemu/object.h
index ca1649c..4ad6758 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -915,5 +915,31 @@ void object_property_add_str(Object *obj, const char *name,
  */
 Object *container_get(Object *root, const char *path);
 
+/**
+ * ObjectPropertyEnumerator:
+ * @obj: the object the property belongs to
+ * @name: the name of the property
+ * @typename: the string typename the property was created with
+ * @read_only: if #true, the property is read-only
+ * @opaque: the opaque handle passed to @object_property_foreach
+ *
+ * Callback function type for @object_property_foreach
+ */
+typedef void (ObjectPropertyEnumerator)(Object *obj,
+                                        const char *name,
+                                        const char *typename,
+                                        bool read_only,
+                                        void *opaque);
+
+/**
+ * object_property_foreach:
+ * @path: object to enumerate properties in
+ * @fn: callback function that will be invoked for each property
+ * @opaque: opaque to pass to @fn
+ *
+ * Enumerate all properties in an existing object.
+ */
+void object_property_foreach(Object *obj, ObjectPropertyEnumerator *fn,
+                             void *opaque);
 
 #endif
diff --git a/qom/object.c b/qom/object.c
index e721fc2..94928c5 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1194,3 +1194,13 @@ void object_property_add_str(Object *obj, const char *name,
                         property_release_str,
                         prop, errp);
 }
+
+void object_property_foreach(Object *obj, ObjectPropertyEnumerator *fn,
+                             void *opaque)
+{
+    ObjectProperty *prop;
+
+    QTAILQ_FOREACH(prop, &obj->properties, node) {
+        fn(obj, prop->name, prop->type, !prop->set, opaque);
+    }
+}
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 02/14] object: add object_property_foreach

[Andreas Färber]

Am 01.05.2012 20:18, schrieb Anthony Liguori:
> Provide a mechanism to walk through each property for an object.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Reviewed-by: Andreas Färber <afaerber@suse.de>

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Anthony Liguori]

This allows a base class to easily add properties.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev.c |   25 ++++++++++++-------------
 hw/qdev.h |    2 ++
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 6a8f6bd..e17a9ab 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -76,22 +76,26 @@ bool qdev_exists(const char *name)
 static void qdev_property_add_legacy(DeviceState *dev, Property *prop,
                                      Error **errp);
 
-void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
+void qdev_add_properties(DeviceState *dev, Property *props)
 {
     Property *prop;
 
+    for (prop = props; prop && prop->name; prop++) {
+        qdev_property_add_legacy(dev, prop, NULL);
+        qdev_property_add_static(dev, prop, NULL);
+    }
+    qdev_prop_set_defaults(dev, props);
+}
+
+void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
+{
     if (qdev_hotplug) {
         assert(bus->allow_hotplug);
     }
 
     dev->parent_bus = bus;
     QTAILQ_INSERT_HEAD(&bus->children, dev, sibling);
-
-    for (prop = qdev_get_bus_info(dev)->props; prop && prop->name; prop++) {
-        qdev_property_add_legacy(dev, prop, NULL);
-        qdev_property_add_static(dev, prop, NULL);
-    }
-    qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
+    qdev_add_properties(dev, dev->parent_bus->info->props);
 }
 
 /* Create a new device.  This only initializes the device state structure
@@ -633,13 +637,8 @@ static void device_initfn(Object *obj)
     dev->instance_id_alias = -1;
     dev->state = DEV_STATE_CREATED;
 
-    for (prop = qdev_get_props(dev); prop && prop->name; prop++) {
-        qdev_property_add_legacy(dev, prop, NULL);
-        qdev_property_add_static(dev, prop, NULL);
-    }
-
+    qdev_add_properties(dev, qdev_get_props(dev));
     object_property_add_str(OBJECT(dev), "type", qdev_get_type, NULL, NULL);
-    qdev_prop_set_defaults(dev, qdev_get_props(dev));
 }
 
 /* Unlink device from bus and free the structure.  */
diff --git a/hw/qdev.h b/hw/qdev.h
index 4e90119..ca8386a 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -360,4 +360,6 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
 
 extern int qdev_hotplug;
 
+void qdev_add_properties(DeviceState *dev, Property *props);
+
 #endif
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Andreas Färber]

Am 01.05.2012 20:18, schrieb Anthony Liguori:
> This allows a base class to easily add properties.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Implementation looks okay but /me not so happy with it: This conflicts
with the move of the qdev static property infrastructure from
DeviceState to Object.

Consider rebasing this onto part of Paolo's series and call it
object_add_properties?

Andreas

> ---
>  hw/qdev.c |   25 ++++++++++++-------------
>  hw/qdev.h |    2 ++
>  2 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 6a8f6bd..e17a9ab 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -76,22 +76,26 @@ bool qdev_exists(const char *name)
>  static void qdev_property_add_legacy(DeviceState *dev, Property *prop,
>                                       Error **errp);
>  
> -void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
> +void qdev_add_properties(DeviceState *dev, Property *props)
>  {
>      Property *prop;
>  
> +    for (prop = props; prop && prop->name; prop++) {
> +        qdev_property_add_legacy(dev, prop, NULL);
> +        qdev_property_add_static(dev, prop, NULL);
> +    }
> +    qdev_prop_set_defaults(dev, props);
> +}
> +
> +void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
> +{
>      if (qdev_hotplug) {
>          assert(bus->allow_hotplug);
>      }
>  
>      dev->parent_bus = bus;
>      QTAILQ_INSERT_HEAD(&bus->children, dev, sibling);
> -
> -    for (prop = qdev_get_bus_info(dev)->props; prop && prop->name; prop++) {
> -        qdev_property_add_legacy(dev, prop, NULL);
> -        qdev_property_add_static(dev, prop, NULL);
> -    }
> -    qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
> +    qdev_add_properties(dev, dev->parent_bus->info->props);
>  }
>  
>  /* Create a new device.  This only initializes the device state structure
> @@ -633,13 +637,8 @@ static void device_initfn(Object *obj)
>      dev->instance_id_alias = -1;
>      dev->state = DEV_STATE_CREATED;
>  
> -    for (prop = qdev_get_props(dev); prop && prop->name; prop++) {
> -        qdev_property_add_legacy(dev, prop, NULL);
> -        qdev_property_add_static(dev, prop, NULL);
> -    }
> -
> +    qdev_add_properties(dev, qdev_get_props(dev));
>      object_property_add_str(OBJECT(dev), "type", qdev_get_type, NULL, NULL);
> -    qdev_prop_set_defaults(dev, qdev_get_props(dev));
>  }
>  
>  /* Unlink device from bus and free the structure.  */
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 4e90119..ca8386a 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -360,4 +360,6 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
>  
>  extern int qdev_hotplug;
>  
> +void qdev_add_properties(DeviceState *dev, Property *props);
> +
>  #endif


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Anthony Liguori]

On 05/01/2012 02:05 PM, Andreas Färber wrote:
> Am 01.05.2012 20:18, schrieb Anthony Liguori:
>> This allows a base class to easily add properties.
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>
> Implementation looks okay but /me not so happy with it: This conflicts
> with the move of the qdev static property infrastructure from
> DeviceState to Object.
>
> Consider rebasing this onto part of Paolo's series and call it
> object_add_properties?

Eh?  There's nothing object_ about these properties and there's no way I'm 
willing to put legacy properties in object...

So I'm not quite sure what you're suggesting.

Regards,

Anthony Liguori

>
> Andreas
>
>> ---
>>   hw/qdev.c |   25 ++++++++++++-------------
>>   hw/qdev.h |    2 ++
>>   2 files changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index 6a8f6bd..e17a9ab 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -76,22 +76,26 @@ bool qdev_exists(const char *name)
>>   static void qdev_property_add_legacy(DeviceState *dev, Property *prop,
>>                                        Error **errp);
>>
>> -void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
>> +void qdev_add_properties(DeviceState *dev, Property *props)
>>   {
>>       Property *prop;
>>
>> +    for (prop = props; prop&&  prop->name; prop++) {
>> +        qdev_property_add_legacy(dev, prop, NULL);
>> +        qdev_property_add_static(dev, prop, NULL);
>> +    }
>> +    qdev_prop_set_defaults(dev, props);
>> +}
>> +
>> +void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
>> +{
>>       if (qdev_hotplug) {
>>           assert(bus->allow_hotplug);
>>       }
>>
>>       dev->parent_bus = bus;
>>       QTAILQ_INSERT_HEAD(&bus->children, dev, sibling);
>> -
>> -    for (prop = qdev_get_bus_info(dev)->props; prop&&  prop->name; prop++) {
>> -        qdev_property_add_legacy(dev, prop, NULL);
>> -        qdev_property_add_static(dev, prop, NULL);
>> -    }
>> -    qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
>> +    qdev_add_properties(dev, dev->parent_bus->info->props);
>>   }
>>
>>   /* Create a new device.  This only initializes the device state structure
>> @@ -633,13 +637,8 @@ static void device_initfn(Object *obj)
>>       dev->instance_id_alias = -1;
>>       dev->state = DEV_STATE_CREATED;
>>
>> -    for (prop = qdev_get_props(dev); prop&&  prop->name; prop++) {
>> -        qdev_property_add_legacy(dev, prop, NULL);
>> -        qdev_property_add_static(dev, prop, NULL);
>> -    }
>> -
>> +    qdev_add_properties(dev, qdev_get_props(dev));
>>       object_property_add_str(OBJECT(dev), "type", qdev_get_type, NULL, NULL);
>> -    qdev_prop_set_defaults(dev, qdev_get_props(dev));
>>   }
>>
>>   /* Unlink device from bus and free the structure.  */
>> diff --git a/hw/qdev.h b/hw/qdev.h
>> index 4e90119..ca8386a 100644
>> --- a/hw/qdev.h
>> +++ b/hw/qdev.h
>> @@ -360,4 +360,6 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
>>
>>   extern int qdev_hotplug;
>>
>> +void qdev_add_properties(DeviceState *dev, Property *props);
>> +
>>   #endif
>
>

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Andreas Färber]

Am 01.05.2012 22:37, schrieb Anthony Liguori:
> On 05/01/2012 02:05 PM, Andreas Färber wrote:
>> Am 01.05.2012 20:18, schrieb Anthony Liguori:
>>> This allows a base class to easily add properties.
>>>
>>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>>
>> Implementation looks okay but /me not so happy with it: This conflicts
>> with the move of the qdev static property infrastructure from
>> DeviceState to Object.
>>
>> Consider rebasing this onto part of Paolo's series and call it
>> object_add_properties?
> 
> Eh?  There's nothing object_ about these properties and there's no way
> I'm willing to put legacy properties in object...
> 
> So I'm not quite sure what you're suggesting.

You just suggested to Peter using qdev_add_properties() in new QOM ARM
classes of his.

I'd rather not propagate using qdev_* functions in new QOM code because
either it remains forever or renaming becomes another touch-all series.

In Paolo's series the Property* concept is moved from qdev to QOM; thus
if it's in Object we usually use an object_ prefix.
Not too long ago you were willing to merge the large part of Paolo's
series which included this code movement, so if you don't want that
after all you should communicate that openly as a reply there. :)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Anthony Liguori]

On 05/01/2012 03:43 PM, Andreas Färber wrote:
> Am 01.05.2012 22:37, schrieb Anthony Liguori:
>> On 05/01/2012 02:05 PM, Andreas Färber wrote:
>>> Am 01.05.2012 20:18, schrieb Anthony Liguori:
>>>> This allows a base class to easily add properties.
>>>>
>>>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>>>
>>> Implementation looks okay but /me not so happy with it: This conflicts
>>> with the move of the qdev static property infrastructure from
>>> DeviceState to Object.
>>>
>>> Consider rebasing this onto part of Paolo's series and call it
>>> object_add_properties?
>>
>> Eh?  There's nothing object_ about these properties and there's no way
>> I'm willing to put legacy properties in object...
>>
>> So I'm not quite sure what you're suggesting.
>
> You just suggested to Peter using qdev_add_properties() in new QOM ARM
> classes of his.
>
> I'd rather not propagate using qdev_* functions in new QOM code because
> either it remains forever or renaming becomes another touch-all series.
>
> In Paolo's series the Property* concept is moved from qdev to QOM; thus
> if it's in Object we usually use an object_ prefix.
> Not too long ago you were willing to merge the large part of Paolo's
> series which included this code movement, so if you don't want that
> after all you should communicate that openly as a reply there. :)

Legacy properties != static properties.

qdev_add_properties adds both legacy and static properties.  I'm happy to put 
static properties into object but not legacy properties.

So qdev_add_properties is going to stick around even after Paolo's changes.

Base classes are free to call object_property_add_static directly and avoid 
qdev_add_properties entirely but we need the later for backwards compat.

Regards,

Anthony Liguori

>
> Andreas
>

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Peter Maydell]

On 1 May 2012 21:48, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Legacy properties != static properties.
>
> qdev_add_properties adds both legacy and static properties.  I'm happy to
> put static properties into object but not legacy properties.

So, er, how are you defining legacy and static properties?
I would have thought 'legacy property' was 'anything we happen
to already have'...

> Base classes are free to call object_property_add_static directly and avoid
> qdev_add_properties entirely but we need the later for backwards compat.

If qdev_add_properties is purely for back-compat it should be
clearly labelled so we don't use it in new code.

-- PMM

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Anthony Liguori]

On 05/01/2012 03:57 PM, Peter Maydell wrote:
> On 1 May 2012 21:48, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>> Legacy properties != static properties.
>>
>> qdev_add_properties adds both legacy and static properties.  I'm happy to
>> put static properties into object but not legacy properties.
>
> So, er, how are you defining legacy and static properties?
> I would have thought 'legacy property' was 'anything we happen
> to already have'...

No, per the naming convention Paolo introduced, it's roughly:

1) legacy properties can be get/set as strings using the qdev parsing methods. 
Via qdev, they're exposed as -device <type>,<prop-name>=<string>.

In QOM, they're exposed as:

/path/to/device/legacy-<prop-name> = <string>

2) static properties are well-behaved QOM properties that just happen to be 
defined by the DEFINE_PROP_XXX macros.  They use the get/set methods of the 
property types.  New code can (and probably should) make use of static properties.

There's magic in the qdev layer now to decide whether a Property in the array of 
properties becomes a legacy or static property (it's only ever exposed as one 
type).  When we move static properties to Object, I'd like to keep legacy 
strictly as a qdev concept so we don't accidentally introduce more legacy 
properties.

>> Base classes are free to call object_property_add_static directly and avoid
>> qdev_add_properties entirely but we need the later for backwards compat.
>
> If qdev_add_properties is purely for back-compat it should be
> clearly labelled so we don't use it in new code.

Yes, I expect it would be once we have Paolo's property series in.  For now, 
it's the only real interface we have for adding bulk properties.

Regards,

Anthony Liguori

>
> -- PMM
>

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Paolo Bonzini]

Il 02/05/2012 00:01, Anthony Liguori ha scritto:
> 
> There's magic in the qdev layer now to decide whether a Property in the
> array of properties becomes a legacy or static property (it's only ever
> exposed as one type).

I don't think this is true: a legacy property always has a static
counterpart.  The magic (really just a fallback) is to decide whether
-device <type>,<prop>=<value> will use the legacy property or a string
visitor + a static property.

Paolo

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Anthony Liguori]

On 05/01/2012 05:12 PM, Paolo Bonzini wrote:
> Il 02/05/2012 00:01, Anthony Liguori ha scritto:
>>
>> There's magic in the qdev layer now to decide whether a Property in the
>> array of properties becomes a legacy or static property (it's only ever
>> exposed as one type).
>
> I don't think this is true: a legacy property always has a static
> counterpart.  The magic (really just a fallback) is to decide whether
> -device<type>,<prop>=<value>  will use the legacy property or a string
> visitor + a static property.

Yes, I mispoke.  All legacy properties are static properties but not all static 
properties are legacy (IIUC).

The exception is PROP_PTR which is always a legacy but never a static.

So there is quite a bit of magic.

Regards,

Anthony Liguori

>
> Paolo
>

[Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Anthony Liguori]

This is technically a compatibility breaker.  However:

1) libvirt does not rely on this (it always uses the driver name)

2) This behavior isn't actually documented anywhere (the docs just say driver).

3) I suspect there are less than three people on earth that even know this is
   possible (minus the people reading this message).

So I think we can safely break it :-)

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev-properties.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 98dd06a..894fa79 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -1172,8 +1172,7 @@ void qdev_prop_set_globals(DeviceState *dev)
     GlobalProperty *prop;
 
     QTAILQ_FOREACH(prop, &global_props, next) {
-        if (strcmp(object_get_typename(OBJECT(dev)), prop->driver) != 0 &&
-            strcmp(qdev_get_bus_info(dev)->name, prop->driver) != 0) {
+        if (strcmp(object_get_typename(OBJECT(dev)), prop->driver) != 0) {
             continue;
         }
         if (qdev_prop_parse(dev, prop->property, prop->value) != 0) {
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Paolo Bonzini]

Il 01/05/2012 20:18, Anthony Liguori ha scritto:
> This is technically a compatibility breaker.  However:
> 
> 1) libvirt does not rely on this (it always uses the driver name)
> 
> 2) This behavior isn't actually documented anywhere (the docs just say driver).
> 
> 3) I suspect there are less than three people on earth that even know this is
>    possible (minus the people reading this message).
> 
> So I think we can safely break it :-)

Does this work with compat properties set on a bus?  Even if it does,
perhaps it's better to avoid the cleverness and wait for my series that
moves bus properties to the appropriate abstract superclass.

Paolo

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Anthony Liguori]

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]

On 05/01/2012 03:37 PM, Paolo Bonzini wrote:
> Il 01/05/2012 20:18, Anthony Liguori ha scritto:
>> This is technically a compatibility breaker.  However:
>>
>> 1) libvirt does not rely on this (it always uses the driver name)
>>
>> 2) This behavior isn't actually documented anywhere (the docs just say driver).
>>
>> 3) I suspect there are less than three people on earth that even know this is
>>     possible (minus the people reading this message).
>>
>> So I think we can safely break it :-)
>
> Does this work with compat properties set on a bus?

No :-(

This is pretty easy to fix though.  The attached should do the trick, I'll 
update and send out.

   Even if it does,
> perhaps it's better to avoid the cleverness and wait for my series that
> moves bus properties to the appropriate abstract superclass.

This series does that FWIW (patch 6/14).

Regards,

Anthony Liguori

>
> Paolo
>


[-- Attachment #2: foo --]
[-- Type: text/plain, Size: 1409 bytes --]

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 6a75718..be809db 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -378,7 +378,7 @@ static QEMUMachine pc_machine_v1_1 = {
             .property = "vapic",\
             .value    = "off",\
         },{\
-            .driver   = "USB",\
+            .driver   = TYPE_USB_DEVICE,\
             .property = "full-path",\
             .value    = "no",\
         }
@@ -451,7 +451,7 @@ static QEMUMachine pc_machine_v0_14 = {
 #define PC_COMPAT_0_13 \
         PC_COMPAT_0_14,\
         {\
-            .driver   = "PCI",\
+            .driver   = TYPE_PCI_DEVICE,\
             .property = "command_serr_enable",\
             .value    = "off",\
         },{\
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 0c6dade..ddd7b25 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -1158,10 +1158,14 @@ void qdev_prop_set_globals(DeviceState *dev)
     GlobalProperty *prop;
 
     QTAILQ_FOREACH(prop, &global_props, next) {
-        if (strcmp(object_get_typename(OBJECT(dev)), prop->driver) != 0) {
+        Object *obj;
+
+        obj = object_dynamic_cast(OBJECT(dev), prop->driver);
+        if (obj == NULL) {
             continue;
         }
-        if (qdev_prop_parse(dev, prop->property, prop->value) != 0) {
+
+        if (qdev_prop_parse(DEVICE(obj), prop->property, prop->value) != 0) {
             exit(1);
         }
     }

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Paolo Bonzini]

Il 01/05/2012 22:46, Anthony Liguori ha scritto:
>>>
>>>
>>> So I think we can safely break it :-)
>>
>> Does this work with compat properties set on a bus?
> 
> No :-(
> 
> This is pretty easy to fix though.  The attached should do the trick,
> I'll update and send out.
> 
>   Even if it does,
>> perhaps it's better to avoid the cleverness and wait for my series that
>> moves bus properties to the appropriate abstract superclass.
> 
> This series does that FWIW (patch 6/14).

Yeah, it should---modulo bisectability of course.

It's a fairly different approach WRT my series (using
qdev_add_properties instead of klass->props).  In theory I like it, but
I fail to see right now whether it breaks "-device foo,?" somehow.  I
think it does:

$ qemu-system-x86_64 -device rtl8139,?
rtl8139.mac=macaddr
rtl8139.vlan=vlan
rtl8139.netdev=netdev
rtl8139.bootindex=int32
rtl8139.addr=pci-devfn                   <<< here start bus props
rtl8139.romfile=string
rtl8139.rombar=uint32
rtl8139.multifunction=on/off
rtl8139.command_serr_enable=on/off

I think it's too late for this series to go into 1.1.

Paolo

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Andreas Färber]

Am 01.05.2012 23:47, schrieb Paolo Bonzini:
> I think it's too late for this series to go into 1.1.

In that case this is calling for a qom-next branch...

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Anthony Liguori]

On 05/01/2012 05:18 PM, Andreas Färber wrote:
> Am 01.05.2012 23:47, schrieb Paolo Bonzini:
>> I think it's too late for this series to go into 1.1.
>
> In that case this is calling for a qom-next branch...

I really prefer to do this for 1.1.  We've got a large enough testing window and 
these patches have been around for a while.

Regards,

Anthony Liguori

>
> Andreas
>

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Anthony Liguori]

On 05/01/2012 04:47 PM, Paolo Bonzini wrote:
> Il 01/05/2012 22:46, Anthony Liguori ha scritto:
>>>>
>>>>
>>>> So I think we can safely break it :-)
>>>
>>> Does this work with compat properties set on a bus?
>>
>> No :-(
>>
>> This is pretty easy to fix though.  The attached should do the trick,
>> I'll update and send out.
>>
>>    Even if it does,
>>> perhaps it's better to avoid the cleverness and wait for my series that
>>> moves bus properties to the appropriate abstract superclass.
>>
>> This series does that FWIW (patch 6/14).
>
> Yeah, it should---modulo bisectability of course.
>
> It's a fairly different approach WRT my series (using
> qdev_add_properties instead of klass->props).  In theory I like it, but
> I fail to see right now whether it breaks "-device foo,?" somehow.  I
> think it does:
>
> $ qemu-system-x86_64 -device rtl8139,?
> rtl8139.mac=macaddr
> rtl8139.vlan=vlan
> rtl8139.netdev=netdev
> rtl8139.bootindex=int32
> rtl8139.addr=pci-devfn<<<  here start bus props
> rtl8139.romfile=string
> rtl8139.rombar=uint32
> rtl8139.multifunction=on/off
> rtl8139.command_serr_enable=on/off
>
> I think it's too late for this series to go into 1.1.

No, this all works as expected (well, almost):

$ qemu-system-x86_64 -device rtl8139,?
rtl8139.vlan=vlan
rtl8139.addr=pci-devfn
rtl8139.romfile=string
rtl8139.multifunction=on/off
rtl8139.command_serr_enable=on/off

But there is stuff missing...

The problem is that qdev_device_help() only prints out legacy properties and 
munges them to look the way they used to.  But when you refactored some of the 
legacy types to be non-legacy, it meant that the legacy- version disappeared and 
those options disappeared from -device <type>,?

So we need to fix this either way.  I think the simple solution is to store 
property info in a list instead of directly printing it.  Then we can walk the 
list and remove non-legacy properties for given legacy properties and munge the 
names in place.

But this is true even with master (but much, much worse):

anthony@titi:~/build/qemu$ x86_64-softmmu/qemu-system-x86_64 -device rtl8139,?
<there is no output>

This is because we only look at DeviceClass::props which only contains what 
happens to only contain static properties for rtl8139.  We totally ignore static 
properties in current master.

So my series makes the situation better and I think it's easier to fix the full 
problem.  I also don't view the current bug as a -rc0 blocker (although it's 
obviously a release blocker).  I can send a proper patch later in the week but 
I'd still like to commit the bus changes before -rc0.

We have a month before 1.1.  I think that's plenty of time to address any fall 
out here..

Regards,

Anthony Liguori

>
> Paolo
>

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Paolo Bonzini]

Il 02/05/2012 00:18, Anthony Liguori ha scritto:
> anthony@titi:~/build/qemu$ x86_64-softmmu/qemu-system-x86_64 -device
> rtl8139,?
> <there is no output>

I don't think this is a fair comparison, or makes sense at all.  The cause
of the bug in master is a cut-and-paste typo:

@@ -157,7 +157,7 @@ int qdev_device_help(QemuOpts *opts)
           * for removal.  This conditional should be removed along with
           * it.
           */
-        if (!prop->info->parse) {
+        if (!prop->info->set) {
              continue;           /* no way to set it, don't show */
          }
          error_printf("%s.%s=%s\n", driver, prop->name,
@@ -165,7 +165,7 @@ int qdev_device_help(QemuOpts *opts)
      }
      if (info->bus_info) {
          for (prop = info->bus_info->props; prop&&  prop->name; prop++) {
-            if (!prop->info->parse) {
+            if (!prop->info->set) {
                  continue;           /* no way to set it, don't show */
              }
              error_printf("%s.%s=%s\n", driver, prop->name,

while here the problem is due to a half-baked change in this series.

Since I redid the same change in my properties series, and I did it
correctly, the only sensible solution is to rebase these patches on
that one.

> So my series makes the situation better and I think it's easier to fix
> the full problem.  I also don't view the current bug as a -rc0 blocker
> (although it's obviously a release blocker).  I can send a proper patch
> later in the week but I'd still like to commit the bus changes before -rc0.

Please don't.  I have barely started reviewing this series and I have
quite a few questions, though some may be trivial.  In the meanwhile,
I'll separate the early pieces of my series and rebase this one on top.

Paolo

[Qemu-devel] [PATCH 05/14] qdev: use wrapper for qdev_get_path

[Anthony Liguori]

This makes it easier to remove it from BusInfo.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 exec.c        |    4 ++--
 hw/qdev.c     |   16 ++++++++++++++++
 hw/qdev.h     |    2 ++
 hw/usb/desc.c |    7 +++++--
 savevm.c      |   12 ++++++------
 5 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/exec.c b/exec.c
index 0607c9b..e3523d2 100644
--- a/exec.c
+++ b/exec.c
@@ -2583,8 +2583,8 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
     assert(new_block);
     assert(!new_block->idstr[0]);
 
-    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
-        char *id = dev->parent_bus->info->get_dev_path(dev);
+    if (dev) {
+        char *id = qdev_get_dev_path(dev);
         if (id) {
             snprintf(new_block->idstr, sizeof(new_block->idstr), "%s/", id);
             g_free(id);
diff --git a/hw/qdev.c b/hw/qdev.c
index e17a9ab..e835650 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -519,6 +519,22 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
     return strdup(path);
 }
 
+char *qdev_get_dev_path(DeviceState *dev)
+{
+    BusClass *bc;
+
+    if (!dev->parent_bus) {
+        return NULL;
+    }
+
+    bc = BUS_GET_CLASS(dev->parent_bus);
+    if (bc->get_dev_path) {
+        return bc->get_dev_path(dev);
+    }
+
+    return NULL;
+}
+
 static char *qdev_get_type(Object *obj, Error **errp)
 {
     return g_strdup(object_get_typename(obj));
diff --git a/hw/qdev.h b/hw/qdev.h
index ca8386a..fc3b50f 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -362,4 +362,6 @@ extern int qdev_hotplug;
 
 void qdev_add_properties(DeviceState *dev, Property *props);
 
+char *qdev_get_dev_path(DeviceState *dev);
+
 #endif
diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index e8a3c6a..64352c9 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -433,11 +433,14 @@ void usb_desc_create_serial(USBDevice *dev)
     int index = desc->id.iSerialNumber;
     char serial[64];
     int dst;
+    char *path = NULL;
 
     assert(index != 0 && desc->str[index] != NULL);
     dst = snprintf(serial, sizeof(serial), "%s", desc->str[index]);
-    if (hcd && hcd->parent_bus && hcd->parent_bus->info->get_dev_path) {
-        char *path = hcd->parent_bus->info->get_dev_path(hcd);
+    if (hcd->parent_bus && hcd->parent_bus->parent) {
+        path = qdev_get_dev_path(hcd->parent_bus->parent);
+    }
+    if (path) {
         dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", path);
     }
     dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", dev->port->path);
diff --git a/savevm.c b/savevm.c
index 2d18bab..818ddfc 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1248,8 +1248,8 @@ int register_savevm_live(DeviceState *dev,
         se->is_ram = 1;
     }
 
-    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
-        char *id = dev->parent_bus->info->get_dev_path(dev);
+    if (dev) {
+        char *id = qdev_get_dev_path(dev);
         if (id) {
             pstrcpy(se->idstr, sizeof(se->idstr), id);
             pstrcat(se->idstr, sizeof(se->idstr), "/");
@@ -1292,8 +1292,8 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
     SaveStateEntry *se, *new_se;
     char id[256] = "";
 
-    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
-        char *path = dev->parent_bus->info->get_dev_path(dev);
+    if (dev) {
+        char *path = qdev_get_dev_path(dev);
         if (path) {
             pstrcpy(id, sizeof(id), path);
             pstrcat(id, sizeof(id), "/");
@@ -1334,8 +1334,8 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
     se->alias_id = alias_id;
     se->no_migrate = vmsd->unmigratable;
 
-    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
-        char *id = dev->parent_bus->info->get_dev_path(dev);
+    if (dev) {
+        char *id = qdev_get_dev_path(dev);
         if (id) {
             pstrcpy(se->idstr, sizeof(se->idstr), id);
             pstrcat(se->idstr, sizeof(se->idstr), "/");
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 05/14] qdev: use wrapper for qdev_get_path

[Anthony Liguori]

Hi Gerd,

Could you carefully review the USB changes here?  I'm not really sure what our 
contract is with the guest in terms of ABI compatibility.  I think it's good but 
it could use a second set of eyes.

Regards,

Anthony Liguori

On 05/01/2012 01:18 PM, Anthony Liguori wrote:
> This makes it easier to remove it from BusInfo.
>
> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
> ---
>   exec.c        |    4 ++--
>   hw/qdev.c     |   16 ++++++++++++++++
>   hw/qdev.h     |    2 ++
>   hw/usb/desc.c |    7 +++++--
>   savevm.c      |   12 ++++++------
>   5 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 0607c9b..e3523d2 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2583,8 +2583,8 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char *name, DeviceState *dev)
>       assert(new_block);
>       assert(!new_block->idstr[0]);
>
> -    if (dev&&  dev->parent_bus&&  dev->parent_bus->info->get_dev_path) {
> -        char *id = dev->parent_bus->info->get_dev_path(dev);
> +    if (dev) {
> +        char *id = qdev_get_dev_path(dev);
>           if (id) {
>               snprintf(new_block->idstr, sizeof(new_block->idstr), "%s/", id);
>               g_free(id);
> diff --git a/hw/qdev.c b/hw/qdev.c
> index e17a9ab..e835650 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -519,6 +519,22 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
>       return strdup(path);
>   }
>
> +char *qdev_get_dev_path(DeviceState *dev)
> +{
> +    BusClass *bc;
> +
> +    if (!dev->parent_bus) {
> +        return NULL;
> +    }
> +
> +    bc = BUS_GET_CLASS(dev->parent_bus);
> +    if (bc->get_dev_path) {
> +        return bc->get_dev_path(dev);
> +    }
> +
> +    return NULL;
> +}
> +
>   static char *qdev_get_type(Object *obj, Error **errp)
>   {
>       return g_strdup(object_get_typename(obj));
> diff --git a/hw/qdev.h b/hw/qdev.h
> index ca8386a..fc3b50f 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -362,4 +362,6 @@ extern int qdev_hotplug;
>
>   void qdev_add_properties(DeviceState *dev, Property *props);
>
> +char *qdev_get_dev_path(DeviceState *dev);
> +
>   #endif
> diff --git a/hw/usb/desc.c b/hw/usb/desc.c
> index e8a3c6a..64352c9 100644
> --- a/hw/usb/desc.c
> +++ b/hw/usb/desc.c
> @@ -433,11 +433,14 @@ void usb_desc_create_serial(USBDevice *dev)
>       int index = desc->id.iSerialNumber;
>       char serial[64];
>       int dst;
> +    char *path = NULL;
>
>       assert(index != 0&&  desc->str[index] != NULL);
>       dst = snprintf(serial, sizeof(serial), "%s", desc->str[index]);
> -    if (hcd&&  hcd->parent_bus&&  hcd->parent_bus->info->get_dev_path) {
> -        char *path = hcd->parent_bus->info->get_dev_path(hcd);
> +    if (hcd->parent_bus&&  hcd->parent_bus->parent) {
> +        path = qdev_get_dev_path(hcd->parent_bus->parent);
> +    }
> +    if (path) {
>           dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", path);
>       }
>       dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", dev->port->path);
> diff --git a/savevm.c b/savevm.c
> index 2d18bab..818ddfc 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1248,8 +1248,8 @@ int register_savevm_live(DeviceState *dev,
>           se->is_ram = 1;
>       }
>
> -    if (dev&&  dev->parent_bus&&  dev->parent_bus->info->get_dev_path) {
> -        char *id = dev->parent_bus->info->get_dev_path(dev);
> +    if (dev) {
> +        char *id = qdev_get_dev_path(dev);
>           if (id) {
>               pstrcpy(se->idstr, sizeof(se->idstr), id);
>               pstrcat(se->idstr, sizeof(se->idstr), "/");
> @@ -1292,8 +1292,8 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
>       SaveStateEntry *se, *new_se;
>       char id[256] = "";
>
> -    if (dev&&  dev->parent_bus&&  dev->parent_bus->info->get_dev_path) {
> -        char *path = dev->parent_bus->info->get_dev_path(dev);
> +    if (dev) {
> +        char *path = qdev_get_dev_path(dev);
>           if (path) {
>               pstrcpy(id, sizeof(id), path);
>               pstrcat(id, sizeof(id), "/");
> @@ -1334,8 +1334,8 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
>       se->alias_id = alias_id;
>       se->no_migrate = vmsd->unmigratable;
>
> -    if (dev&&  dev->parent_bus&&  dev->parent_bus->info->get_dev_path) {
> -        char *id = dev->parent_bus->info->get_dev_path(dev);
> +    if (dev) {
> +        char *id = qdev_get_dev_path(dev);
>           if (id) {
>               pstrcpy(se->idstr, sizeof(se->idstr), id);
>               pstrcat(se->idstr, sizeof(se->idstr), "/");

Re: [Qemu-devel] [PATCH 05/14] qdev: use wrapper for qdev_get_path

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 301 bytes --]

On 05/01/12 20:36, Anthony Liguori wrote:
> Hi Gerd,
> 
> Could you carefully review the USB changes here?  I'm not really sure
> what our contract is with the guest in terms of ABI compatibility.  I
> think it's good but it could use a second set of eyes.

incremental fix attached.

cheers,
  Gerd


[-- Attachment #2: 0001-fixup.patch --]
[-- Type: text/plain, Size: 991 bytes --]

>From 8823f1f5491639119d267289610fcad6dd1a0872 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Wed, 2 May 2012 14:30:54 +0200
Subject: [PATCH] fixup

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/desc.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index 64352c9..84ea9f2 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -433,13 +433,11 @@ void usb_desc_create_serial(USBDevice *dev)
     int index = desc->id.iSerialNumber;
     char serial[64];
     int dst;
-    char *path = NULL;
+    char *path;
 
     assert(index != 0 && desc->str[index] != NULL);
     dst = snprintf(serial, sizeof(serial), "%s", desc->str[index]);
-    if (hcd->parent_bus && hcd->parent_bus->parent) {
-        path = qdev_get_dev_path(hcd->parent_bus->parent);
-    }
+    path = qdev_get_dev_path(hcd);
     if (path) {
         dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", path);
     }
-- 
1.7.1

[Qemu-devel] [PATCH 06/14] qdev: move properties from businfo to base class instance init

[Anthony Liguori]

This removes knowledge of specific DeviceState subclasses from BusInfos.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/i2c.c                      |   15 +++++++++++----
 hw/ide/qdev.c                 |   15 +++++++++++----
 hw/intel-hda.c                |   15 +++++++++++----
 hw/pci.c                      |   27 +++++++++++++++++----------
 hw/scsi-bus.c                 |   19 +++++++++++++------
 hw/spapr_vio.c                |   15 +++++++++++----
 hw/usb/bus.c                  |   19 +++++++++++++------
 hw/usb/dev-smartcard-reader.c |   15 +++++++++++----
 hw/virtio-serial-bus.c        |   17 ++++++++++++-----
 9 files changed, 110 insertions(+), 47 deletions(-)

diff --git a/hw/i2c.c b/hw/i2c.c
index 23dfccb..6d9a7be 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -20,10 +20,6 @@ struct i2c_bus
 static struct BusInfo i2c_bus_info = {
     .name = "I2C",
     .size = sizeof(i2c_bus),
-    .props = (Property[]) {
-        DEFINE_PROP_UINT8("address", struct I2CSlave, address, 0),
-        DEFINE_PROP_END_OF_LIST(),
-    }
 };
 
 static void i2c_bus_pre_save(void *opaque)
@@ -221,10 +217,21 @@ static void i2c_slave_class_init(ObjectClass *klass, void *data)
     k->bus_info = &i2c_bus_info;
 }
 
+static Property i2c_bus_properties[] = {
+    DEFINE_PROP_UINT8("address", struct I2CSlave, address, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void i2c_slave_initfn(Object *obj)
+{
+    qdev_add_properties(DEVICE(obj), i2c_bus_properties);
+}
+
 static TypeInfo i2c_slave_type_info = {
     .name = TYPE_I2C_SLAVE,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(I2CSlave),
+    .instance_init = i2c_slave_initfn,
     .abstract = true,
     .class_size = sizeof(I2CSlaveClass),
     .class_init = i2c_slave_class_init,
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index a46578d..020207a 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -31,10 +31,6 @@ static struct BusInfo ide_bus_info = {
     .name  = "IDE",
     .size  = sizeof(IDEBus),
     .get_fw_dev_path = idebus_get_fw_dev_path,
-    .props = (Property[]) {
-        DEFINE_PROP_UINT32("unit", IDEDevice, unit, -1),
-        DEFINE_PROP_END_OF_LIST(),
-    },
 };
 
 void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
@@ -251,10 +247,21 @@ static void ide_device_class_init(ObjectClass *klass, void *data)
     k->bus_info = &ide_bus_info;
 }
 
+static Property ide_bus_properties[] = {
+    DEFINE_PROP_UINT32("unit", IDEDevice, unit, -1),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ide_device_initfn(Object *obj)
+{
+    qdev_add_properties(DEVICE(obj), ide_bus_properties);
+}
+
 static TypeInfo ide_device_type_info = {
     .name = TYPE_IDE_DEVICE,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(IDEDevice),
+    .instance_init = ide_device_initfn,
     .abstract = true,
     .class_size = sizeof(IDEDeviceClass),
     .class_init = ide_device_class_init,
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index bb11af2..9405f54 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -32,10 +32,6 @@
 static struct BusInfo hda_codec_bus_info = {
     .name      = "HDA",
     .size      = sizeof(HDACodecBus),
-    .props     = (Property[]) {
-        DEFINE_PROP_UINT32("cad", HDACodecDevice, cad, -1),
-        DEFINE_PROP_END_OF_LIST()
-    }
 };
 
 void hda_codec_bus_init(DeviceState *dev, HDACodecBus *bus,
@@ -1278,10 +1274,21 @@ static void hda_codec_device_class_init(ObjectClass *klass, void *data)
     k->bus_info = &hda_codec_bus_info;
 }
 
+static Property hda_codec_bus_properties[] = {
+    DEFINE_PROP_UINT32("cad", HDACodecDevice, cad, -1),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void hda_codec_device_initfn(Object *obj)
+{
+    qdev_add_properties(DEVICE(obj), hda_codec_bus_properties);
+}
+
 static TypeInfo hda_codec_device_type_info = {
     .name = TYPE_HDA_CODEC_DEVICE,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(HDACodecDevice),
+    .instance_init = hda_codec_device_initfn,
     .abstract = true,
     .class_size = sizeof(HDACodecDeviceClass),
     .class_init = hda_codec_device_class_init,
diff --git a/hw/pci.c b/hw/pci.c
index b706e69..0a77025 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -51,16 +51,6 @@ struct BusInfo pci_bus_info = {
     .get_dev_path = pcibus_get_dev_path,
     .get_fw_dev_path = pcibus_get_fw_dev_path,
     .reset      = pcibus_reset,
-    .props      = (Property[]) {
-        DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
-        DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
-        DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, 1),
-        DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
-                        QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
-        DEFINE_PROP_BIT("command_serr_enable", PCIDevice, cap_present,
-                        QEMU_PCI_CAP_SERR_BITNR, true),
-        DEFINE_PROP_END_OF_LIST()
-    }
 };
 
 static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
@@ -2004,10 +1994,27 @@ static void pci_device_class_init(ObjectClass *klass, void *data)
     k->bus_info = &pci_bus_info;
 }
 
+static Property pci_bus_properties[] = {
+    DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
+    DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
+    DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, 1),
+    DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
+                    QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
+    DEFINE_PROP_BIT("command_serr_enable", PCIDevice, cap_present,
+                    QEMU_PCI_CAP_SERR_BITNR, true),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void pci_device_initfn(Object *obj)
+{
+    qdev_add_properties(DEVICE(obj), pci_bus_properties);
+}
+
 static TypeInfo pci_device_type_info = {
     .name = TYPE_PCI_DEVICE,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(PCIDevice),
+    .instance_init = pci_device_initfn,
     .abstract = true,
     .class_size = sizeof(PCIDeviceClass),
     .class_init = pci_device_class_init,
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index dbdb99c..8071b1c 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -17,12 +17,6 @@ static struct BusInfo scsi_bus_info = {
     .size  = sizeof(SCSIBus),
     .get_dev_path = scsibus_get_dev_path,
     .get_fw_dev_path = scsibus_get_fw_dev_path,
-    .props = (Property[]) {
-        DEFINE_PROP_UINT32("channel", SCSIDevice, channel, 0),
-        DEFINE_PROP_UINT32("scsi-id", SCSIDevice, id, -1),
-        DEFINE_PROP_UINT32("lun", SCSIDevice, lun, -1),
-        DEFINE_PROP_END_OF_LIST(),
-    },
 };
 static int next_scsi_bus;
 
@@ -1579,10 +1573,23 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
     k->exit     = scsi_qdev_exit;
 }
 
+static Property scsi_bus_properties[] = {
+    DEFINE_PROP_UINT32("channel", SCSIDevice, channel, 0),
+    DEFINE_PROP_UINT32("scsi-id", SCSIDevice, id, -1),
+    DEFINE_PROP_UINT32("lun", SCSIDevice, lun, -1),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void scsi_device_initfn(Object *obj)
+{
+    qdev_add_properties(DEVICE(obj), scsi_bus_properties);
+}
+
 static TypeInfo scsi_device_type_info = {
     .name = TYPE_SCSI_DEVICE,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(SCSIDevice),
+    .instance_init = scsi_device_initfn,
     .abstract = true,
     .class_size = sizeof(SCSIDeviceClass),
     .class_init = scsi_device_class_init,
diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index fccf48b..2529e09 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -52,10 +52,6 @@
 static struct BusInfo spapr_vio_bus_info = {
     .name       = "spapr-vio",
     .size       = sizeof(VIOsPAPRBus),
-    .props = (Property[]) {
-        DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, vio_irq_num, 0), \
-        DEFINE_PROP_END_OF_LIST(),
-    },
 };
 
 VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg)
@@ -783,10 +779,21 @@ static void vio_spapr_device_class_init(ObjectClass *klass, void *data)
     k->bus_info = &spapr_vio_bus_info;
 }
 
+static Property spapr_vio_bus_properties[] = {
+    DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, vio_irq_num, 0),  \
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void vio_spapr_device_initfn(Object *obj)
+{
+    qdev_add_properties(DEVICE(obj), spapr_vio_bus_properties);
+}
+
 static TypeInfo spapr_vio_type_info = {
     .name = TYPE_VIO_SPAPR_DEVICE,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(VIOsPAPRDevice),
+    .instance_init = vio_spapr_device_initfn,
     .abstract = true,
     .class_size = sizeof(VIOsPAPRDeviceClass),
     .class_init = vio_spapr_device_class_init,
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 2068640..8f1eae0 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -17,12 +17,6 @@ static struct BusInfo usb_bus_info = {
     .print_dev = usb_bus_dev_print,
     .get_dev_path = usb_get_dev_path,
     .get_fw_dev_path = usb_get_fw_dev_path,
-    .props      = (Property[]) {
-        DEFINE_PROP_STRING("port", USBDevice, port_path),
-        DEFINE_PROP_BIT("full-path", USBDevice, flags,
-                        USB_DEV_FLAG_FULL_PATH, true),
-        DEFINE_PROP_END_OF_LIST()
-    },
 };
 static int next_usb_bus = 0;
 static QTAILQ_HEAD(, USBBus) busses = QTAILQ_HEAD_INITIALIZER(busses);
@@ -582,10 +576,23 @@ static void usb_device_class_init(ObjectClass *klass, void *data)
     k->exit     = usb_qdev_exit;
 }
 
+static Property usb_bus_properties[] = {
+    DEFINE_PROP_STRING("port", USBDevice, port_path),
+    DEFINE_PROP_BIT("full-path", USBDevice, flags,
+                    USB_DEV_FLAG_FULL_PATH, true),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void usb_device_initfn(Object *obj)
+{
+    qdev_add_properties(DEVICE(obj), usb_bus_properties);
+}
+
 static TypeInfo usb_device_type_info = {
     .name = TYPE_USB_DEVICE,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(USBDevice),
+    .instance_init = usb_device_initfn,
     .abstract = true,
     .class_size = sizeof(USBDeviceClass),
     .class_init = usb_device_class_init,
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 3b7604e..e64e5c9 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1058,10 +1058,6 @@ static Answer *ccid_peek_next_answer(USBCCIDState *s)
 static struct BusInfo ccid_bus_info = {
     .name = "ccid-bus",
     .size = sizeof(CCIDBus),
-    .props = (Property[]) {
-        DEFINE_PROP_UINT32("slot", struct CCIDCardState, slot, 0),
-        DEFINE_PROP_END_OF_LIST(),
-    }
 };
 
 void ccid_card_send_apdu_to_guest(CCIDCardState *card,
@@ -1347,10 +1343,21 @@ static void ccid_card_class_init(ObjectClass *klass, void *data)
     k->exit = ccid_card_exit;
 }
 
+static Property ccid_bus_properties[] = {
+    DEFINE_PROP_UINT32("slot", struct CCIDCardState, slot, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ccid_card_instance_initfn(Object *obj)
+{
+    qdev_add_properties(DEVICE(obj), ccid_bus_properties);
+}
+
 static TypeInfo ccid_card_type_info = {
     .name = TYPE_CCID_CARD,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(CCIDCardState),
+    .instance_init = ccid_card_instance_initfn,
     .abstract = true,
     .class_size = sizeof(CCIDCardClass),
     .class_init = ccid_card_class_init,
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index ffbdfc2..ca480dd 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -728,11 +728,6 @@ static struct BusInfo virtser_bus_info = {
     .name      = "virtio-serial-bus",
     .size      = sizeof(VirtIOSerialBus),
     .print_dev = virtser_bus_dev_print,
-    .props      = (Property[]) {
-        DEFINE_PROP_UINT32("nr", VirtIOSerialPort, id, VIRTIO_CONSOLE_BAD_ID),
-        DEFINE_PROP_STRING("name", VirtIOSerialPort, name),
-        DEFINE_PROP_END_OF_LIST()
-    }
 };
 
 static void virtser_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
@@ -981,10 +976,22 @@ static void virtio_serial_port_class_init(ObjectClass *klass, void *data)
     k->unplug = qdev_simple_unplug_cb;
 }
 
+static Property virtser_bus_properties[] = {
+    DEFINE_PROP_UINT32("nr", VirtIOSerialPort, id, VIRTIO_CONSOLE_BAD_ID),
+    DEFINE_PROP_STRING("name", VirtIOSerialPort, name),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void virtio_serial_port_initfn(Object *obj)
+{
+    qdev_add_properties(DEVICE(obj), virtser_bus_properties);
+}
+
 static TypeInfo virtio_serial_port_type_info = {
     .name = TYPE_VIRTIO_SERIAL_PORT,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(VirtIOSerialPort),
+    .instance_init = virtio_serial_port_initfn,
     .abstract = true,
     .class_size = sizeof(VirtIOSerialPortClass),
     .class_init = virtio_serial_port_class_init,
-- 
1.7.5.4

[Qemu-devel] [PATCH 07/14] qdev: fix info qtree/qdm

[Anthony Liguori]

Don't rely on bus_info.  I took a little liberty in the last commit as it would
cause info qtree/info qdm to not show any useful information.  But since this
is not considered a supported interface, breaking it across a single commit
seems okay.

This commit makes info qtree/qdm work again.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev-monitor.c    |  120 ++++++++++++++++++++++++++++----------------------
 hw/qdev-properties.c |   30 +++---------
 hw/qdev.c            |   26 +++--------
 hw/qdev.h            |    1 -
 4 files changed, 83 insertions(+), 94 deletions(-)

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index dc4e4e1..f5ee166 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -118,17 +118,44 @@ static const char *find_typename_by_alias(const char *alias)
     return NULL;
 }
 
+static void display_property(Object *obj, const char *name,
+                             const char *typename, bool read_only,
+                             void *opaque)
+{
+    char *legacy_typename;
+
+    if (!strstart(typename, "legacy<", NULL)) {
+        return;
+    }
+
+    legacy_typename = g_strdup(&typename[7]);
+    legacy_typename[strlen(legacy_typename) - 1] = 0;
+
+    /*
+     * TODO Properties without a parser are just for dirty hacks.
+     * qdev_prop_ptr is the only such PropertyInfo.  It's marked
+     * for removal.  This conditional should be removed along with
+     * it.
+     */
+    if (!read_only) {
+        error_printf("%s.%s=%s\n", object_get_typename(obj),
+                     &name[7], legacy_typename);
+    }
+
+    g_free(legacy_typename);
+}
+
 int qdev_device_help(QemuOpts *opts)
 {
     const char *driver;
-    Property *prop;
     ObjectClass *klass;
-    DeviceClass *info;
+    Object *obj;
 
     driver = qemu_opt_get(opts, "driver");
     if (driver && !strcmp(driver, "?")) {
         bool show_no_user = false;
-        object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, &show_no_user);
+        object_class_foreach(qdev_print_devinfo, TYPE_DEVICE,
+                             false, &show_no_user);
         return 1;
     }
 
@@ -149,30 +176,11 @@ int qdev_device_help(QemuOpts *opts)
     if (!klass) {
         return 0;
     }
-    info = DEVICE_CLASS(klass);
-
-    for (prop = info->props; prop && prop->name; prop++) {
-        /*
-         * TODO Properties without a parser are just for dirty hacks.
-         * qdev_prop_ptr is the only such PropertyInfo.  It's marked
-         * for removal.  This conditional should be removed along with
-         * it.
-         */
-        if (!prop->info->parse) {
-            continue;           /* no way to set it, don't show */
-        }
-        error_printf("%s.%s=%s\n", driver, prop->name,
-                     prop->info->legacy_name ?: prop->info->name);
-    }
-    if (info->bus_info) {
-        for (prop = info->bus_info->props; prop && prop->name; prop++) {
-            if (!prop->info->parse) {
-                continue;           /* no way to set it, don't show */
-            }
-            error_printf("%s.%s=%s\n", driver, prop->name,
-                         prop->info->legacy_name ?: prop->info->name);
-        }
-    }
+
+    obj = object_new(driver);
+    object_property_foreach(obj, display_property, NULL);
+    object_delete(obj);
+
     return 1;
 }
 
@@ -481,50 +489,56 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 #define qdev_printf(fmt, ...) monitor_printf(mon, "%*s" fmt, indent, "", ## __VA_ARGS__)
 static void qbus_print(Monitor *mon, BusState *bus, int indent);
 
-static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
-                             const char *prefix, int indent)
+typedef struct QTreePrinter
 {
-    if (!props)
-        return;
-    for (; props->name; props++) {
-        Error *err = NULL;
-        char *value;
-        char *legacy_name = g_strdup_printf("legacy-%s", props->name);
-        if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
-            value = object_property_get_str(OBJECT(dev), legacy_name, &err);
-        } else {
-            value = object_property_get_str(OBJECT(dev), props->name, &err);
-        }
-        g_free(legacy_name);
+    Visitor visitor;
+    Monitor *mon;
+    int indent;
+    const char *prefix;
+} QTreePrinter;
+
+static void qtree_printer_visit_str(Visitor *v, char **obj,
+                                    const char *name, Error **errp)
+{
+    QTreePrinter *p = (QTreePrinter *)v;
 
-        if (err) {
-            error_free(err);
-            continue;
-        }
-        qdev_printf("%s-prop: %s = %s\n", prefix, props->name,
-                    value && *value ? value : "<null>");
-        g_free(value);
+    monitor_printf(p->mon, "%*sdev-prop: %s = %s\n",
+                   p->indent, "", &name[7], *obj);
+}
+
+static void qdev_print_prop(Object *obj, const char *name,
+                             const char *typename, bool read_only,
+                             void *opaque)
+{
+    QTreePrinter *printer = opaque;
+
+    if (strstart(typename, "legacy<", NULL)) {
+        object_property_get(obj, &printer->visitor, name, NULL);
     }
 }
 
 static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
 {
+    QTreePrinter printer = {
+        .visitor.type_str = qtree_printer_visit_str,
+        .mon = mon,
+        .indent = indent + 2,
+    };
     BusState *child;
+
     qdev_printf("dev: %s, id \"%s\"\n", object_get_typename(OBJECT(dev)),
                 dev->id ? dev->id : "");
-    indent += 2;
     if (dev->num_gpio_in) {
         qdev_printf("gpio-in %d\n", dev->num_gpio_in);
     }
     if (dev->num_gpio_out) {
         qdev_printf("gpio-out %d\n", dev->num_gpio_out);
     }
-    qdev_print_props(mon, dev, qdev_get_props(dev), "dev", indent);
-    qdev_print_props(mon, dev, dev->parent_bus->info->props, "bus", indent);
+    object_property_foreach(OBJECT(dev), qdev_print_prop, &printer);
     if (dev->parent_bus->info->print_dev)
-        dev->parent_bus->info->print_dev(mon, dev, indent);
+        dev->parent_bus->info->print_dev(mon, dev, indent + 2);
     QLIST_FOREACH(child, &dev->child_bus, sibling) {
-        qbus_print(mon, child, indent);
+        qbus_print(mon, child, indent + 2);
     }
 }
 
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 894fa79..0c6dade 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -925,31 +925,17 @@ PropertyInfo qdev_prop_blocksize = {
 
 /* --- public helpers --- */
 
-static Property *qdev_prop_walk(Property *props, const char *name)
-{
-    if (!props)
-        return NULL;
-    while (props->name) {
-        if (strcmp(props->name, name) == 0)
-            return props;
-        props++;
-    }
-    return NULL;
-}
-
 static Property *qdev_prop_find(DeviceState *dev, const char *name)
 {
-    Property *prop;
-
-    /* device properties */
-    prop = qdev_prop_walk(qdev_get_props(dev), name);
-    if (prop)
-        return prop;
+    Object *obj = OBJECT(dev);
+    ObjectProperty *prop;
 
-    /* bus properties */
-    prop = qdev_prop_walk(dev->parent_bus->info->props, name);
-    if (prop)
-        return prop;
+    QTAILQ_FOREACH(prop, &obj->properties, node) {
+        if (strstart(prop->type, "legacy<", NULL) &&
+            strcmp(&prop->name[7], name) == 0) {
+            return prop->opaque;
+        }
+    }
 
     return NULL;
 }
diff --git a/hw/qdev.c b/hw/qdev.c
index e835650..05ed0c9 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -45,18 +45,6 @@ const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
     return dc->vmsd;
 }
 
-BusInfo *qdev_get_bus_info(DeviceState *dev)
-{
-    DeviceClass *dc = DEVICE_GET_CLASS(dev);
-    return dc->bus_info;
-}
-
-Property *qdev_get_props(DeviceState *dev)
-{
-    DeviceClass *dc = DEVICE_GET_CLASS(dev);
-    return dc->props;
-}
-
 const char *qdev_fw_name(DeviceState *dev)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
@@ -95,7 +83,6 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
 
     dev->parent_bus = bus;
     QTAILQ_INSERT_HEAD(&bus->children, dev, sibling);
-    qdev_add_properties(dev, dev->parent_bus->info->props);
 }
 
 /* Create a new device.  This only initializes the device state structure
@@ -609,7 +596,7 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop,
     object_property_add(OBJECT(dev), name, type,
                         prop->info->print ? qdev_get_legacy_property : prop->info->get,
                         prop->info->parse ? qdev_set_legacy_property : prop->info->set,
-                        NULL,
+                        prop->info->release,
                         prop, errp);
 
     g_free(type);
@@ -643,7 +630,7 @@ void qdev_property_add_static(DeviceState *dev, Property *prop,
 static void device_initfn(Object *obj)
 {
     DeviceState *dev = DEVICE(obj);
-    Property *prop;
+    DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
     if (qdev_hotplug) {
         dev->hotplugged = 1;
@@ -653,7 +640,8 @@ static void device_initfn(Object *obj)
     dev->instance_id_alias = -1;
     dev->state = DEV_STATE_CREATED;
 
-    qdev_add_properties(dev, qdev_get_props(dev));
+    qdev_add_properties(dev, dc->props);
+
     object_property_add_str(OBJECT(dev), "type", qdev_get_type, NULL, NULL);
 }
 
@@ -661,8 +649,8 @@ static void device_initfn(Object *obj)
 static void device_finalize(Object *obj)
 {
     DeviceState *dev = DEVICE(obj);
-    BusState *bus;
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
+    BusState *bus;
 
     if (dev->state == DEV_STATE_INITIALIZED) {
         while (dev->num_child_bus) {
@@ -679,7 +667,9 @@ static void device_finalize(Object *obj)
             qemu_opts_del(dev->opts);
         }
     }
-    QTAILQ_REMOVE(&dev->parent_bus->children, dev, sibling);
+    if (dev->parent_bus) {
+        QTAILQ_REMOVE(&dev->parent_bus->children, dev, sibling);
+    }
 }
 
 void device_reset(DeviceState *dev)
diff --git a/hw/qdev.h b/hw/qdev.h
index fc3b50f..6d2261f 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -96,7 +96,6 @@ struct BusInfo {
     bus_get_dev_path get_dev_path;
     bus_get_fw_dev_path get_fw_dev_path;
     qbus_resetfn *reset;
-    Property *props;
 };
 
 struct BusState {
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 07/14] qdev: fix info qtree/qdm

[Paolo Bonzini]

Il 01/05/2012 20:18, Anthony Liguori ha scritto:
> Don't rely on bus_info.  I took a little liberty in the last commit as it would
> cause info qtree/info qdm to not show any useful information.  But since this
> is not considered a supported interface, breaking it across a single commit
> seems okay.
> 
> This commit makes info qtree/qdm work again.

It omits all non-legacy properties, so I think it doesn't.

Paolo

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

[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 <aliguori@us.ibm.com>
---
 hw/i2c.c                      |   14 ++++++---
 hw/ide/qdev.c                 |   23 ++++++++++++----
 hw/intel-hda.c                |   14 ++++++---
 hw/isa-bus.c                  |   25 ++++++++++++-----
 hw/pci-hotplug.c              |    6 +---
 hw/pci.c                      |   27 ++++++++++++------
 hw/pci_bridge.c               |    2 +-
 hw/pci_internals.h            |    2 +-
 hw/qdev-monitor.c             |   33 ++++++++++++++--------
 hw/qdev.c                     |   59 +++++++++++++++++++++++++++++------------
 hw/qdev.h                     |   38 +++++++++++++-------------
 hw/s390-virtio-bus.c          |   14 ++++++---
 hw/scsi-bus.c                 |   23 +++++++++++-----
 hw/scsi.h                     |    4 +++
 hw/spapr_vio.c                |   14 ++++++---
 hw/ssi.c                      |   14 ++++++---
 hw/sysbus.c                   |   21 ++++++++++----
 hw/usb/bus.c                  |   28 ++++++++++++++-----
 hw/usb/dev-smartcard-reader.c |   14 ++++++---
 hw/virtio-serial-bus.c        |   22 +++++++++++----
 20 files changed, 263 insertions(+), 134 deletions(-)

diff --git a/hw/i2c.c b/hw/i2c.c
index 6d9a7be..1cf1c17 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -17,9 +17,12 @@ struct i2c_bus
     uint8_t saved_address;
 };
 
-static struct BusInfo i2c_bus_info = {
-    .name = "I2C",
-    .size = sizeof(i2c_bus),
+#define TYPE_I2C_BUS "i2c-bus"
+
+static TypeInfo i2c_bus_info = {
+    .name = TYPE_I2C_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(i2c_bus),
 };
 
 static void i2c_bus_pre_save(void *opaque)
@@ -57,7 +60,7 @@ i2c_bus *i2c_init_bus(DeviceState *parent, const char *name)
 {
     i2c_bus *bus;
 
-    bus = FROM_QBUS(i2c_bus, qbus_create(&i2c_bus_info, parent, name));
+    bus = FROM_QBUS(i2c_bus, qbus_create(TYPE_I2C_BUS, parent, name));
     vmstate_register(NULL, -1, &vmstate_i2c_bus, bus);
     return bus;
 }
@@ -214,7 +217,7 @@ static void i2c_slave_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = i2c_slave_qdev_init;
-    k->bus_info = &i2c_bus_info;
+    k->bus_type = TYPE_I2C_BUS;
 }
 
 static Property i2c_bus_properties[] = {
@@ -239,6 +242,7 @@ static TypeInfo i2c_slave_type_info = {
 
 static void i2c_slave_register_types(void)
 {
+    type_register_static(&i2c_bus_info);
     type_register_static(&i2c_slave_type_info);
 }
 
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 020207a..4a468f8 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -27,15 +27,25 @@
 
 static char *idebus_get_fw_dev_path(DeviceState *dev);
 
-static struct BusInfo ide_bus_info = {
-    .name  = "IDE",
-    .size  = sizeof(IDEBus),
-    .get_fw_dev_path = idebus_get_fw_dev_path,
+#define TYPE_IDE_BUS "IDE"
+
+static void ide_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *k = BUS_CLASS(klass);
+
+    k->get_fw_dev_path = idebus_get_fw_dev_path;
+}
+
+static TypeInfo ide_bus_info = {
+    .name = TYPE_IDE_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(IDEBus),
+    .class_init = ide_bus_class_init,
 };
 
 void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
 {
-    qbus_create_inplace(&idebus->qbus, &ide_bus_info, dev, NULL);
+    qbus_create_inplace(&idebus->qbus, TYPE_IDE_BUS, dev, NULL);
     idebus->bus_id = bus_id;
 }
 
@@ -244,7 +254,7 @@ static void ide_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = ide_qdev_init;
-    k->bus_info = &ide_bus_info;
+    k->bus_type = TYPE_IDE_BUS;
 }
 
 static Property ide_bus_properties[] = {
@@ -269,6 +279,7 @@ static TypeInfo ide_device_type_info = {
 
 static void ide_register_types(void)
 {
+    type_register_static(&ide_bus_info);
     type_register_static(&ide_hd_info);
     type_register_static(&ide_cd_info);
     type_register_static(&ide_drive_info);
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"
+
+static TypeInfo hda_codec_bus_info = {
+    .name = TYPE_HDA_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(HDACodecBus),
 };
 
 void hda_codec_bus_init(DeviceState *dev, HDACodecBus *bus,
                         hda_codec_response_func response,
                         hda_codec_xfer_func xfer)
 {
-    qbus_create_inplace(&bus->qbus, &hda_codec_bus_info, dev, NULL);
+    qbus_create_inplace(&bus->qbus, TYPE_HDA_BUS, dev, NULL);
     bus->response = response;
     bus->xfer = xfer;
 }
@@ -1271,7 +1274,7 @@ static void hda_codec_device_class_init(ObjectClass *klass, void *data)
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = hda_codec_dev_init;
     k->exit = hda_codec_dev_exit;
-    k->bus_info = &hda_codec_bus_info;
+    k->bus_type = TYPE_HDA_BUS;
 }
 
 static Property hda_codec_bus_properties[] = {
@@ -1296,6 +1299,7 @@ static TypeInfo hda_codec_device_type_info = {
 
 static void intel_hda_register_types(void)
 {
+    type_register_static(&hda_codec_bus_info);
     type_register_static(&intel_hda_info);
     type_register_static(&hda_codec_device_type_info);
 }
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 5a43f03..2616f22 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -28,11 +28,21 @@ target_phys_addr_t isa_mem_base = 0;
 static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent);
 static char *isabus_get_fw_dev_path(DeviceState *dev);
 
-static struct BusInfo isa_bus_info = {
-    .name      = "ISA",
-    .size      = sizeof(ISABus),
-    .print_dev = isabus_dev_print,
-    .get_fw_dev_path = isabus_get_fw_dev_path,
+#define TYPE_ISA_BUS "ISA"
+
+static void isa_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *k = BUS_CLASS(klass);
+
+    k->print_dev = isabus_dev_print;
+    k->get_fw_dev_path = isabus_get_fw_dev_path;
+}
+
+static TypeInfo isa_bus_info = {
+    .name = TYPE_ISA_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(ISABus),
+    .class_init = isa_bus_class_init,
 };
 
 ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space_io)
@@ -46,7 +56,7 @@ ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space_io)
         qdev_init_nofail(dev);
     }
 
-    isabus = FROM_QBUS(ISABus, qbus_create(&isa_bus_info, dev, NULL));
+    isabus = FROM_QBUS(ISABus, qbus_create(TYPE_ISA_BUS, dev, NULL));
     isabus->address_space_io = address_space_io;
     return isabus;
 }
@@ -198,7 +208,7 @@ static void isa_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = isa_qdev_init;
-    k->bus_info = &isa_bus_info;
+    k->bus_type = TYPE_ISA_BUS;
 }
 
 static TypeInfo isa_device_type_info = {
@@ -212,6 +222,7 @@ static TypeInfo isa_device_type_info = {
 
 static void isabus_register_types(void)
 {
+    type_register_static(&isa_bus_info);
     type_register_static(&isabus_bridge_info);
     type_register_static(&isa_device_type_info);
 }
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index c55d8b9..e77fea0 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -76,11 +76,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
     SCSIBus *scsibus;
     SCSIDevice *scsidev;
 
-    scsibus = DO_UPCAST(SCSIBus, qbus, QLIST_FIRST(&adapter->child_bus));
-    if (!scsibus || strcmp(scsibus->qbus.info->name, "SCSI") != 0) {
-        error_report("Device is not a SCSI adapter");
-        return -1;
-    }
+    scsibus = SCSI_BUS(QLIST_FIRST(&adapter->child_bus));
 
     /*
      * drive_init() tries to find a default for dinfo->unit.  Doesn't
diff --git a/hw/pci.c b/hw/pci.c
index 0a77025..bff303b 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -44,13 +44,21 @@ static char *pcibus_get_dev_path(DeviceState *dev);
 static char *pcibus_get_fw_dev_path(DeviceState *dev);
 static int pcibus_reset(BusState *qbus);
 
-struct BusInfo pci_bus_info = {
-    .name       = "PCI",
-    .size       = sizeof(PCIBus),
-    .print_dev  = pcibus_dev_print,
-    .get_dev_path = pcibus_get_dev_path,
-    .get_fw_dev_path = pcibus_get_fw_dev_path,
-    .reset      = pcibus_reset,
+static void pci_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *k = BUS_CLASS(klass);
+
+    k->print_dev = pcibus_dev_print;
+    k->get_dev_path = pcibus_get_dev_path;
+    k->get_fw_dev_path = pcibus_get_fw_dev_path;
+    k->reset = pcibus_reset;
+}
+
+static TypeInfo pci_bus_info = {
+    .name = TYPE_PCI_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(PCIBus),
+    .class_init = pci_bus_class_init,
 };
 
 static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
@@ -255,7 +263,7 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
                          MemoryRegion *address_space_io,
                          uint8_t devfn_min)
 {
-    qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
+    qbus_create_inplace(&bus->qbus, TYPE_PCI_BUS, parent, name);
     assert(PCI_FUNC(devfn_min) == 0);
     bus->devfn_min = devfn_min;
     bus->address_space_mem = address_space_mem;
@@ -1991,7 +1999,7 @@ static void pci_device_class_init(ObjectClass *klass, void *data)
     k->init = pci_qdev_init;
     k->unplug = pci_unplug_device;
     k->exit = pci_unregister_device;
-    k->bus_info = &pci_bus_info;
+    k->bus_type = TYPE_PCI_BUS;
 }
 
 static Property pci_bus_properties[] = {
@@ -2022,6 +2030,7 @@ static TypeInfo pci_device_type_info = {
 
 static void pci_register_types(void)
 {
+    type_register_static(&pci_bus_info);
     type_register_static(&pci_device_type_info);
 }
 
diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index 866f0b6..253e034 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -324,7 +324,7 @@ int pci_bridge_initfn(PCIDevice *dev)
 	    br->bus_name = dev->qdev.id;
     }
 
-    qbus_create_inplace(&sec_bus->qbus, &pci_bus_info, &dev->qdev,
+    qbus_create_inplace(&sec_bus->qbus, TYPE_PCI_BUS, &dev->qdev,
                         br->bus_name);
     sec_bus->parent_dev = dev;
     sec_bus->map_irq = br->map_irq;
diff --git a/hw/pci_internals.h b/hw/pci_internals.h
index 96690b7..321e1fc 100644
--- a/hw/pci_internals.h
+++ b/hw/pci_internals.h
@@ -12,7 +12,7 @@
  * Use accessor function in pci.h, pci_bridge.h
  */
 
-extern struct BusInfo pci_bus_info;
+#define TYPE_PCI_BUS "PCI"
 
 struct PCIBus {
     BusState qbus;
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
@@ -75,8 +75,8 @@ static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
     }
 
     error_printf("name \"%s\"", object_class_get_name(klass));
-    if (dc->bus_info) {
-        error_printf(", bus %s", dc->bus_info->name);
+    if (dc->bus_type) {
+        error_printf(", bus %s", dc->bus_type);
     }
     if (qdev_class_has_alias(dc)) {
         error_printf(", alias \"%s\"", qdev_class_get_alias(dc));
@@ -279,7 +279,7 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem)
 }
 
 static BusState *qbus_find_recursive(BusState *bus, const char *name,
-                                     const BusInfo *info)
+                                     const char *bus_typename)
 {
     DeviceState *dev;
     BusState *child, *ret;
@@ -288,7 +288,8 @@ static BusState *qbus_find_recursive(BusState *bus, const char *name,
     if (name && (strcmp(bus->name, name) != 0)) {
         match = 0;
     }
-    if (info && (bus->info != info)) {
+    if (bus_typename &&
+        (strcmp(object_get_typename(OBJECT(bus)), bus_typename) != 0)) {
         match = 0;
     }
     if (match) {
@@ -297,7 +298,7 @@ static BusState *qbus_find_recursive(BusState *bus, const char *name,
 
     QTAILQ_FOREACH(dev, &bus->children, sibling) {
         QLIST_FOREACH(child, &dev->child_bus, sibling) {
-            ret = qbus_find_recursive(child, name, info);
+            ret = qbus_find_recursive(child, name, bus_typename);
             if (ret) {
                 return ret;
             }
@@ -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){
             qerror_report(QERR_BAD_BUS_FOR_DEVICE,
-                           driver, bus->info->name);
+                          driver, object_get_typename(OBJECT(bus)));
             return NULL;
         }
     } else {
-        bus = qbus_find_recursive(sysbus_get_default(), NULL, k->bus_info);
+        bus = qbus_find_recursive(sysbus_get_default(), NULL, k->bus_type);
         if (!bus) {
             qerror_report(QERR_NO_BUS_FOR_DEVICE,
-                          driver, k->bus_info->name);
+                          driver, k->bus_type);
             return NULL;
         }
     }
@@ -517,6 +518,15 @@ static void qdev_print_prop(Object *obj, const char *name,
     }
 }
 
+static void bus_print_dev(BusState *bus, Monitor *mon, DeviceState *dev, int indent)
+{
+    BusClass *bc = BUS_GET_CLASS(bus);
+
+    if (bc->print_dev) {
+        bc->print_dev(mon, dev, indent);
+    }
+}
+
 static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
 {
     QTreePrinter printer = {
@@ -535,8 +545,7 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
         qdev_printf("gpio-out %d\n", dev->num_gpio_out);
     }
     object_property_foreach(OBJECT(dev), qdev_print_prop, &printer);
-    if (dev->parent_bus->info->print_dev)
-        dev->parent_bus->info->print_dev(mon, dev, indent + 2);
+    bus_print_dev(dev->parent_bus, mon, dev, indent + 2);
     QLIST_FOREACH(child, &dev->child_bus, sibling) {
         qbus_print(mon, child, indent + 2);
     }
@@ -548,7 +557,7 @@ static void qbus_print(Monitor *mon, BusState *bus, int indent)
 
     qdev_printf("bus: %s\n", bus->name);
     indent += 2;
-    qdev_printf("type %s\n", bus->info->name);
+    qdev_printf("type %s\n", object_get_typename(OBJECT(bus)));
     QTAILQ_FOREACH(dev, &bus->children, sibling) {
         qdev_print(mon, dev, indent);
     }
diff --git a/hw/qdev.c b/hw/qdev.c
index 05ed0c9..19d510e 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -96,7 +96,7 @@ DeviceState *qdev_create(BusState *bus, const char *name)
     if (!dev) {
         if (bus) {
             hw_error("Unknown device '%s' for bus '%s'\n", name,
-                     bus->info->name);
+                     object_get_typename(OBJECT(bus)));
         } else {
             hw_error("Unknown device '%s' for default sysbus\n", name);
         }
@@ -210,8 +210,9 @@ BusState *sysbus_get_default(void)
 
 static int qbus_reset_one(BusState *bus, void *opaque)
 {
-    if (bus->info->reset) {
-        return bus->info->reset(bus);
+    BusClass *bc = BUS_GET_CLASS(bus);
+    if (bc->reset) {
+        return bc->reset(bus);
     }
     return 0;
 }
@@ -394,13 +395,13 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id)
     return NULL;
 }
 
-void qbus_create_inplace(BusState *bus, BusInfo *info,
-                         DeviceState *parent, const char *name)
+/* FIXME move this logic into instance_init */
+static void do_qbus_create_inplace(BusState *bus, const char *typename,
+                                   DeviceState *parent, const char *name)
 {
     char *buf;
     int i,len;
 
-    bus->info = info;
     bus->parent = parent;
 
     if (name) {
@@ -414,9 +415,9 @@ void qbus_create_inplace(BusState *bus, BusInfo *info,
         bus->name = buf;
     } else {
         /* no id -> use lowercase bus type for bus name */
-        len = strlen(info->name) + 16;
+        len = strlen(typename) + 16;
         buf = g_malloc(len);
-        len = snprintf(buf, len, "%s.%d", info->name,
+        len = snprintf(buf, len, "%s.%d", typename,
                        parent ? parent->num_child_bus : 0);
         for (i = 0; i < len; i++)
             buf[i] = qemu_tolower(buf[i]);
@@ -434,13 +435,20 @@ void qbus_create_inplace(BusState *bus, BusInfo *info,
     }
 }
 
-BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name)
+void qbus_create_inplace(BusState *bus, const char *typename,
+                         DeviceState *parent, const char *name)
+{
+    object_initialize(bus, typename);
+    do_qbus_create_inplace(bus, typename, parent, name);
+}
+
+BusState *qbus_create(const char *typename, DeviceState *parent, const char *name)
 {
     BusState *bus;
 
-    bus = g_malloc0(info->size);
+    bus = BUS(object_new(typename));
     bus->qdev_allocated = 1;
-    qbus_create_inplace(bus, info, parent, name);
+    do_qbus_create_inplace(bus, typename, parent, name);
     return bus;
 }
 
@@ -448,10 +456,7 @@ 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 = g_malloc0(system_bus_info.size);
-    main_system_bus->qdev_allocated = 1;
-    qbus_create_inplace(main_system_bus, &system_bus_info, NULL,
-                        "main-system-bus");
+    main_system_bus = qbus_create(TYPE_SYSTEM_BUS, NULL, "main-system-bus");
 }
 
 void qbus_free(BusState *bus)
@@ -474,6 +479,17 @@ void qbus_free(BusState *bus)
     }
 }
 
+static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev)
+{
+    BusClass *bc = BUS_GET_CLASS(bus);
+
+    if (bc->get_fw_dev_path) {
+        return bc->get_fw_dev_path(dev);
+    }
+
+    return NULL;
+}
+
 static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
 {
     int l = 0;
@@ -481,8 +497,8 @@ static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
     if (dev && dev->parent_bus) {
         char *d;
         l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
-        if (dev->parent_bus->info->get_fw_dev_path) {
-            d = dev->parent_bus->info->get_fw_dev_path(dev);
+        d = bus_get_fw_dev_path(dev->parent_bus, dev);
+        if (d) {
             l += snprintf(p + l, size - l, "%s", d);
             g_free(d);
         } else {
@@ -702,8 +718,17 @@ static TypeInfo device_type_info = {
     .class_size = sizeof(DeviceClass),
 };
 
+static TypeInfo bus_info = {
+    .name = TYPE_BUS,
+    .parent = TYPE_OBJECT,
+    .instance_size = sizeof(BusState),
+    .abstract = true,
+    .class_size = sizeof(BusClass),
+};
+
 static void qdev_register_types(void)
 {
+    type_register_static(&bus_info);
     type_register_static(&device_type_info);
 }
 
diff --git a/hw/qdev.h b/hw/qdev.h
index 6d2261f..8ac703e 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -17,7 +17,7 @@ typedef struct CompatProperty CompatProperty;
 
 typedef struct BusState BusState;
 
-typedef struct BusInfo BusInfo;
+typedef struct BusClass BusClass;
 
 enum DevState {
     DEV_STATE_CREATED = 1,
@@ -55,7 +55,7 @@ typedef struct DeviceClass {
     qdev_initfn init;
     qdev_event unplug;
     qdev_event exit;
-    BusInfo *bus_info;
+    const char *bus_type;
 } DeviceClass;
 
 /* This structure should not be accessed directly.  We declare it here
@@ -79,28 +79,30 @@ struct DeviceState {
     int alias_required_for_version;
 };
 
-typedef void (*bus_dev_printfn)(Monitor *mon, DeviceState *dev, int indent);
-typedef char *(*bus_get_dev_path)(DeviceState *dev);
 /*
  * This callback is used to create Open Firmware device path in accordance with
  * OF spec http://forthworks.com/standards/of1275.pdf. Indicidual bus bindings
  * can be found here http://playground.sun.com/1275/bindings/.
  */
-typedef char *(*bus_get_fw_dev_path)(DeviceState *dev);
-typedef int (qbus_resetfn)(BusState *bus);
 
-struct BusInfo {
-    const char *name;
-    size_t size;
-    bus_dev_printfn print_dev;
-    bus_get_dev_path get_dev_path;
-    bus_get_fw_dev_path get_fw_dev_path;
-    qbus_resetfn *reset;
+#define TYPE_BUS "bus"
+#define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
+#define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
+#define BUS_GET_CLASS(obj) OBJECT_GET_CLASS(BusClass, (obj), TYPE_BUS)
+
+struct BusClass {
+    ObjectClass parent_class;
+
+    /* FIXME first arg should be BusState */
+    void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
+    char *(*get_dev_path)(DeviceState *dev);
+    char *(*get_fw_dev_path)(DeviceState *dev);
+    int (*reset)(BusState *bus);
 };
 
 struct BusState {
+    Object obj;
     DeviceState *parent;
-    BusInfo *info;
     const char *name;
     int allow_hotplug;
     int qdev_allocated;
@@ -176,9 +178,9 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id);
 typedef int (qbus_walkerfn)(BusState *bus, void *opaque);
 typedef int (qdev_walkerfn)(DeviceState *dev, void *opaque);
 
-void qbus_create_inplace(BusState *bus, BusInfo *info,
+void qbus_create_inplace(BusState *bus, const char *typename,
                          DeviceState *parent, const char *name);
-BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name);
+BusState *qbus_create(const char *typename, DeviceState *parent, const char *name);
 /* Returns > 0 if either devfn or busfn skip walk somewhere in cursion,
  *         < 0 if either devfn or busfn terminate walk somewhere in cursion,
  *           0 otherwise. */
@@ -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"
 
 /**
  * @qdev_property_add_static - add a @Property to a device referencing a
@@ -348,8 +350,6 @@ const VMStateDescription *qdev_get_vmsd(DeviceState *dev);
 
 const char *qdev_fw_name(DeviceState *dev);
 
-BusInfo *qdev_get_bus_info(DeviceState *dev);
-
 Property *qdev_get_props(DeviceState *dev);
 
 Object *qdev_get_machine(void);
diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index be1f5f1..60871a2 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -45,9 +45,12 @@
 
 #define VIRTIO_EXT_CODE   0x2603
 
-struct BusInfo s390_virtio_bus_info = {
-    .name       = "s390-virtio",
-    .size       = sizeof(VirtIOS390Bus),
+#define TYPE_S390_VIRTIO_BUS "s390-virtio-bus"
+
+static TypeInfo s390_virtio_bus_info = {
+    .name = TYPE_S390_VIRTIO_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(VirtIOS390Bus),
 };
 
 static const VirtIOBindings virtio_s390_bindings;
@@ -69,7 +72,7 @@ VirtIOS390Bus *s390_virtio_bus_init(ram_addr_t *ram_size)
 
     /* Create bus on bridge device */
 
-    _bus = qbus_create(&s390_virtio_bus_info, dev, "s390-virtio");
+    _bus = qbus_create(TYPE_S390_VIRTIO_BUS, dev, "s390-virtio");
     bus = DO_UPCAST(VirtIOS390Bus, bus, _bus);
 
     bus->dev_page = *ram_size;
@@ -432,7 +435,7 @@ static void virtio_s390_device_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->init = s390_virtio_busdev_init;
-    dc->bus_info = &s390_virtio_bus_info;
+    dc->bus_type = TYPE_S390_VIRTIO_BUS;
     dc->unplug = qdev_simple_unplug_cb;
 }
 
@@ -493,6 +496,7 @@ static TypeInfo s390_virtio_bridge_info = {
 
 static void s390_virtio_register_types(void)
 {
+    type_register_static(&s390_virtio_bus_info);
     type_register_static(&virtio_s390_device_info);
     type_register_static(&s390_virtio_serial);
     type_register_static(&s390_virtio_blk);
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 8071b1c..9949192 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -12,11 +12,19 @@ static char *scsibus_get_fw_dev_path(DeviceState *dev);
 static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
 static void scsi_req_dequeue(SCSIRequest *req);
 
-static struct BusInfo scsi_bus_info = {
-    .name  = "SCSI",
-    .size  = sizeof(SCSIBus),
-    .get_dev_path = scsibus_get_dev_path,
-    .get_fw_dev_path = scsibus_get_fw_dev_path,
+static void scsi_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *k = BUS_CLASS(klass);
+
+    k->get_dev_path = scsibus_get_dev_path;
+    k->get_fw_dev_path = scsibus_get_fw_dev_path;
+}
+
+static TypeInfo scsi_bus_info = {
+    .name = TYPE_SCSI_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(SCSIBus),
+    .class_init = scsi_bus_class_init,
 };
 static int next_scsi_bus;
 
@@ -59,7 +67,7 @@ static void scsi_device_unit_attention_reported(SCSIDevice *s)
 /* Create a scsi bus, and attach devices to it.  */
 void scsi_bus_new(SCSIBus *bus, DeviceState *host, const SCSIBusInfo *info)
 {
-    qbus_create_inplace(&bus->qbus, &scsi_bus_info, host, NULL);
+    qbus_create_inplace(&bus->qbus, TYPE_SCSI_BUS, host, NULL);
     bus->busnr = next_scsi_bus++;
     bus->info = info;
     bus->qbus.allow_hotplug = 1;
@@ -1567,7 +1575,7 @@ const VMStateDescription vmstate_scsi_device = {
 static void scsi_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
-    k->bus_info = &scsi_bus_info;
+    k->bus_type = TYPE_SCSI_BUS;
     k->init     = scsi_qdev_init;
     k->unplug   = qdev_simple_unplug_cb;
     k->exit     = scsi_qdev_exit;
@@ -1597,6 +1605,7 @@ static TypeInfo scsi_device_type_info = {
 
 static void scsi_register_types(void)
 {
+    type_register_static(&scsi_bus_info);
     type_register_static(&scsi_device_type_info);
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index 2eb66f7..16d7b17 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -136,6 +136,10 @@ struct SCSIBusInfo {
     void *(*load_request)(QEMUFile *f, SCSIRequest *req);
 };
 
+#define TYPE_SCSI_BUS "SCSI"
+#define SCSI_BUS(obj) \
+    OBJECT_CHECK(SCSIBus, (obj), TYPE_SCSI_BUS)
+
 struct SCSIBus {
     BusState qbus;
     int busnr;
diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index 2529e09..c7c3935 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -49,9 +49,12 @@
     do { } while (0)
 #endif
 
-static struct BusInfo spapr_vio_bus_info = {
-    .name       = "spapr-vio",
-    .size       = sizeof(VIOsPAPRBus),
+#define TYPE_SPAPR_VIO_BUS "spapr-vio-bus"
+
+static TypeInfo spapr_vio_bus_info = {
+    .name = TYPE_SPAPR_VIO_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(VIOsPAPRBus),
 };
 
 VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg)
@@ -725,7 +728,7 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
 
     /* Create bus on bridge device */
 
-    qbus = qbus_create(&spapr_vio_bus_info, dev, "spapr-vio");
+    qbus = qbus_create(TYPE_SPAPR_VIO_BUS, dev, "spapr-vio");
     bus = DO_UPCAST(VIOsPAPRBus, bus, qbus);
 
     /* hcall-vio */
@@ -776,7 +779,7 @@ static void vio_spapr_device_class_init(ObjectClass *klass, void *data)
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = spapr_vio_busdev_init;
     k->reset = spapr_vio_busdev_reset;
-    k->bus_info = &spapr_vio_bus_info;
+    k->bus_type = TYPE_SPAPR_VIO_BUS;
 }
 
 static Property spapr_vio_bus_properties[] = {
@@ -801,6 +804,7 @@ static TypeInfo spapr_vio_type_info = {
 
 static void spapr_vio_register_types(void)
 {
+    type_register_static(&spapr_vio_bus_info);
     type_register_static(&spapr_vio_bridge_info);
     type_register_static(&spapr_vio_type_info);
 }
diff --git a/hw/ssi.c b/hw/ssi.c
index 8f2d9bc..543adff 100644
--- a/hw/ssi.c
+++ b/hw/ssi.c
@@ -16,9 +16,12 @@ struct SSIBus {
     BusState qbus;
 };
 
-static struct BusInfo ssi_bus_info = {
-    .name = "SSI",
-    .size = sizeof(SSIBus),
+#define TYPE_SSI_BUS "SSI"
+
+static TypeInfo ssi_bus_info = {
+    .name = TYPE_SSI_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(SSIBus),
 };
 
 static int ssi_slave_init(DeviceState *dev)
@@ -40,7 +43,7 @@ static void ssi_slave_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     dc->init = ssi_slave_init;
-    dc->bus_info = &ssi_bus_info;
+    dc->bus_type = TYPE_SSI_BUS;
 }
 
 static TypeInfo ssi_slave_info = {
@@ -62,7 +65,7 @@ DeviceState *ssi_create_slave(SSIBus *bus, const char *name)
 SSIBus *ssi_create_bus(DeviceState *parent, const char *name)
 {
     BusState *bus;
-    bus = qbus_create(&ssi_bus_info, parent, name);
+    bus = qbus_create(TYPE_SSI_BUS, parent, name);
     return FROM_QBUS(SSIBus, bus);
 }
 
@@ -82,6 +85,7 @@ uint32_t ssi_transfer(SSIBus *bus, uint32_t val)
 
 static void ssi_slave_register_types(void)
 {
+    type_register_static(&ssi_bus_info);
     type_register_static(&ssi_slave_info);
 }
 
diff --git a/hw/sysbus.c b/hw/sysbus.c
index db4efcc..bad1bdc 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -24,11 +24,19 @@
 static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent);
 static char *sysbus_get_fw_dev_path(DeviceState *dev);
 
-struct BusInfo system_bus_info = {
-    .name       = "System",
-    .size       = sizeof(BusState),
-    .print_dev  = sysbus_dev_print,
-    .get_fw_dev_path = sysbus_get_fw_dev_path,
+static void system_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *k = BUS_CLASS(klass);
+
+    k->print_dev = sysbus_dev_print;
+    k->get_fw_dev_path = sysbus_get_fw_dev_path;
+}
+
+static TypeInfo system_bus_info = {
+    .name = TYPE_SYSTEM_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(BusState),
+    .class_init = system_bus_class_init,
 };
 
 void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
@@ -244,7 +252,7 @@ static void sysbus_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = sysbus_device_init;
-    k->bus_info = &system_bus_info;
+    k->bus_type = TYPE_SYSTEM_BUS;
 }
 
 static TypeInfo sysbus_device_type_info = {
@@ -258,6 +266,7 @@ static TypeInfo sysbus_device_type_info = {
 
 static void sysbus_register_types(void)
 {
+    type_register_static(&system_bus_info);
     type_register_static(&sysbus_device_type_info);
 }
 
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 8f1eae0..e2a87ed 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -11,13 +11,24 @@ static char *usb_get_dev_path(DeviceState *dev);
 static char *usb_get_fw_dev_path(DeviceState *qdev);
 static int usb_qdev_exit(DeviceState *qdev);
 
-static struct BusInfo usb_bus_info = {
-    .name      = "USB",
-    .size      = sizeof(USBBus),
-    .print_dev = usb_bus_dev_print,
-    .get_dev_path = usb_get_dev_path,
-    .get_fw_dev_path = usb_get_fw_dev_path,
+#define TYPE_USB_BUS "usb-bus"
+
+static void usb_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *k = BUS_CLASS(klass);
+
+    k->print_dev = usb_bus_dev_print;
+    k->get_dev_path = usb_get_dev_path;
+    k->get_fw_dev_path = usb_get_fw_dev_path;
+}
+
+static TypeInfo usb_bus_info = {
+    .name = TYPE_USB_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(USBBus),
+    .class_init = usb_bus_class_init,
 };
+
 static int next_usb_bus = 0;
 static QTAILQ_HEAD(, USBBus) busses = QTAILQ_HEAD_INITIALIZER(busses);
 
@@ -39,7 +50,7 @@ const VMStateDescription vmstate_usb_device = {
 
 void usb_bus_new(USBBus *bus, USBBusOps *ops, DeviceState *host)
 {
-    qbus_create_inplace(&bus->qbus, &usb_bus_info, host, NULL);
+    qbus_create_inplace(&bus->qbus, TYPE_USB_BUS, host, NULL);
     bus->ops = ops;
     bus->busnr = next_usb_bus++;
     bus->qbus.allow_hotplug = 1; /* Yes, we can */
@@ -570,7 +581,7 @@ USBDevice *usbdevice_create(const char *cmdline)
 static void usb_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
-    k->bus_info = &usb_bus_info;
+    k->bus_type = TYPE_USB_BUS;
     k->init     = usb_qdev_init;
     k->unplug   = qdev_simple_unplug_cb;
     k->exit     = usb_qdev_exit;
@@ -600,6 +611,7 @@ static TypeInfo usb_device_type_info = {
 
 static void usb_register_types(void)
 {
+    type_register_static(&usb_bus_info);
     type_register_static(&usb_device_type_info);
 }
 
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index e64e5c9..f93ae36 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1055,9 +1055,12 @@ static Answer *ccid_peek_next_answer(USBCCIDState *s)
         : &s->pending_answers[s->pending_answers_start % PENDING_ANSWERS_NUM];
 }
 
-static struct BusInfo ccid_bus_info = {
-    .name = "ccid-bus",
-    .size = sizeof(CCIDBus),
+#define TYPE_CCID_BUS "ccid-bus"
+
+static TypeInfo ccid_bus_info = {
+    .name = TYPE_CCID_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(CCIDBus),
 };
 
 void ccid_card_send_apdu_to_guest(CCIDCardState *card,
@@ -1187,7 +1190,7 @@ static int ccid_initfn(USBDevice *dev)
 
     usb_desc_create_serial(dev);
     usb_desc_init(dev);
-    qbus_create_inplace(&s->bus.qbus, &ccid_bus_info, &dev->qdev, NULL);
+    qbus_create_inplace(&s->bus.qbus, TYPE_CCID_BUS, &dev->qdev, NULL);
     s->intr = usb_ep_get(dev, USB_TOKEN_IN, CCID_INT_IN_EP);
     s->bus.qbus.allow_hotplug = 1;
     s->card = NULL;
@@ -1338,7 +1341,7 @@ static TypeInfo ccid_info = {
 static void ccid_card_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
-    k->bus_info = &ccid_bus_info;
+    k->bus_type = TYPE_CCID_BUS;
     k->init = ccid_card_init;
     k->exit = ccid_card_exit;
 }
@@ -1365,6 +1368,7 @@ static TypeInfo ccid_card_type_info = {
 
 static void ccid_register_types(void)
 {
+    type_register_static(&ccid_bus_info);
     type_register_static(&ccid_card_type_info);
     type_register_static(&ccid_info);
     usb_legacy_register(CCID_DEV_NAME, "ccid", NULL);
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index ca480dd..459219a 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -724,10 +724,19 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
 
 static void virtser_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
 
-static struct BusInfo virtser_bus_info = {
-    .name      = "virtio-serial-bus",
-    .size      = sizeof(VirtIOSerialBus),
-    .print_dev = virtser_bus_dev_print,
+#define TYPE_VIRTIO_SERIAL_BUS "virtio-serial-bus"
+
+static void virtser_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *k = BUS_CLASS(klass);
+    k->print_dev = virtser_bus_dev_print;
+}
+
+static TypeInfo virtser_bus_info = {
+    .name = TYPE_VIRTIO_SERIAL_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(VirtIOSerialBus),
+    .class_init = virtser_bus_class_init,
 };
 
 static void virtser_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
@@ -895,7 +904,7 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf)
     vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
 
     /* Spawn a new virtio-serial bus on which the ports will ride as devices */
-    qbus_create_inplace(&vser->bus.qbus, &virtser_bus_info, dev, NULL);
+    qbus_create_inplace(&vser->bus.qbus, TYPE_VIRTIO_SERIAL_BUS, dev, NULL);
     vser->bus.qbus.allow_hotplug = 1;
     vser->bus.vser = vser;
     QTAILQ_INIT(&vser->ports);
@@ -971,7 +980,7 @@ static void virtio_serial_port_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = virtser_port_qdev_init;
-    k->bus_info = &virtser_bus_info;
+    k->bus_type = TYPE_VIRTIO_SERIAL_BUS;
     k->exit = virtser_port_qdev_exit;
     k->unplug = qdev_simple_unplug_cb;
 }
@@ -999,6 +1008,7 @@ static TypeInfo virtio_serial_port_type_info = {
 
 static void virtio_serial_register_types(void)
 {
+    type_register_static(&virtser_bus_info);
     type_register_static(&virtio_serial_port_type_info);
 }
 
-- 
1.7.5.4

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

[Andreas Färber]

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 <aliguori@us.ibm.com>
> ---
[...]
> 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?

> 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 {.

> diff --git a/hw/qdev.h b/hw/qdev.h
> index 6d2261f..8ac703e 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
[...]
> @@ -79,28 +79,30 @@ struct DeviceState {
>      int alias_required_for_version;
>  };
>  
> -typedef void (*bus_dev_printfn)(Monitor *mon, DeviceState *dev, int indent);
> -typedef char *(*bus_get_dev_path)(DeviceState *dev);
>  /*
>   * This callback is used to create Open Firmware device path in accordance with
>   * OF spec http://forthworks.com/standards/of1275.pdf. Indicidual bus bindings
>   * can be found here http://playground.sun.com/1275/bindings/.
>   */
> -typedef char *(*bus_get_fw_dev_path)(DeviceState *dev);
> -typedef int (qbus_resetfn)(BusState *bus);
>  
> -struct BusInfo {
> -    const char *name;
> -    size_t size;
> -    bus_dev_printfn print_dev;
> -    bus_get_dev_path get_dev_path;
> -    bus_get_fw_dev_path get_fw_dev_path;
> -    qbus_resetfn *reset;
> +#define TYPE_BUS "bus"
> +#define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
> +#define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
> +#define BUS_GET_CLASS(obj) OBJECT_GET_CLASS(BusClass, (obj), TYPE_BUS)
> +
> +struct BusClass {
> +    ObjectClass parent_class;
> +
> +    /* FIXME first arg should be BusState */
> +    void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
> +    char *(*get_dev_path)(DeviceState *dev);
> +    char *(*get_fw_dev_path)(DeviceState *dev);
> +    int (*reset)(BusState *bus);
>  };
>  
>  struct BusState {
> +    Object obj;
>      DeviceState *parent;
> -    BusInfo *info;
>      const char *name;
>      int allow_hotplug;
>      int qdev_allocated;

Indicate what is /*< private >*/ and what not?

> @@ -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.

Otherwise looks okay, thanks for your work on this.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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<aliguori@us.ibm.com>
>> ---
> [...]
>> 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

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

[Anthony Liguori]

This makes sysbus part of the root hierarchy and all busses children of their
respective parent DeviceState.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - move sysbus to /machine/unattached (Andreas)
---
 hw/qdev.c    |   12 ++++++------
 qom/object.c |   12 ++++++++++++
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 19d510e..87ff1a5 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -428,6 +428,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. */
@@ -457,6 +458,9 @@ 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(container_get(qdev_get_machine(),
+                                            "/unattached"),
+                              "sysbus", OBJECT(main_system_bus), NULL);
 }
 
 void qbus_free(BusState *bus)
@@ -538,11 +542,6 @@ char *qdev_get_dev_path(DeviceState *dev)
     return NULL;
 }
 
-static char *qdev_get_type(Object *obj, Error **errp)
-{
-    return g_strdup(object_get_typename(obj));
-}
-
 /**
  * Legacy property handling
  */
@@ -658,7 +657,8 @@ static void device_initfn(Object *obj)
 
     qdev_add_properties(dev, dc->props);
 
-    object_property_add_str(OBJECT(dev), "type", qdev_get_type, NULL, NULL);
+    object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
+                             (Object **)&dev->parent_bus, NULL);
 }
 
 /* Unlink device from bus and free the structure.  */
diff --git a/qom/object.c b/qom/object.c
index 94928c5..0268f2a 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -256,12 +256,24 @@ static void object_interface_init(Object *obj, InterfaceImpl *iface)
     obj->interfaces = g_slist_prepend(obj->interfaces, iface_obj);
 }
 
+static char *object_get_typename_dup(Object *obj, Error **errp)
+{
+    return g_strdup(object_get_typename(obj));
+}
+
+static void object_base_init(Object *obj)
+{
+    object_property_add_str(obj, "type", object_get_typename_dup, NULL, NULL);
+}
+
 static void object_init_with_type(Object *obj, TypeImpl *ti)
 {
     int i;
 
     if (type_has_parent(ti)) {
         object_init_with_type(obj, type_get_parent(ti));
+    } else {
+        object_base_init(obj);
     }
 
     for (i = 0; i < ti->num_interfaces; i++) {
-- 
1.7.5.4

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

[Andreas Färber]

Am 01.05.2012 20:18, 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 <aliguori@us.ibm.com>

This conflicts with Paolo's patch set but since this one is less
intrusive (hardcoding the base init rather than renaming the base class
to non-null) and the other series appears not to get merged for 1.1,

Reviewed-by: Andreas Färber <afaerber@suse.de>

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 10/14] qbus: move get_dev_path to DeviceState

[Anthony Liguori]

It should have never been a bus method.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/pci.c      |   75 ++++++++++++++++++++++++++++-----------------------------
 hw/qdev.c     |   11 ++------
 hw/qdev.h     |    2 +-
 hw/scsi-bus.c |   10 ++++----
 hw/usb/bus.c  |   41 +++++++++++++++----------------
 5 files changed, 66 insertions(+), 73 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index bff303b..291181e 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -40,7 +40,6 @@
 #endif
 
 static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-static char *pcibus_get_dev_path(DeviceState *dev);
 static char *pcibus_get_fw_dev_path(DeviceState *dev);
 static int pcibus_reset(BusState *qbus);
 
@@ -49,7 +48,6 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
     BusClass *k = BUS_CLASS(klass);
 
     k->print_dev = pcibus_dev_print;
-    k->get_dev_path = pcibus_get_dev_path;
     k->get_fw_dev_path = pcibus_get_fw_dev_path;
     k->reset = pcibus_reset;
 }
@@ -1899,7 +1897,42 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev)
     return strdup(path);
 }
 
-static char *pcibus_get_dev_path(DeviceState *dev)
+static int pci_qdev_find_recursive(PCIBus *bus,
+                                   const char *id, PCIDevice **pdev)
+{
+    DeviceState *qdev = qdev_find_recursive(&bus->qbus, id);
+    if (!qdev) {
+        return -ENODEV;
+    }
+
+    /* roughly check if given qdev is pci device */
+    if (object_dynamic_cast(OBJECT(qdev), TYPE_PCI_DEVICE)) {
+        *pdev = PCI_DEVICE(qdev);
+        return 0;
+    }
+    return -EINVAL;
+}
+
+int pci_qdev_find_device(const char *id, PCIDevice **pdev)
+{
+    struct PCIHostBus *host;
+    int rc = -ENODEV;
+
+    QLIST_FOREACH(host, &host_buses, next) {
+        int tmp = pci_qdev_find_recursive(host->bus, id, pdev);
+        if (!tmp) {
+            rc = 0;
+            break;
+        }
+        if (tmp != -ENODEV) {
+            rc = tmp;
+        }
+    }
+
+    return rc;
+}
+
+static char *pci_qdev_get_dev_path(DeviceState *dev)
 {
     PCIDevice *d = container_of(dev, PCIDevice, qdev);
     PCIDevice *t;
@@ -1948,41 +1981,6 @@ static char *pcibus_get_dev_path(DeviceState *dev)
     return path;
 }
 
-static int pci_qdev_find_recursive(PCIBus *bus,
-                                   const char *id, PCIDevice **pdev)
-{
-    DeviceState *qdev = qdev_find_recursive(&bus->qbus, id);
-    if (!qdev) {
-        return -ENODEV;
-    }
-
-    /* roughly check if given qdev is pci device */
-    if (object_dynamic_cast(OBJECT(qdev), TYPE_PCI_DEVICE)) {
-        *pdev = PCI_DEVICE(qdev);
-        return 0;
-    }
-    return -EINVAL;
-}
-
-int pci_qdev_find_device(const char *id, PCIDevice **pdev)
-{
-    struct PCIHostBus *host;
-    int rc = -ENODEV;
-
-    QLIST_FOREACH(host, &host_buses, next) {
-        int tmp = pci_qdev_find_recursive(host->bus, id, pdev);
-        if (!tmp) {
-            rc = 0;
-            break;
-        }
-        if (tmp != -ENODEV) {
-            rc = tmp;
-        }
-    }
-
-    return rc;
-}
-
 MemoryRegion *pci_address_space(PCIDevice *dev)
 {
     return dev->bus->address_space_mem;
@@ -2000,6 +1998,7 @@ static void pci_device_class_init(ObjectClass *klass, void *data)
     k->unplug = pci_unplug_device;
     k->exit = pci_unregister_device;
     k->bus_type = TYPE_PCI_BUS;
+    k->get_dev_path = pci_qdev_get_dev_path;
 }
 
 static Property pci_bus_properties[] = {
diff --git a/hw/qdev.c b/hw/qdev.c
index 87ff1a5..eaa3e12 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -528,15 +528,10 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
 
 char *qdev_get_dev_path(DeviceState *dev)
 {
-    BusClass *bc;
-
-    if (!dev->parent_bus) {
-        return NULL;
-    }
+    DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
-    bc = BUS_GET_CLASS(dev->parent_bus);
-    if (bc->get_dev_path) {
-        return bc->get_dev_path(dev);
+    if (dc->get_dev_path) {
+        return dc->get_dev_path(dev);
     }
 
     return NULL;
diff --git a/hw/qdev.h b/hw/qdev.h
index 8ac703e..30bfbef 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -47,6 +47,7 @@ typedef struct DeviceClass {
 
     /* callbacks */
     void (*reset)(DeviceState *dev);
+    char *(*get_dev_path)(DeviceState *dev);
 
     /* device state */
     const VMStateDescription *vmsd;
@@ -95,7 +96,6 @@ struct BusClass {
 
     /* FIXME first arg should be BusState */
     void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
-    char *(*get_dev_path)(DeviceState *dev);
     char *(*get_fw_dev_path)(DeviceState *dev);
     int (*reset)(BusState *bus);
 };
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 9949192..38189f3 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -16,7 +16,6 @@ static void scsi_bus_class_init(ObjectClass *klass, void *data)
 {
     BusClass *k = BUS_CLASS(klass);
 
-    k->get_dev_path = scsibus_get_dev_path;
     k->get_fw_dev_path = scsibus_get_fw_dev_path;
 }
 
@@ -1428,15 +1427,15 @@ void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense)
     sdev->unit_attention = sense;
 }
 
-static char *scsibus_get_dev_path(DeviceState *dev)
+static char *scsi_qdev_get_dev_path(DeviceState *dev)
 {
-    SCSIDevice *d = DO_UPCAST(SCSIDevice, qdev, dev);
+    SCSIDevice *d = SCSI_DEVICE(dev);
     DeviceState *hba = dev->parent_bus->parent;
     char *id = NULL;
     char *path;
 
-    if (hba && hba->parent_bus && hba->parent_bus->info->get_dev_path) {
-        id = hba->parent_bus->info->get_dev_path(hba);
+    if (hba) {
+        id = qdev_get_dev_path(hba);
     }
     if (id) {
         path = g_strdup_printf("%s/%d:%d:%d", id, d->channel, d->id, d->lun);
@@ -1579,6 +1578,7 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
     k->init     = scsi_qdev_init;
     k->unplug   = qdev_simple_unplug_cb;
     k->exit     = scsi_qdev_exit;
+    k->get_dev_path = scsi_qdev_get_dev_path;
 }
 
 static Property scsi_bus_properties[] = {
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index e2a87ed..9b57d1c 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -7,7 +7,6 @@
 
 static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
 
-static char *usb_get_dev_path(DeviceState *dev);
 static char *usb_get_fw_dev_path(DeviceState *qdev);
 static int usb_qdev_exit(DeviceState *qdev);
 
@@ -18,7 +17,6 @@ static void usb_bus_class_init(ObjectClass *klass, void *data)
     BusClass *k = BUS_CLASS(klass);
 
     k->print_dev = usb_bus_dev_print;
-    k->get_dev_path = usb_get_dev_path;
     k->get_fw_dev_path = usb_get_fw_dev_path;
 }
 
@@ -464,25 +462,6 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
                    dev->attached ? ", attached" : "");
 }
 
-static char *usb_get_dev_path(DeviceState *qdev)
-{
-    USBDevice *dev = USB_DEVICE(qdev);
-    DeviceState *hcd = qdev->parent_bus->parent;
-    char *id = NULL;
-
-    if ((dev->flags & (1 << USB_DEV_FLAG_FULL_PATH)) &&
-        hcd && hcd->parent_bus && hcd->parent_bus->info->get_dev_path) {
-        id = hcd->parent_bus->info->get_dev_path(hcd);
-    }
-    if (id) {
-        char *ret = g_strdup_printf("%s/%s", id, dev->port->path);
-        g_free(id);
-        return ret;
-    } else {
-        return g_strdup(dev->port->path);
-    }
-}
-
 static char *usb_get_fw_dev_path(DeviceState *qdev)
 {
     USBDevice *dev = USB_DEVICE(qdev);
@@ -578,13 +557,33 @@ USBDevice *usbdevice_create(const char *cmdline)
     return f->usbdevice_init(bus, params);
 }
 
+static char *usb_qdev_get_dev_path(DeviceState *qdev)
+{
+    USBDevice *dev = USB_DEVICE(qdev);
+    DeviceState *hcd = qdev->parent_bus->parent;
+    char *id = NULL;
+
+    if ((dev->flags & (1 << USB_DEV_FLAG_FULL_PATH))) {
+        id = qdev_get_dev_path(hcd);
+    }
+    if (id) {
+        char *ret = g_strdup_printf("%s/%s", id, dev->port->path);
+        g_free(id);
+        return ret;
+    } else {
+        return g_strdup(dev->port->path);
+    }
+}
+
 static void usb_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
+
     k->bus_type = TYPE_USB_BUS;
     k->init     = usb_qdev_init;
     k->unplug   = qdev_simple_unplug_cb;
     k->exit     = usb_qdev_exit;
+    k->get_dev_path = usb_qdev_get_dev_path;
 }
 
 static Property usb_bus_properties[] = {
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 10/14] qbus: move get_dev_path to DeviceState

[Paolo Bonzini]

Il 01/05/2012 20:18, Anthony Liguori ha scritto:
> It should have never been a bus method.

If in the long term you want slots to be child properties in the bus,
the method _will_ actually belong to buses.

It is clear cut for print_dev, but not so much for the others.

Paolo

> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  hw/pci.c      |   75 ++++++++++++++++++++++++++++-----------------------------
>  hw/qdev.c     |   11 ++------
>  hw/qdev.h     |    2 +-
>  hw/scsi-bus.c |   10 ++++----
>  hw/usb/bus.c  |   41 +++++++++++++++----------------
>  5 files changed, 66 insertions(+), 73 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index bff303b..291181e 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -40,7 +40,6 @@
>  #endif
>  
>  static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
> -static char *pcibus_get_dev_path(DeviceState *dev);
>  static char *pcibus_get_fw_dev_path(DeviceState *dev);
>  static int pcibus_reset(BusState *qbus);
>  
> @@ -49,7 +48,6 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
>      BusClass *k = BUS_CLASS(klass);
>  
>      k->print_dev = pcibus_dev_print;
> -    k->get_dev_path = pcibus_get_dev_path;
>      k->get_fw_dev_path = pcibus_get_fw_dev_path;
>      k->reset = pcibus_reset;
>  }
> @@ -1899,7 +1897,42 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev)
>      return strdup(path);
>  }
>  
> -static char *pcibus_get_dev_path(DeviceState *dev)
> +static int pci_qdev_find_recursive(PCIBus *bus,
> +                                   const char *id, PCIDevice **pdev)
> +{
> +    DeviceState *qdev = qdev_find_recursive(&bus->qbus, id);
> +    if (!qdev) {
> +        return -ENODEV;
> +    }
> +
> +    /* roughly check if given qdev is pci device */
> +    if (object_dynamic_cast(OBJECT(qdev), TYPE_PCI_DEVICE)) {
> +        *pdev = PCI_DEVICE(qdev);
> +        return 0;
> +    }
> +    return -EINVAL;
> +}
> +
> +int pci_qdev_find_device(const char *id, PCIDevice **pdev)
> +{
> +    struct PCIHostBus *host;
> +    int rc = -ENODEV;
> +
> +    QLIST_FOREACH(host, &host_buses, next) {
> +        int tmp = pci_qdev_find_recursive(host->bus, id, pdev);
> +        if (!tmp) {
> +            rc = 0;
> +            break;
> +        }
> +        if (tmp != -ENODEV) {
> +            rc = tmp;
> +        }
> +    }
> +
> +    return rc;
> +}
> +
> +static char *pci_qdev_get_dev_path(DeviceState *dev)
>  {
>      PCIDevice *d = container_of(dev, PCIDevice, qdev);
>      PCIDevice *t;
> @@ -1948,41 +1981,6 @@ static char *pcibus_get_dev_path(DeviceState *dev)
>      return path;
>  }
>  
> -static int pci_qdev_find_recursive(PCIBus *bus,
> -                                   const char *id, PCIDevice **pdev)
> -{
> -    DeviceState *qdev = qdev_find_recursive(&bus->qbus, id);
> -    if (!qdev) {
> -        return -ENODEV;
> -    }
> -
> -    /* roughly check if given qdev is pci device */
> -    if (object_dynamic_cast(OBJECT(qdev), TYPE_PCI_DEVICE)) {
> -        *pdev = PCI_DEVICE(qdev);
> -        return 0;
> -    }
> -    return -EINVAL;
> -}
> -
> -int pci_qdev_find_device(const char *id, PCIDevice **pdev)
> -{
> -    struct PCIHostBus *host;
> -    int rc = -ENODEV;
> -
> -    QLIST_FOREACH(host, &host_buses, next) {
> -        int tmp = pci_qdev_find_recursive(host->bus, id, pdev);
> -        if (!tmp) {
> -            rc = 0;
> -            break;
> -        }
> -        if (tmp != -ENODEV) {
> -            rc = tmp;
> -        }
> -    }
> -
> -    return rc;
> -}
> -
>  MemoryRegion *pci_address_space(PCIDevice *dev)
>  {
>      return dev->bus->address_space_mem;
> @@ -2000,6 +1998,7 @@ static void pci_device_class_init(ObjectClass *klass, void *data)
>      k->unplug = pci_unplug_device;
>      k->exit = pci_unregister_device;
>      k->bus_type = TYPE_PCI_BUS;
> +    k->get_dev_path = pci_qdev_get_dev_path;
>  }
>  
>  static Property pci_bus_properties[] = {
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 87ff1a5..eaa3e12 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -528,15 +528,10 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
>  
>  char *qdev_get_dev_path(DeviceState *dev)
>  {
> -    BusClass *bc;
> -
> -    if (!dev->parent_bus) {
> -        return NULL;
> -    }
> +    DeviceClass *dc = DEVICE_GET_CLASS(dev);
>  
> -    bc = BUS_GET_CLASS(dev->parent_bus);
> -    if (bc->get_dev_path) {
> -        return bc->get_dev_path(dev);
> +    if (dc->get_dev_path) {
> +        return dc->get_dev_path(dev);
>      }
>  
>      return NULL;
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 8ac703e..30bfbef 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -47,6 +47,7 @@ typedef struct DeviceClass {
>  
>      /* callbacks */
>      void (*reset)(DeviceState *dev);
> +    char *(*get_dev_path)(DeviceState *dev);
>  
>      /* device state */
>      const VMStateDescription *vmsd;
> @@ -95,7 +96,6 @@ struct BusClass {
>  
>      /* FIXME first arg should be BusState */
>      void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
> -    char *(*get_dev_path)(DeviceState *dev);
>      char *(*get_fw_dev_path)(DeviceState *dev);
>      int (*reset)(BusState *bus);
>  };
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index 9949192..38189f3 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -16,7 +16,6 @@ static void scsi_bus_class_init(ObjectClass *klass, void *data)
>  {
>      BusClass *k = BUS_CLASS(klass);
>  
> -    k->get_dev_path = scsibus_get_dev_path;
>      k->get_fw_dev_path = scsibus_get_fw_dev_path;
>  }
>  
> @@ -1428,15 +1427,15 @@ void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense)
>      sdev->unit_attention = sense;
>  }
>  
> -static char *scsibus_get_dev_path(DeviceState *dev)
> +static char *scsi_qdev_get_dev_path(DeviceState *dev)
>  {
> -    SCSIDevice *d = DO_UPCAST(SCSIDevice, qdev, dev);
> +    SCSIDevice *d = SCSI_DEVICE(dev);
>      DeviceState *hba = dev->parent_bus->parent;
>      char *id = NULL;
>      char *path;
>  
> -    if (hba && hba->parent_bus && hba->parent_bus->info->get_dev_path) {
> -        id = hba->parent_bus->info->get_dev_path(hba);
> +    if (hba) {
> +        id = qdev_get_dev_path(hba);
>      }
>      if (id) {
>          path = g_strdup_printf("%s/%d:%d:%d", id, d->channel, d->id, d->lun);
> @@ -1579,6 +1578,7 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
>      k->init     = scsi_qdev_init;
>      k->unplug   = qdev_simple_unplug_cb;
>      k->exit     = scsi_qdev_exit;
> +    k->get_dev_path = scsi_qdev_get_dev_path;
>  }
>  
>  static Property scsi_bus_properties[] = {
> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
> index e2a87ed..9b57d1c 100644
> --- a/hw/usb/bus.c
> +++ b/hw/usb/bus.c
> @@ -7,7 +7,6 @@
>  
>  static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
>  
> -static char *usb_get_dev_path(DeviceState *dev);
>  static char *usb_get_fw_dev_path(DeviceState *qdev);
>  static int usb_qdev_exit(DeviceState *qdev);
>  
> @@ -18,7 +17,6 @@ static void usb_bus_class_init(ObjectClass *klass, void *data)
>      BusClass *k = BUS_CLASS(klass);
>  
>      k->print_dev = usb_bus_dev_print;
> -    k->get_dev_path = usb_get_dev_path;
>      k->get_fw_dev_path = usb_get_fw_dev_path;
>  }
>  
> @@ -464,25 +462,6 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
>                     dev->attached ? ", attached" : "");
>  }
>  
> -static char *usb_get_dev_path(DeviceState *qdev)
> -{
> -    USBDevice *dev = USB_DEVICE(qdev);
> -    DeviceState *hcd = qdev->parent_bus->parent;
> -    char *id = NULL;
> -
> -    if ((dev->flags & (1 << USB_DEV_FLAG_FULL_PATH)) &&
> -        hcd && hcd->parent_bus && hcd->parent_bus->info->get_dev_path) {
> -        id = hcd->parent_bus->info->get_dev_path(hcd);
> -    }
> -    if (id) {
> -        char *ret = g_strdup_printf("%s/%s", id, dev->port->path);
> -        g_free(id);
> -        return ret;
> -    } else {
> -        return g_strdup(dev->port->path);
> -    }
> -}
> -
>  static char *usb_get_fw_dev_path(DeviceState *qdev)
>  {
>      USBDevice *dev = USB_DEVICE(qdev);
> @@ -578,13 +557,33 @@ USBDevice *usbdevice_create(const char *cmdline)
>      return f->usbdevice_init(bus, params);
>  }
>  
> +static char *usb_qdev_get_dev_path(DeviceState *qdev)
> +{
> +    USBDevice *dev = USB_DEVICE(qdev);
> +    DeviceState *hcd = qdev->parent_bus->parent;
> +    char *id = NULL;
> +
> +    if ((dev->flags & (1 << USB_DEV_FLAG_FULL_PATH))) {
> +        id = qdev_get_dev_path(hcd);
> +    }
> +    if (id) {
> +        char *ret = g_strdup_printf("%s/%s", id, dev->port->path);
> +        g_free(id);
> +        return ret;
> +    } else {
> +        return g_strdup(dev->port->path);
> +    }
> +}
> +
>  static void usb_device_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *k = DEVICE_CLASS(klass);
> +
>      k->bus_type = TYPE_USB_BUS;
>      k->init     = usb_qdev_init;
>      k->unplug   = qdev_simple_unplug_cb;
>      k->exit     = usb_qdev_exit;
> +    k->get_dev_path = usb_qdev_get_dev_path;
>  }
>  
>  static Property usb_bus_properties[] = {

[Qemu-devel] [PATCH 11/14] qbus: move get_fw_dev_path to DeviceClass

[Anthony Liguori]

It should have never been a bus method.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/ide/qdev.c |   33 +++++++++++++--------------------
 hw/isa-bus.c  |   31 +++++++++++++++----------------
 hw/pci.c      |   31 +++++++++++++++----------------
 hw/qdev.c     |   10 +++++-----
 hw/qdev.h     |    2 +-
 hw/scsi-bus.c |   33 ++++++++++++---------------------
 hw/sysbus.c   |   39 +++++++++++++++++++--------------------
 hw/usb/bus.c  |   55 +++++++++++++++++++++++++++----------------------------
 8 files changed, 107 insertions(+), 127 deletions(-)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 4a468f8..5044018 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -25,22 +25,13 @@
 
 /* --------------------------------- */
 
-static char *idebus_get_fw_dev_path(DeviceState *dev);
-
 #define TYPE_IDE_BUS "IDE"
-
-static void ide_bus_class_init(ObjectClass *klass, void *data)
-{
-    BusClass *k = BUS_CLASS(klass);
-
-    k->get_fw_dev_path = idebus_get_fw_dev_path;
-}
+#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
 
 static TypeInfo ide_bus_info = {
     .name = TYPE_IDE_BUS,
     .parent = TYPE_BUS,
     .instance_size = sizeof(IDEBus),
-    .class_init = ide_bus_class_init,
 };
 
 void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
@@ -49,16 +40,6 @@ void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
     idebus->bus_id = bus_id;
 }
 
-static char *idebus_get_fw_dev_path(DeviceState *dev)
-{
-    char path[30];
-
-    snprintf(path, sizeof(path), "%s@%d", qdev_fw_name(dev),
-             ((IDEBus*)dev->parent_bus)->bus_id);
-
-    return strdup(path);
-}
-
 static int ide_qdev_init(DeviceState *qdev)
 {
     IDEDevice *dev = IDE_DEVICE(qdev);
@@ -250,11 +231,23 @@ static TypeInfo ide_drive_info = {
     .class_init    = ide_drive_class_init,
 };
 
+static char *ide_device_get_fw_dev_path(DeviceState *dev)
+{
+    IDEBus *parent_bus = IDE_BUS(dev->parent_bus);
+    char path[30];
+
+    snprintf(path, sizeof(path), "%s@%d", qdev_fw_name(dev),
+             parent_bus->bus_id);
+
+    return g_strdup(path);
+}
+
 static void ide_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = ide_qdev_init;
     k->bus_type = TYPE_IDE_BUS;
+    k->get_fw_dev_path = ide_device_get_fw_dev_path;
 }
 
 static Property ide_bus_properties[] = {
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 2616f22..6141515 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -26,7 +26,6 @@ static ISABus *isabus;
 target_phys_addr_t isa_mem_base = 0;
 
 static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-static char *isabus_get_fw_dev_path(DeviceState *dev);
 
 #define TYPE_ISA_BUS "ISA"
 
@@ -35,7 +34,6 @@ static void isa_bus_class_init(ObjectClass *klass, void *data)
     BusClass *k = BUS_CLASS(klass);
 
     k->print_dev = isabus_dev_print;
-    k->get_fw_dev_path = isabus_get_fw_dev_path;
 }
 
 static TypeInfo isa_bus_info = {
@@ -204,11 +202,26 @@ static TypeInfo isabus_bridge_info = {
     .class_init    = isabus_bridge_class_init,
 };
 
+static char *isa_device_get_fw_dev_path(DeviceState *dev)
+{
+    ISADevice *d = (ISADevice*)dev;
+    char path[40];
+    int off;
+
+    off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
+    if (d->ioport_id) {
+        snprintf(path + off, sizeof(path) - off, "@%04x", d->ioport_id);
+    }
+
+    return strdup(path);
+}
+
 static void isa_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = isa_qdev_init;
     k->bus_type = TYPE_ISA_BUS;
+    k->get_fw_dev_path = isa_device_get_fw_dev_path;
 }
 
 static TypeInfo isa_device_type_info = {
@@ -227,20 +240,6 @@ static void isabus_register_types(void)
     type_register_static(&isa_device_type_info);
 }
 
-static char *isabus_get_fw_dev_path(DeviceState *dev)
-{
-    ISADevice *d = (ISADevice*)dev;
-    char path[40];
-    int off;
-
-    off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
-    if (d->ioport_id) {
-        snprintf(path + off, sizeof(path) - off, "@%04x", d->ioport_id);
-    }
-
-    return strdup(path);
-}
-
 MemoryRegion *isa_address_space(ISADevice *dev)
 {
     return get_system_memory();
diff --git a/hw/pci.c b/hw/pci.c
index 291181e..425ceaa 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -40,7 +40,6 @@
 #endif
 
 static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-static char *pcibus_get_fw_dev_path(DeviceState *dev);
 static int pcibus_reset(BusState *qbus);
 
 static void pci_bus_class_init(ObjectClass *klass, void *data)
@@ -48,7 +47,6 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
     BusClass *k = BUS_CLASS(klass);
 
     k->print_dev = pcibus_dev_print;
-    k->get_fw_dev_path = pcibus_get_fw_dev_path;
     k->reset = pcibus_reset;
 }
 
@@ -1883,20 +1881,6 @@ static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len)
     return buf;
 }
 
-static char *pcibus_get_fw_dev_path(DeviceState *dev)
-{
-    PCIDevice *d = (PCIDevice *)dev;
-    char path[50], name[33];
-    int off;
-
-    off = snprintf(path, sizeof(path), "%s@%x",
-                   pci_dev_fw_name(dev, name, sizeof name),
-                   PCI_SLOT(d->devfn));
-    if (PCI_FUNC(d->devfn))
-        snprintf(path + off, sizeof(path) + off, ",%x", PCI_FUNC(d->devfn));
-    return strdup(path);
-}
-
 static int pci_qdev_find_recursive(PCIBus *bus,
                                    const char *id, PCIDevice **pdev)
 {
@@ -1991,6 +1975,20 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
     return dev->bus->address_space_io;
 }
 
+static char *pci_qdev_get_fw_dev_path(DeviceState *dev)
+{
+    PCIDevice *d = PCI_DEVICE(dev);
+    char path[50], name[33];
+    int off;
+
+    off = snprintf(path, sizeof(path), "%s@%x",
+                   pci_dev_fw_name(dev, name, sizeof name),
+                   PCI_SLOT(d->devfn));
+    if (PCI_FUNC(d->devfn))
+        snprintf(path + off, sizeof(path) + off, ",%x", PCI_FUNC(d->devfn));
+    return strdup(path);
+}
+
 static void pci_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
@@ -1999,6 +1997,7 @@ static void pci_device_class_init(ObjectClass *klass, void *data)
     k->exit = pci_unregister_device;
     k->bus_type = TYPE_PCI_BUS;
     k->get_dev_path = pci_qdev_get_dev_path;
+    k->get_fw_dev_path = pci_qdev_get_fw_dev_path;
 }
 
 static Property pci_bus_properties[] = {
diff --git a/hw/qdev.c b/hw/qdev.c
index eaa3e12..bee0cd6 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -483,12 +483,12 @@ void qbus_free(BusState *bus)
     }
 }
 
-static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev)
+static char *do_qdev_get_fw_dev_path(DeviceState *dev)
 {
-    BusClass *bc = BUS_GET_CLASS(bus);
+    DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
-    if (bc->get_fw_dev_path) {
-        return bc->get_fw_dev_path(dev);
+    if (dc->get_fw_dev_path) {
+        return dc->get_fw_dev_path(dev);
     }
 
     return NULL;
@@ -501,7 +501,7 @@ static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
     if (dev && dev->parent_bus) {
         char *d;
         l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
-        d = bus_get_fw_dev_path(dev->parent_bus, dev);
+        d = do_qdev_get_fw_dev_path(dev);
         if (d) {
             l += snprintf(p + l, size - l, "%s", d);
             g_free(d);
diff --git a/hw/qdev.h b/hw/qdev.h
index 30bfbef..b09a07b 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -48,6 +48,7 @@ typedef struct DeviceClass {
     /* callbacks */
     void (*reset)(DeviceState *dev);
     char *(*get_dev_path)(DeviceState *dev);
+    char *(*get_fw_dev_path)(DeviceState *dev);
 
     /* device state */
     const VMStateDescription *vmsd;
@@ -96,7 +97,6 @@ struct BusClass {
 
     /* FIXME first arg should be BusState */
     void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
-    char *(*get_fw_dev_path)(DeviceState *dev);
     int (*reset)(BusState *bus);
 };
 
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 38189f3..98bb217 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -7,23 +7,13 @@
 #include "trace.h"
 #include "dma.h"
 
-static char *scsibus_get_dev_path(DeviceState *dev);
-static char *scsibus_get_fw_dev_path(DeviceState *dev);
 static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
 static void scsi_req_dequeue(SCSIRequest *req);
 
-static void scsi_bus_class_init(ObjectClass *klass, void *data)
-{
-    BusClass *k = BUS_CLASS(klass);
-
-    k->get_fw_dev_path = scsibus_get_fw_dev_path;
-}
-
 static TypeInfo scsi_bus_info = {
     .name = TYPE_SCSI_BUS,
     .parent = TYPE_BUS,
     .instance_size = sizeof(SCSIBus),
-    .class_init = scsi_bus_class_init,
 };
 static int next_scsi_bus;
 
@@ -1446,17 +1436,6 @@ static char *scsi_qdev_get_dev_path(DeviceState *dev)
     return path;
 }
 
-static char *scsibus_get_fw_dev_path(DeviceState *dev)
-{
-    SCSIDevice *d = SCSI_DEVICE(dev);
-    char path[100];
-
-    snprintf(path, sizeof(path), "channel@%x/%s@%x,%x", d->channel,
-             qdev_fw_name(dev), d->id, d->lun);
-
-    return strdup(path);
-}
-
 SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
 {
     DeviceState *qdev;
@@ -1571,6 +1550,17 @@ const VMStateDescription vmstate_scsi_device = {
     }
 };
 
+static char *scsi_qdev_get_fw_dev_path(DeviceState *dev)
+{
+    SCSIDevice *d = SCSI_DEVICE(dev);
+    char path[100];
+
+    snprintf(path, sizeof(path), "channel@%x/%s@%x,%x", d->channel,
+             qdev_fw_name(dev), d->id, d->lun);
+
+    return strdup(path);
+}
+
 static void scsi_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
@@ -1579,6 +1569,7 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
     k->unplug   = qdev_simple_unplug_cb;
     k->exit     = scsi_qdev_exit;
     k->get_dev_path = scsi_qdev_get_dev_path;
+    k->get_fw_dev_path = scsi_qdev_get_fw_dev_path;
 }
 
 static Property scsi_bus_properties[] = {
diff --git a/hw/sysbus.c b/hw/sysbus.c
index bad1bdc..92b86ba 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -22,14 +22,12 @@
 #include "exec-memory.h"
 
 static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-static char *sysbus_get_fw_dev_path(DeviceState *dev);
 
 static void system_bus_class_init(ObjectClass *klass, void *data)
 {
     BusClass *k = BUS_CLASS(klass);
 
     k->print_dev = sysbus_dev_print;
-    k->get_fw_dev_path = sysbus_get_fw_dev_path;
 }
 
 static TypeInfo system_bus_info = {
@@ -196,24 +194,6 @@ static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent)
     }
 }
 
-static char *sysbus_get_fw_dev_path(DeviceState *dev)
-{
-    SysBusDevice *s = sysbus_from_qdev(dev);
-    char path[40];
-    int off;
-
-    off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
-
-    if (s->num_mmio) {
-        snprintf(path + off, sizeof(path) - off, "@"TARGET_FMT_plx,
-                 s->mmio[0].addr);
-    } else if (s->num_pio) {
-        snprintf(path + off, sizeof(path) - off, "@i%04x", s->pio[0]);
-    }
-
-    return strdup(path);
-}
-
 void sysbus_add_memory(SysBusDevice *dev, target_phys_addr_t addr,
                        MemoryRegion *mem)
 {
@@ -248,11 +228,30 @@ MemoryRegion *sysbus_address_space(SysBusDevice *dev)
     return get_system_memory();
 }
 
+static char *sysbus_get_fw_dev_path(DeviceState *dev)
+{
+    SysBusDevice *s = sysbus_from_qdev(dev);
+    char path[40];
+    int off;
+
+    off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
+
+    if (s->num_mmio) {
+        snprintf(path + off, sizeof(path) - off, "@"TARGET_FMT_plx,
+                 s->mmio[0].addr);
+    } else if (s->num_pio) {
+        snprintf(path + off, sizeof(path) - off, "@i%04x", s->pio[0]);
+    }
+
+    return strdup(path);
+}
+
 static void sysbus_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = sysbus_device_init;
     k->bus_type = TYPE_SYSTEM_BUS;
+    k->get_fw_dev_path = sysbus_get_fw_dev_path;
 }
 
 static TypeInfo sysbus_device_type_info = {
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 9b57d1c..da39282 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -7,7 +7,6 @@
 
 static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
 
-static char *usb_get_fw_dev_path(DeviceState *qdev);
 static int usb_qdev_exit(DeviceState *qdev);
 
 #define TYPE_USB_BUS "usb-bus"
@@ -17,7 +16,6 @@ static void usb_bus_class_init(ObjectClass *klass, void *data)
     BusClass *k = BUS_CLASS(klass);
 
     k->print_dev = usb_bus_dev_print;
-    k->get_fw_dev_path = usb_get_fw_dev_path;
 }
 
 static TypeInfo usb_bus_info = {
@@ -462,32 +460,6 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
                    dev->attached ? ", attached" : "");
 }
 
-static char *usb_get_fw_dev_path(DeviceState *qdev)
-{
-    USBDevice *dev = USB_DEVICE(qdev);
-    char *fw_path, *in;
-    ssize_t pos = 0, fw_len;
-    long nr;
-
-    fw_len = 32 + strlen(dev->port->path) * 6;
-    fw_path = g_malloc(fw_len);
-    in = dev->port->path;
-    while (fw_len - pos > 0) {
-        nr = strtol(in, &in, 10);
-        if (in[0] == '.') {
-            /* some hub between root port and device */
-            pos += snprintf(fw_path + pos, fw_len - pos, "hub@%ld/", nr);
-            in++;
-        } else {
-            /* the device itself */
-            pos += snprintf(fw_path + pos, fw_len - pos, "%s@%ld",
-                            qdev_fw_name(qdev), nr);
-            break;
-        }
-    }
-    return fw_path;
-}
-
 void usb_info(Monitor *mon)
 {
     USBBus *bus;
@@ -575,6 +547,32 @@ static char *usb_qdev_get_dev_path(DeviceState *qdev)
     }
 }
 
+static char *usb_qdev_get_fw_dev_path(DeviceState *qdev)
+{
+    USBDevice *dev = USB_DEVICE(qdev);
+    char *fw_path, *in;
+    ssize_t pos = 0, fw_len;
+    long nr;
+
+    fw_len = 32 + strlen(dev->port->path) * 6;
+    fw_path = g_malloc(fw_len);
+    in = dev->port->path;
+    while (fw_len - pos > 0) {
+        nr = strtol(in, &in, 10);
+        if (in[0] == '.') {
+            /* some hub between root port and device */
+            pos += snprintf(fw_path + pos, fw_len - pos, "hub@%ld/", nr);
+            in++;
+        } else {
+            /* the device itself */
+            pos += snprintf(fw_path + pos, fw_len - pos, "%s@%ld",
+                            qdev_fw_name(qdev), nr);
+            break;
+        }
+    }
+    return fw_path;
+}
+
 static void usb_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
@@ -584,6 +582,7 @@ static void usb_device_class_init(ObjectClass *klass, void *data)
     k->unplug   = qdev_simple_unplug_cb;
     k->exit     = usb_qdev_exit;
     k->get_dev_path = usb_qdev_get_dev_path;
+    k->get_fw_dev_path = usb_qdev_get_fw_dev_path;
 }
 
 static Property usb_bus_properties[] = {
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 11/14] qbus: move get_fw_dev_path to DeviceClass

[Andreas Färber]

Am 01.05.2012 20:18, schrieb Anthony Liguori:
> It should have never been a bus method.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
[...]
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 4a468f8..5044018 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -25,22 +25,13 @@
>  
>  /* --------------------------------- */
>  
> -static char *idebus_get_fw_dev_path(DeviceState *dev);
> -
>  #define TYPE_IDE_BUS "IDE"
> -
> -static void ide_bus_class_init(ObjectClass *klass, void *data)
> -{
> -    BusClass *k = BUS_CLASS(klass);
> -
> -    k->get_fw_dev_path = idebus_get_fw_dev_path;
> -}
> +#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)

Move macro to preceding patch?

Otherwise looks good.

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 11/14] qbus: move get_fw_dev_path to DeviceClass

[Anthony Liguori]

On 05/01/2012 02:34 PM, Andreas Färber wrote:
> Am 01.05.2012 20:18, schrieb Anthony Liguori:
>> It should have never been a bus method.
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
> [...]
>> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
>> index 4a468f8..5044018 100644
>> --- a/hw/ide/qdev.c
>> +++ b/hw/ide/qdev.c
>> @@ -25,22 +25,13 @@
>>
>>   /* --------------------------------- */
>>
>> -static char *idebus_get_fw_dev_path(DeviceState *dev);
>> -
>>   #define TYPE_IDE_BUS "IDE"
>> -
>> -static void ide_bus_class_init(ObjectClass *klass, void *data)
>> -{
>> -    BusClass *k = BUS_CLASS(klass);
>> -
>> -    k->get_fw_dev_path = idebus_get_fw_dev_path;
>> -}
>> +#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
>
> Move macro to preceding patch?
>
> Otherwise looks good.

Do you mean an independent patch?

Regards,

Anthony Liguori

>
> /-F
>

Re: [Qemu-devel] [PATCH 11/14] qbus: move get_fw_dev_path to DeviceClass

[Andreas Färber]

Am 02.05.2012 00:24, schrieb Anthony Liguori:
> On 05/01/2012 02:34 PM, Andreas Färber wrote:
>> Am 01.05.2012 20:18, schrieb Anthony Liguori:
>>> +#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
>>
>> Move macro to preceding patch?
> 
> Do you mean an independent patch?

Sorry, I reviewed the patches in mail reception order. ;)

I meant 08/14 (qdev: convert busses to QEMU object model) where macros
for other bus types were introduced. Seemed like an oversight.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 11/14] qbus: move get_fw_dev_path to DeviceClass

[Paolo Bonzini]

Il 02/05/2012 00:36, Andreas Färber ha scritto:
> Sorry, I reviewed the patches in mail reception order. ;)
> 
> I meant 08/14 (qdev: convert busses to QEMU object model) where macros
> for other bus types were introduced. Seemed like an oversight.

A lot of these were missing, and furthermore they should be in a header
file.

Paolo

[Qemu-devel] [PATCH 12/14] qbus: move print_dev to DeviceClass

[Anthony Liguori]

It should have never been a bus method.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/isa-bus.c           |   37 ++++++++--------------
 hw/pci.c               |   79 +++++++++++++++++++++++------------------------
 hw/qdev-monitor.c      |   10 +++---
 hw/qdev.h              |    3 +-
 hw/sysbus.c            |   39 +++++++++--------------
 hw/usb/bus.c           |   35 ++++++++-------------
 hw/virtio-serial-bus.c |   32 +++++++------------
 7 files changed, 99 insertions(+), 136 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 6141515..07a9ae4 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -25,22 +25,12 @@
 static ISABus *isabus;
 target_phys_addr_t isa_mem_base = 0;
 
-static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-
 #define TYPE_ISA_BUS "ISA"
 
-static void isa_bus_class_init(ObjectClass *klass, void *data)
-{
-    BusClass *k = BUS_CLASS(klass);
-
-    k->print_dev = isabus_dev_print;
-}
-
 static TypeInfo isa_bus_info = {
     .name = TYPE_ISA_BUS,
     .parent = TYPE_BUS,
     .instance_size = sizeof(ISABus),
-    .class_init = isa_bus_class_init,
 };
 
 ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space_io)
@@ -166,19 +156,6 @@ ISADevice *isa_create_simple(ISABus *bus, const char *name)
     return dev;
 }
 
-static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
-{
-    ISADevice *d = ISA_DEVICE(dev);
-
-    if (d->isairq[1] != -1) {
-        monitor_printf(mon, "%*sisa irqs %d,%d\n", indent, "",
-                       d->isairq[0], d->isairq[1]);
-    } else if (d->isairq[0] != -1) {
-        monitor_printf(mon, "%*sisa irq %d\n", indent, "",
-                       d->isairq[0]);
-    }
-}
-
 static int isabus_bridge_init(SysBusDevice *dev)
 {
     /* nothing */
@@ -216,12 +193,26 @@ static char *isa_device_get_fw_dev_path(DeviceState *dev)
     return strdup(path);
 }
 
+static void isa_qdev_dev_print(DeviceState *dev, Monitor *mon, int indent)
+{
+    ISADevice *d = ISA_DEVICE(dev);
+
+    if (d->isairq[1] != -1) {
+        monitor_printf(mon, "%*sisa irqs %d,%d\n", indent, "",
+                       d->isairq[0], d->isairq[1]);
+    } else if (d->isairq[0] != -1) {
+        monitor_printf(mon, "%*sisa irq %d\n", indent, "",
+                       d->isairq[0]);
+    }
+}
+
 static void isa_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = isa_qdev_init;
     k->bus_type = TYPE_ISA_BUS;
     k->get_fw_dev_path = isa_device_get_fw_dev_path;
+    k->print_dev = isa_qdev_dev_print;
 }
 
 static TypeInfo isa_device_type_info = {
diff --git a/hw/pci.c b/hw/pci.c
index 425ceaa..39c44b2 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -39,14 +39,12 @@
 # define PCI_DPRINTF(format, ...)       do { } while (0)
 #endif
 
-static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
 static int pcibus_reset(BusState *qbus);
 
 static void pci_bus_class_init(ObjectClass *klass, void *data)
 {
     BusClass *k = BUS_CLASS(klass);
 
-    k->print_dev = pcibus_dev_print;
     k->reset = pcibus_reset;
 }
 
@@ -1815,44 +1813,6 @@ uint8_t pci_find_capability(PCIDevice *pdev, uint8_t cap_id)
     return pci_find_capability_list(pdev, cap_id, NULL);
 }
 
-static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
-{
-    PCIDevice *d = (PCIDevice *)dev;
-    const pci_class_desc *desc;
-    char ctxt[64];
-    PCIIORegion *r;
-    int i, class;
-
-    class = pci_get_word(d->config + PCI_CLASS_DEVICE);
-    desc = pci_class_descriptions;
-    while (desc->desc && class != desc->class)
-        desc++;
-    if (desc->desc) {
-        snprintf(ctxt, sizeof(ctxt), "%s", desc->desc);
-    } else {
-        snprintf(ctxt, sizeof(ctxt), "Class %04x", class);
-    }
-
-    monitor_printf(mon, "%*sclass %s, addr %02x:%02x.%x, "
-                   "pci id %04x:%04x (sub %04x:%04x)\n",
-                   indent, "", ctxt, pci_bus_num(d->bus),
-                   PCI_SLOT(d->devfn), PCI_FUNC(d->devfn),
-                   pci_get_word(d->config + PCI_VENDOR_ID),
-                   pci_get_word(d->config + PCI_DEVICE_ID),
-                   pci_get_word(d->config + PCI_SUBSYSTEM_VENDOR_ID),
-                   pci_get_word(d->config + PCI_SUBSYSTEM_ID));
-    for (i = 0; i < PCI_NUM_REGIONS; i++) {
-        r = &d->io_regions[i];
-        if (!r->size)
-            continue;
-        monitor_printf(mon, "%*sbar %d: %s at 0x%"FMT_PCIBUS
-                       " [0x%"FMT_PCIBUS"]\n",
-                       indent, "",
-                       i, r->type & PCI_BASE_ADDRESS_SPACE_IO ? "i/o" : "mem",
-                       r->addr, r->addr + r->size - 1);
-    }
-}
-
 static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len)
 {
     PCIDevice *d = (PCIDevice *)dev;
@@ -1989,6 +1949,44 @@ static char *pci_qdev_get_fw_dev_path(DeviceState *dev)
     return strdup(path);
 }
 
+static void pci_qdev_dev_print(DeviceState *dev, Monitor *mon, int indent)
+{
+    PCIDevice *d = PCI_DEVICE(dev);
+    const pci_class_desc *desc;
+    char ctxt[64];
+    PCIIORegion *r;
+    int i, class;
+
+    class = pci_get_word(d->config + PCI_CLASS_DEVICE);
+    desc = pci_class_descriptions;
+    while (desc->desc && class != desc->class)
+        desc++;
+    if (desc->desc) {
+        snprintf(ctxt, sizeof(ctxt), "%s", desc->desc);
+    } else {
+        snprintf(ctxt, sizeof(ctxt), "Class %04x", class);
+    }
+
+    monitor_printf(mon, "%*sclass %s, addr %02x:%02x.%x, "
+                   "pci id %04x:%04x (sub %04x:%04x)\n",
+                   indent, "", ctxt, pci_bus_num(d->bus),
+                   PCI_SLOT(d->devfn), PCI_FUNC(d->devfn),
+                   pci_get_word(d->config + PCI_VENDOR_ID),
+                   pci_get_word(d->config + PCI_DEVICE_ID),
+                   pci_get_word(d->config + PCI_SUBSYSTEM_VENDOR_ID),
+                   pci_get_word(d->config + PCI_SUBSYSTEM_ID));
+    for (i = 0; i < PCI_NUM_REGIONS; i++) {
+        r = &d->io_regions[i];
+        if (!r->size)
+            continue;
+        monitor_printf(mon, "%*sbar %d: %s at 0x%"FMT_PCIBUS
+                       " [0x%"FMT_PCIBUS"]\n",
+                       indent, "",
+                       i, r->type & PCI_BASE_ADDRESS_SPACE_IO ? "i/o" : "mem",
+                       r->addr, r->addr + r->size - 1);
+    }
+}
+
 static void pci_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
@@ -1998,6 +1996,7 @@ static void pci_device_class_init(ObjectClass *klass, void *data)
     k->bus_type = TYPE_PCI_BUS;
     k->get_dev_path = pci_qdev_get_dev_path;
     k->get_fw_dev_path = pci_qdev_get_fw_dev_path;
+    k->print_dev = pci_qdev_dev_print;
 }
 
 static Property pci_bus_properties[] = {
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 97673dd..42c088e 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -518,12 +518,12 @@ static void qdev_print_prop(Object *obj, const char *name,
     }
 }
 
-static void bus_print_dev(BusState *bus, Monitor *mon, DeviceState *dev, int indent)
+static void qdev_print_dev(DeviceState *dev, Monitor *mon, int indent)
 {
-    BusClass *bc = BUS_GET_CLASS(bus);
+    DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
-    if (bc->print_dev) {
-        bc->print_dev(mon, dev, indent);
+    if (dc->print_dev) {
+        dc->print_dev(dev, mon, indent);
     }
 }
 
@@ -545,7 +545,7 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
         qdev_printf("gpio-out %d\n", dev->num_gpio_out);
     }
     object_property_foreach(OBJECT(dev), qdev_print_prop, &printer);
-    bus_print_dev(dev->parent_bus, mon, dev, indent + 2);
+    qdev_print_dev(dev, mon, indent + 2);
     QLIST_FOREACH(child, &dev->child_bus, sibling) {
         qbus_print(mon, child, indent + 2);
     }
diff --git a/hw/qdev.h b/hw/qdev.h
index b09a07b..2141bba 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -49,6 +49,7 @@ typedef struct DeviceClass {
     void (*reset)(DeviceState *dev);
     char *(*get_dev_path)(DeviceState *dev);
     char *(*get_fw_dev_path)(DeviceState *dev);
+    void (*print_dev)(DeviceState *dev, Monitor *mon, int indent);
 
     /* device state */
     const VMStateDescription *vmsd;
@@ -95,8 +96,6 @@ struct DeviceState {
 struct BusClass {
     ObjectClass parent_class;
 
-    /* FIXME first arg should be BusState */
-    void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
     int (*reset)(BusState *bus);
 };
 
diff --git a/hw/sysbus.c b/hw/sysbus.c
index 92b86ba..19de287 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -21,20 +21,10 @@
 #include "monitor.h"
 #include "exec-memory.h"
 
-static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-
-static void system_bus_class_init(ObjectClass *klass, void *data)
-{
-    BusClass *k = BUS_CLASS(klass);
-
-    k->print_dev = sysbus_dev_print;
-}
-
 static TypeInfo system_bus_info = {
     .name = TYPE_SYSTEM_BUS,
     .parent = TYPE_BUS,
     .instance_size = sizeof(BusState),
-    .class_init = system_bus_class_init,
 };
 
 void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
@@ -180,20 +170,6 @@ DeviceState *sysbus_try_create_varargs(const char *name,
     return dev;
 }
 
-static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent)
-{
-    SysBusDevice *s = sysbus_from_qdev(dev);
-    target_phys_addr_t size;
-    int i;
-
-    monitor_printf(mon, "%*sirq %d\n", indent, "", s->num_irq);
-    for (i = 0; i < s->num_mmio; i++) {
-        size = memory_region_size(s->mmio[i].memory);
-        monitor_printf(mon, "%*smmio " TARGET_FMT_plx "/" TARGET_FMT_plx "\n",
-                       indent, "", s->mmio[i].addr, size);
-    }
-}
-
 void sysbus_add_memory(SysBusDevice *dev, target_phys_addr_t addr,
                        MemoryRegion *mem)
 {
@@ -246,12 +222,27 @@ static char *sysbus_get_fw_dev_path(DeviceState *dev)
     return strdup(path);
 }
 
+static void sysbus_dev_print(DeviceState *dev, Monitor *mon, int indent)
+{
+    SysBusDevice *s = sysbus_from_qdev(dev);
+    target_phys_addr_t size;
+    int i;
+
+    monitor_printf(mon, "%*sirq %d\n", indent, "", s->num_irq);
+    for (i = 0; i < s->num_mmio; i++) {
+        size = memory_region_size(s->mmio[i].memory);
+        monitor_printf(mon, "%*smmio " TARGET_FMT_plx "/" TARGET_FMT_plx "\n",
+                       indent, "", s->mmio[i].addr, size);
+    }
+}
+
 static void sysbus_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = sysbus_device_init;
     k->bus_type = TYPE_SYSTEM_BUS;
     k->get_fw_dev_path = sysbus_get_fw_dev_path;
+    k->print_dev = sysbus_dev_print;
 }
 
 static TypeInfo sysbus_device_type_info = {
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index da39282..b132d0a 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -5,24 +5,14 @@
 #include "monitor.h"
 #include "trace.h"
 
-static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
-
 static int usb_qdev_exit(DeviceState *qdev);
 
 #define TYPE_USB_BUS "usb-bus"
 
-static void usb_bus_class_init(ObjectClass *klass, void *data)
-{
-    BusClass *k = BUS_CLASS(klass);
-
-    k->print_dev = usb_bus_dev_print;
-}
-
 static TypeInfo usb_bus_info = {
     .name = TYPE_USB_BUS,
     .parent = TYPE_BUS,
     .instance_size = sizeof(USBBus),
-    .class_init = usb_bus_class_init,
 };
 
 static int next_usb_bus = 0;
@@ -448,18 +438,6 @@ static const char *usb_speed(unsigned int speed)
     return txt[speed];
 }
 
-static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
-{
-    USBDevice *dev = USB_DEVICE(qdev);
-    USBBus *bus = usb_bus_from_device(dev);
-
-    monitor_printf(mon, "%*saddr %d.%d, port %s, speed %s, name %s%s\n",
-                   indent, "", bus->busnr, dev->addr,
-                   dev->port ? dev->port->path : "-",
-                   usb_speed(dev->speed), dev->product_desc,
-                   dev->attached ? ", attached" : "");
-}
-
 void usb_info(Monitor *mon)
 {
     USBBus *bus;
@@ -573,6 +551,18 @@ static char *usb_qdev_get_fw_dev_path(DeviceState *qdev)
     return fw_path;
 }
 
+static void usb_qdev_dev_print(DeviceState *qdev, Monitor *mon, int indent)
+{
+    USBDevice *dev = USB_DEVICE(qdev);
+    USBBus *bus = usb_bus_from_device(dev);
+
+    monitor_printf(mon, "%*saddr %d.%d, port %s, speed %s, name %s%s\n",
+                   indent, "", bus->busnr, dev->addr,
+                   dev->port ? dev->port->path : "-",
+                   usb_speed(dev->speed), dev->product_desc,
+                   dev->attached ? ", attached" : "");
+}
+
 static void usb_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
@@ -583,6 +573,7 @@ static void usb_device_class_init(ObjectClass *klass, void *data)
     k->exit     = usb_qdev_exit;
     k->get_dev_path = usb_qdev_get_dev_path;
     k->get_fw_dev_path = usb_qdev_get_fw_dev_path;
+    k->print_dev = usb_qdev_dev_print;
 }
 
 static Property usb_bus_properties[] = {
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 459219a..b15ea0d 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -722,34 +722,14 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
-static void virtser_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
-
 #define TYPE_VIRTIO_SERIAL_BUS "virtio-serial-bus"
 
-static void virtser_bus_class_init(ObjectClass *klass, void *data)
-{
-    BusClass *k = BUS_CLASS(klass);
-    k->print_dev = virtser_bus_dev_print;
-}
-
 static TypeInfo virtser_bus_info = {
     .name = TYPE_VIRTIO_SERIAL_BUS,
     .parent = TYPE_BUS,
     .instance_size = sizeof(VirtIOSerialBus),
-    .class_init = virtser_bus_class_init,
 };
 
-static void virtser_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
-{
-    VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, qdev);
-
-    monitor_printf(mon, "%*sport %d, guest %s, host %s, throttle %s\n",
-                   indent, "", port->id,
-                   port->guest_connected ? "on" : "off",
-                   port->host_connected ? "on" : "off",
-                   port->throttled ? "on" : "off");
-}
-
 /* This function is only used if a port id is not provided by the user */
 static uint32_t find_free_port_id(VirtIOSerial *vser)
 {
@@ -976,6 +956,17 @@ void virtio_serial_exit(VirtIODevice *vdev)
     virtio_cleanup(vdev);
 }
 
+static void virtio_serial_port_dev_print(DeviceState *qdev, Monitor *mon, int indent)
+{
+    VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, qdev);
+
+    monitor_printf(mon, "%*sport %d, guest %s, host %s, throttle %s\n",
+                   indent, "", port->id,
+                   port->guest_connected ? "on" : "off",
+                   port->host_connected ? "on" : "off",
+                   port->throttled ? "on" : "off");
+}
+
 static void virtio_serial_port_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
@@ -983,6 +974,7 @@ static void virtio_serial_port_class_init(ObjectClass *klass, void *data)
     k->bus_type = TYPE_VIRTIO_SERIAL_BUS;
     k->exit = virtser_port_qdev_exit;
     k->unplug = qdev_simple_unplug_cb;
+    k->print_dev = virtio_serial_port_dev_print;
 }
 
 static Property virtser_bus_properties[] = {
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 12/14] qbus: move print_dev to DeviceClass

[Andreas Färber]

Am 01.05.2012 20:18, schrieb Anthony Liguori:
> It should have never been a bus method.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Looks good,

Reviewed-by: Andreas Färber <afaerber@suse.de>

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 13/14] qbus: make child devices links

[Anthony Liguori]

Make qbus children show up as link<> properties.  There is no stable addressing
for qbus children so we use an unstable naming convention.

This is okay in QOM though because the composition name is expected to be what's
stable.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/acpi_piix4.c      |   10 +++++---
 hw/i2c.c             |    5 ++-
 hw/intel-hda.c       |   15 +++++++-----
 hw/lsi53c895a.c      |    5 ++-
 hw/qdev-monitor.c    |   26 +++++++++++++--------
 hw/qdev.c            |   62 +++++++++++++++++++++++++++++++++++++++++---------
 hw/qdev.h            |   11 +++++++-
 hw/s390-virtio-bus.c |   25 +++++++++----------
 hw/scsi-bus.c        |   13 ++++++----
 hw/spapr_pci.c       |    7 +++--
 hw/spapr_vio.c       |   27 +++++++++++----------
 hw/spapr_vty.c       |    6 +++-
 hw/ssi.c             |   14 ++++++-----
 hw/virtio-scsi.c     |    6 ++--
 qom/object.c         |   11 +++++++-
 15 files changed, 159 insertions(+), 84 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 585da4e..03daf9a 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -284,7 +284,7 @@ static const VMStateDescription vmstate_acpi = {
 
 static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots)
 {
-    DeviceState *qdev, *next;
+    BusChild *kid, *next;
     BusState *bus = qdev_get_parent_bus(&s->dev.qdev);
     int slot = ffs(slots) - 1;
     bool slot_free = true;
@@ -292,7 +292,8 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots)
     /* Mark request as complete */
     s->pci0_status.down &= ~(1U << slot);
 
-    QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
+    QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
+        DeviceState *qdev = kid->child;
         PCIDevice *dev = PCI_DEVICE(qdev);
         PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
         if (PCI_SLOT(dev->devfn) == slot) {
@@ -312,7 +313,7 @@ static void piix4_update_hotplug(PIIX4PMState *s)
 {
     PCIDevice *dev = &s->dev;
     BusState *bus = qdev_get_parent_bus(&dev->qdev);
-    DeviceState *qdev, *next;
+    BusChild *kid, *next;
 
     /* Execute any pending removes during reset */
     while (s->pci0_status.down) {
@@ -322,7 +323,8 @@ static void piix4_update_hotplug(PIIX4PMState *s)
     s->pci0_hotplug_enable = ~0;
     s->pci0_slot_device_present = 0;
 
-    QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
+    QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
+        DeviceState *qdev = kid->child;
         PCIDevice *pdev = PCI_DEVICE(qdev);
         PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pdev);
         int slot = PCI_SLOT(pdev->devfn);
diff --git a/hw/i2c.c b/hw/i2c.c
index 1cf1c17..50d5a84 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -80,11 +80,12 @@ int i2c_bus_busy(i2c_bus *bus)
 /* TODO: Make this handle multiple masters.  */
 int i2c_start_transfer(i2c_bus *bus, uint8_t address, int recv)
 {
-    DeviceState *qdev;
+    BusChild *kid;
     I2CSlave *slave = NULL;
     I2CSlaveClass *sc;
 
-    QTAILQ_FOREACH(qdev, &bus->qbus.children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
+        DeviceState *qdev = kid->child;
         I2CSlave *candidate = I2C_SLAVE_FROM_QDEV(qdev);
         if (candidate->address == address) {
             slave = candidate;
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 58497e5..00237c0 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -75,10 +75,11 @@ static int hda_codec_dev_exit(DeviceState *qdev)
 
 HDACodecDevice *hda_codec_find(HDACodecBus *bus, uint32_t cad)
 {
-    DeviceState *qdev;
+    BusChild *kid;
     HDACodecDevice *cdev;
 
-    QTAILQ_FOREACH(qdev, &bus->qbus.children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
+        DeviceState *qdev = kid->child;
         cdev = DO_UPCAST(HDACodecDevice, qdev, qdev);
         if (cdev->cad == cad) {
             return cdev;
@@ -480,10 +481,11 @@ static void intel_hda_parse_bdl(IntelHDAState *d, IntelHDAStream *st)
 
 static void intel_hda_notify_codecs(IntelHDAState *d, uint32_t stream, bool running, bool output)
 {
-    DeviceState *qdev;
+    BusChild *kid;
     HDACodecDevice *cdev;
 
-    QTAILQ_FOREACH(qdev, &d->codecs.qbus.children, sibling) {
+    QTAILQ_FOREACH(kid, &d->codecs.qbus.children, sibling) {
+        DeviceState *qdev = kid->child;
         HDACodecDeviceClass *cdc;
 
         cdev = DO_UPCAST(HDACodecDevice, qdev, qdev);
@@ -1102,15 +1104,16 @@ static const MemoryRegionOps intel_hda_mmio_ops = {
 
 static void intel_hda_reset(DeviceState *dev)
 {
+    BusChild *kid;
     IntelHDAState *d = DO_UPCAST(IntelHDAState, pci.qdev, dev);
-    DeviceState *qdev;
     HDACodecDevice *cdev;
 
     intel_hda_regs_reset(d);
     d->wall_base_ns = qemu_get_clock_ns(vm_clock);
 
     /* reset codecs */
-    QTAILQ_FOREACH(qdev, &d->codecs.qbus.children, sibling) {
+    QTAILQ_FOREACH(kid, &d->codecs.qbus.children, sibling) {
+        DeviceState *qdev = kid->child;
         cdev = DO_UPCAST(HDACodecDevice, qdev, qdev);
         device_reset(DEVICE(cdev));
         d->state_sts |= (1 << cdev->cad);
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index f022a02..2fe141d 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -1677,9 +1677,10 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
         }
         if (val & LSI_SCNTL1_RST) {
             if (!(s->sstat0 & LSI_SSTAT0_RST)) {
-                DeviceState *dev;
+                BusChild *kid;
 
-                QTAILQ_FOREACH(dev, &s->bus.qbus.children, sibling) {
+                QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
+                    DeviceState *dev = kid->child;
                     device_reset(dev);
                 }
                 s->sstat0 |= LSI_SSTAT0_RST;
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 42c088e..d0e4a86 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -222,11 +222,12 @@ static void qbus_list_bus(DeviceState *dev)
 
 static void qbus_list_dev(BusState *bus)
 {
-    DeviceState *dev;
+    BusChild *kid;
     const char *sep = " ";
 
     error_printf("devices at \"%s\":", bus->name);
-    QTAILQ_FOREACH(dev, &bus->children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->children, sibling) {
+        DeviceState *dev = kid->child;
         error_printf("%s\"%s\"", sep, object_get_typename(OBJECT(dev)));
         if (dev->id)
             error_printf("/\"%s\"", dev->id);
@@ -249,7 +250,7 @@ static BusState *qbus_find_bus(DeviceState *dev, char *elem)
 
 static DeviceState *qbus_find_dev(BusState *bus, char *elem)
 {
-    DeviceState *dev;
+    BusChild *kid;
 
     /*
      * try to match in order:
@@ -257,17 +258,20 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem)
      *   (2) driver name
      *   (3) driver alias, if present
      */
-    QTAILQ_FOREACH(dev, &bus->children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->children, sibling) {
+        DeviceState *dev = kid->child;
         if (dev->id  &&  strcmp(dev->id, elem) == 0) {
             return dev;
         }
     }
-    QTAILQ_FOREACH(dev, &bus->children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->children, sibling) {
+        DeviceState *dev = kid->child;
         if (strcmp(object_get_typename(OBJECT(dev)), elem) == 0) {
             return dev;
         }
     }
-    QTAILQ_FOREACH(dev, &bus->children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->children, sibling) {
+        DeviceState *dev = kid->child;
         DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
         if (qdev_class_has_alias(dc) &&
@@ -281,7 +285,7 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem)
 static BusState *qbus_find_recursive(BusState *bus, const char *name,
                                      const char *bus_typename)
 {
-    DeviceState *dev;
+    BusChild *kid;
     BusState *child, *ret;
     int match = 1;
 
@@ -296,7 +300,8 @@ static BusState *qbus_find_recursive(BusState *bus, const char *name,
         return bus;
     }
 
-    QTAILQ_FOREACH(dev, &bus->children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->children, sibling) {
+        DeviceState *dev = kid->child;
         QLIST_FOREACH(child, &dev->child_bus, sibling) {
             ret = qbus_find_recursive(child, name, bus_typename);
             if (ret) {
@@ -553,12 +558,13 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
 
 static void qbus_print(Monitor *mon, BusState *bus, int indent)
 {
-    struct DeviceState *dev;
+    BusChild *kid;
 
     qdev_printf("bus: %s\n", bus->name);
     indent += 2;
     qdev_printf("type %s\n", object_get_typename(OBJECT(bus)));
-    QTAILQ_FOREACH(dev, &bus->children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->children, sibling) {
+        DeviceState *dev = kid->child;
         qdev_print(mon, dev, indent);
     }
 }
diff --git a/hw/qdev.c b/hw/qdev.c
index bee0cd6..166c599 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -75,14 +75,48 @@ void qdev_add_properties(DeviceState *dev, Property *props)
     qdev_prop_set_defaults(dev, props);
 }
 
-void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
+static void bus_remove_child(BusState *bus, DeviceState *child)
+{
+    BusChild *kid;
+
+    QTAILQ_FOREACH(kid, &bus->children, sibling) {
+        if (kid->child == child) {
+            char name[32];
+
+            snprintf(name, sizeof(name), "child[%d]", kid->index);
+            QTAILQ_REMOVE(&bus->children, kid, sibling);
+            object_property_del(OBJECT(bus), name, NULL);
+            g_free(kid);
+            return;
+        }
+    }
+}
+
+static void bus_add_child(BusState *bus, DeviceState *child)
 {
+    char name[32];
+    BusChild *kid = g_malloc0(sizeof(*kid));
+
     if (qdev_hotplug) {
         assert(bus->allow_hotplug);
     }
 
+    kid->index = bus->max_index++;
+    kid->child = child;
+
+    QTAILQ_INSERT_HEAD(&bus->children, kid, sibling);
+
+    snprintf(name, sizeof(name), "child[%d]", kid->index);
+    object_property_add_link(OBJECT(bus), name,
+                             object_get_typename(OBJECT(child)),
+                             (Object **)&kid->child,
+                             NULL);
+}
+
+void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
+{
     dev->parent_bus = bus;
-    QTAILQ_INSERT_HEAD(&bus->children, dev, sibling);
+    bus_add_child(bus, dev);
 }
 
 /* Create a new device.  This only initializes the device state structure
@@ -334,7 +368,7 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
                        qbus_walkerfn *busfn, void *opaque)
 {
-    DeviceState *dev;
+    BusChild *kid;
     int err;
 
     if (busfn) {
@@ -344,8 +378,8 @@ int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
         }
     }
 
-    QTAILQ_FOREACH(dev, &bus->children, sibling) {
-        err = qdev_walk_children(dev, devfn, busfn, opaque);
+    QTAILQ_FOREACH(kid, &bus->children, sibling) {
+        err = qdev_walk_children(kid->child, devfn, busfn, opaque);
         if (err < 0) {
             return err;
         }
@@ -379,12 +413,17 @@ int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
 
 DeviceState *qdev_find_recursive(BusState *bus, const char *id)
 {
-    DeviceState *dev, *ret;
+    BusChild *kid;
+    DeviceState *ret;
     BusState *child;
 
-    QTAILQ_FOREACH(dev, &bus->children, sibling) {
-        if (dev->id && strcmp(dev->id, id) == 0)
+    QTAILQ_FOREACH(kid, &bus->children, sibling) {
+        DeviceState *dev = kid->child;
+
+        if (dev->id && strcmp(dev->id, id) == 0) {
             return dev;
+        }
+
         QLIST_FOREACH(child, &dev->child_bus, sibling) {
             ret = qdev_find_recursive(child, id);
             if (ret) {
@@ -465,9 +504,10 @@ static void main_system_bus_create(void)
 
 void qbus_free(BusState *bus)
 {
-    DeviceState *dev;
+    BusChild *kid;
 
-    while ((dev = QTAILQ_FIRST(&bus->children)) != NULL) {
+    while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
+        DeviceState *dev = kid->child;
         qdev_free(dev);
     }
     if (bus->parent) {
@@ -679,7 +719,7 @@ static void device_finalize(Object *obj)
         }
     }
     if (dev->parent_bus) {
-        QTAILQ_REMOVE(&dev->parent_bus->children, dev, sibling);
+        bus_remove_child(dev->parent_bus, dev);
     }
 }
 
diff --git a/hw/qdev.h b/hw/qdev.h
index 2141bba..7b319d6 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -77,7 +77,6 @@ struct DeviceState {
     qemu_irq *gpio_in;
     QLIST_HEAD(, BusState) child_bus;
     int num_child_bus;
-    QTAILQ_ENTRY(DeviceState) sibling;
     int instance_id_alias;
     int alias_required_for_version;
 };
@@ -99,13 +98,21 @@ struct BusClass {
     int (*reset)(BusState *bus);
 };
 
+typedef struct BusChild
+{
+    DeviceState *child;
+    int index;
+    QTAILQ_ENTRY(BusChild) sibling;
+} BusChild;
+
 struct BusState {
     Object obj;
     DeviceState *parent;
     const char *name;
     int allow_hotplug;
     int qdev_allocated;
-    QTAILQ_HEAD(ChildrenHead, DeviceState) children;
+    int max_index;
+    QTAILQ_HEAD(ChildrenHead, BusChild) children;
     QLIST_ENTRY(BusState) sibling;
 };
 
diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index 60871a2..77d2e69 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -293,20 +293,20 @@ VirtIOS390Device *s390_virtio_bus_find_vring(VirtIOS390Bus *bus,
                                              ram_addr_t mem,
                                              int *vq_num)
 {
-    VirtIOS390Device *_dev;
-    DeviceState *dev;
+    BusChild *kid;
     int i;
 
-    QTAILQ_FOREACH(dev, &bus->bus.children, sibling) {
-        _dev = (VirtIOS390Device *)dev;
+    QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
+        VirtIOS390Device *dev = (VirtIOS390Device *)kid->child;
+
         for(i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
-            if (!virtio_queue_get_addr(_dev->vdev, i))
+            if (!virtio_queue_get_addr(dev->vdev, i))
                 break;
-            if (virtio_queue_get_addr(_dev->vdev, i) == mem) {
+            if (virtio_queue_get_addr(dev->vdev, i) == mem) {
                 if (vq_num) {
                     *vq_num = i;
                 }
-                return _dev;
+                return dev;
             }
         }
     }
@@ -317,13 +317,12 @@ VirtIOS390Device *s390_virtio_bus_find_vring(VirtIOS390Bus *bus,
 /* Find a device by device descriptor location */
 VirtIOS390Device *s390_virtio_bus_find_mem(VirtIOS390Bus *bus, ram_addr_t mem)
 {
-    VirtIOS390Device *_dev;
-    DeviceState *dev;
+    BusChild *kid;
 
-    QTAILQ_FOREACH(dev, &bus->bus.children, sibling) {
-        _dev = (VirtIOS390Device *)dev;
-        if (_dev->dev_offs == mem) {
-            return _dev;
+    QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
+        VirtIOS390Device *dev = (VirtIOS390Device *)kid->child;
+        if (dev->dev_offs == mem) {
+            return dev;
         }
     }
 
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 98bb217..c8a0bdd 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -285,7 +285,7 @@ static void store_lun(uint8_t *outbuf, int lun)
 
 static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
 {
-    DeviceState *qdev;
+    BusChild *kid;
     int i, len, n;
     int channel, id;
     bool found_lun0;
@@ -300,7 +300,8 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
     id = r->req.dev->id;
     found_lun0 = false;
     n = 0;
-    QTAILQ_FOREACH(qdev, &r->req.bus->qbus.children, sibling) {
+    QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) {
+        DeviceState *qdev = kid->child;
         SCSIDevice *dev = SCSI_DEVICE(qdev);
 
         if (dev->channel == channel && dev->id == id) {
@@ -322,7 +323,8 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
     memset(r->buf, 0, len);
     stl_be_p(&r->buf, n);
     i = found_lun0 ? 8 : 16;
-    QTAILQ_FOREACH(qdev, &r->req.bus->qbus.children, sibling) {
+    QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) {
+        DeviceState *qdev = kid->child;
         SCSIDevice *dev = SCSI_DEVICE(qdev);
 
         if (dev->channel == channel && dev->id == id) {
@@ -1438,10 +1440,11 @@ static char *scsi_qdev_get_dev_path(DeviceState *dev)
 
 SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
 {
-    DeviceState *qdev;
+    BusChild *kid;
     SCSIDevice *target_dev = NULL;
 
-    QTAILQ_FOREACH_REVERSE(qdev, &bus->qbus.children, ChildrenHead, sibling) {
+    QTAILQ_FOREACH_REVERSE(kid, &bus->qbus.children, ChildrenHead, sibling) {
+        DeviceState *qdev = kid->child;
         SCSIDevice *dev = SCSI_DEVICE(qdev);
 
         if (dev->channel == channel && dev->id == id) {
diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
index a564c00..fcc6d58 100644
--- a/hw/spapr_pci.c
+++ b/hw/spapr_pci.c
@@ -35,17 +35,18 @@
 static PCIDevice *find_dev(sPAPREnvironment *spapr,
                            uint64_t buid, uint32_t config_addr)
 {
-    DeviceState *qdev;
     int devfn = (config_addr >> 8) & 0xFF;
     sPAPRPHBState *phb;
 
     QLIST_FOREACH(phb, &spapr->phbs, list) {
+        BusChild *kid;
+
         if (phb->buid != buid) {
             continue;
         }
 
-        QTAILQ_FOREACH(qdev, &phb->host_state.bus->qbus.children, sibling) {
-            PCIDevice *dev = (PCIDevice *)qdev;
+        QTAILQ_FOREACH(kid, &phb->host_state.bus->qbus.children, sibling) {
+            PCIDevice *dev = (PCIDevice *)kid->child;
             if (dev->devfn == devfn) {
                 return dev;
             }
diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index c7c3935..bdfe1df 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -59,11 +59,11 @@ static TypeInfo spapr_vio_bus_info = {
 
 VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg)
 {
-    DeviceState *qdev;
+    BusChild *kid;
     VIOsPAPRDevice *dev = NULL;
 
-    QTAILQ_FOREACH(qdev, &bus->bus.children, sibling) {
-        dev = (VIOsPAPRDevice *)qdev;
+    QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
+        dev = (VIOsPAPRDevice *)kid->child;
         if (dev->reg == reg) {
             return dev;
         }
@@ -603,7 +603,7 @@ static void rtas_quiesce(sPAPREnvironment *spapr, uint32_t token,
                          uint32_t nret, target_ulong rets)
 {
     VIOsPAPRBus *bus = spapr->vio_bus;
-    DeviceState *qdev;
+    BusChild *kid;
     VIOsPAPRDevice *dev = NULL;
 
     if (nargs != 0) {
@@ -611,8 +611,8 @@ static void rtas_quiesce(sPAPREnvironment *spapr, uint32_t token,
         return;
     }
 
-    QTAILQ_FOREACH(qdev, &bus->bus.children, sibling) {
-        dev = (VIOsPAPRDevice *)qdev;
+    QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
+        dev = (VIOsPAPRDevice *)kid->child;
         spapr_vio_quiesce_one(dev);
     }
 
@@ -622,7 +622,7 @@ static void rtas_quiesce(sPAPREnvironment *spapr, uint32_t token,
 static int spapr_vio_check_reg(VIOsPAPRDevice *sdev)
 {
     VIOsPAPRDevice *other_sdev;
-    DeviceState *qdev;
+    BusChild *kid;
     VIOsPAPRBus *sbus;
 
     sbus = DO_UPCAST(VIOsPAPRBus, bus, sdev->qdev.parent_bus);
@@ -632,13 +632,13 @@ static int spapr_vio_check_reg(VIOsPAPRDevice *sdev)
      * other mechanism). We have to open code this because we have to check
      * for matches with devices other than us.
      */
-    QTAILQ_FOREACH(qdev, &sbus->bus.children, sibling) {
-        other_sdev = DO_UPCAST(VIOsPAPRDevice, qdev, qdev);
+    QTAILQ_FOREACH(kid, &sbus->bus.children, sibling) {
+        other_sdev = DO_UPCAST(VIOsPAPRDevice, qdev, kid->child);
 
         if (other_sdev != sdev && other_sdev->reg == sdev->reg) {
             fprintf(stderr, "vio: %s and %s devices conflict at address %#x\n",
                     object_get_typename(OBJECT(sdev)),
-                    object_get_typename(OBJECT(qdev)),
+                    object_get_typename(OBJECT(other_sdev)),
                     sdev->reg);
             return -EEXIST;
         }
@@ -833,19 +833,20 @@ static int compare_reg(const void *p1, const void *p2)
 int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt)
 {
     DeviceState *qdev, **qdevs;
+    BusChild *kid;
     int i, num, ret = 0;
 
     /* Count qdevs on the bus list */
     num = 0;
-    QTAILQ_FOREACH(qdev, &bus->bus.children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
         num++;
     }
 
     /* Copy out into an array of pointers */
     qdevs = g_malloc(sizeof(qdev) * num);
     num = 0;
-    QTAILQ_FOREACH(qdev, &bus->bus.children, sibling) {
-        qdevs[num++] = qdev;
+    QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
+        qdevs[num++] = kid->child;
     }
 
     /* Sort the array */
diff --git a/hw/spapr_vty.c b/hw/spapr_vty.c
index a30c040..cfdc500 100644
--- a/hw/spapr_vty.c
+++ b/hw/spapr_vty.c
@@ -161,7 +161,7 @@ static TypeInfo spapr_vty_info = {
 VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus)
 {
     VIOsPAPRDevice *sdev, *selected;
-    DeviceState *iter;
+    BusChild *kid;
 
     /*
      * To avoid the console bouncing around we want one VTY to be
@@ -170,7 +170,9 @@ VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus)
      */
 
     selected = NULL;
-    QTAILQ_FOREACH(iter, &bus->bus.children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
+        DeviceState *iter = kid->child;
+
         /* Only look at VTY devices */
         if (!object_dynamic_cast(OBJECT(iter), "spapr-vty")) {
             continue;
diff --git a/hw/ssi.c b/hw/ssi.c
index 543adff..ef03bc9 100644
--- a/hw/ssi.c
+++ b/hw/ssi.c
@@ -29,10 +29,11 @@ static int ssi_slave_init(DeviceState *dev)
     SSISlave *s = SSI_SLAVE(dev);
     SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s);
     SSIBus *bus;
+    BusChild *kid;
 
     bus = FROM_QBUS(SSIBus, qdev_get_parent_bus(dev));
-    if (QTAILQ_FIRST(&bus->qbus.children) != dev
-        || QTAILQ_NEXT(dev, sibling) != NULL) {
+    kid = QTAILQ_FIRST(&bus->qbus.children);
+    if (kid->child != dev || QTAILQ_NEXT(kid, sibling) != NULL) {
         hw_error("Too many devices on SSI bus");
     }
 
@@ -71,14 +72,15 @@ SSIBus *ssi_create_bus(DeviceState *parent, const char *name)
 
 uint32_t ssi_transfer(SSIBus *bus, uint32_t val)
 {
-    DeviceState *dev;
+    BusChild *kid;
     SSISlave *slave;
     SSISlaveClass *ssc;
-    dev = QTAILQ_FIRST(&bus->qbus.children);
-    if (!dev) {
+
+    kid = QTAILQ_FIRST(&bus->qbus.children);
+    if (!kid) {
         return 0;
     }
-    slave = SSI_SLAVE(dev);
+    slave = SSI_SLAVE(kid->child);
     ssc = SSI_SLAVE_GET_CLASS(slave);
     return ssc->transfer(slave, val);
 }
diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index e8328f4..970b691 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -275,7 +275,7 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
 {
     SCSIDevice *d = virtio_scsi_device_find(s, req->req.tmf->lun);
     SCSIRequest *r, *next;
-    DeviceState *qdev;
+    BusChild *kid;
     int target;
 
     /* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE".  */
@@ -346,8 +346,8 @@ static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
     case VIRTIO_SCSI_T_TMF_I_T_NEXUS_RESET:
         target = req->req.tmf->lun[1];
         s->resetting++;
-        QTAILQ_FOREACH(qdev, &s->bus.qbus.children, sibling) {
-             d = DO_UPCAST(SCSIDevice, qdev, qdev);
+        QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) {
+             d = DO_UPCAST(SCSIDevice, qdev, kid->child);
              if (d->channel == 0 && d->id == target) {
                 qdev_reset_all(&d->qdev);
              }
diff --git a/qom/object.c b/qom/object.c
index 0268f2a..d894036 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -665,9 +665,16 @@ void object_property_del(Object *obj, const char *name, Error **errp)
 {
     ObjectProperty *prop = object_property_find(obj, name);
 
-    QTAILQ_REMOVE(&obj->properties, prop, node);
+    if (prop == NULL) {
+        error_set(errp, QERR_PROPERTY_NOT_FOUND, "", name);
+        return;
+    }
+
+    if (prop->release) {
+        prop->release(obj, name, prop->opaque);
+    }
 
-    prop->release(obj, prop->name, prop->opaque);
+    QTAILQ_REMOVE(&obj->properties, prop, node);
 
     g_free(prop->name);
     g_free(prop->type);
-- 
1.7.5.4

[Qemu-devel] [PATCH 14/14] qbus: initialize in standard way

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/qdev.c |   84 +++++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 51 insertions(+), 33 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 166c599..f637351 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -434,40 +434,35 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id)
     return NULL;
 }
 
-/* FIXME move this logic into instance_init */
-static void do_qbus_create_inplace(BusState *bus, const char *typename,
-                                   DeviceState *parent, const char *name)
+static void qbus_realize(BusState *bus)
 {
+    const char *typename = object_get_typename(OBJECT(bus));
     char *buf;
     int i,len;
 
-    bus->parent = parent;
-
-    if (name) {
+    if (bus->name) {
         /* use supplied name */
-        bus->name = g_strdup(name);
-    } else if (parent && parent->id) {
+    } else if (bus->parent && bus->parent->id) {
         /* parent device has id -> use it for bus name */
-        len = strlen(parent->id) + 16;
+        len = strlen(bus->parent->id) + 16;
         buf = g_malloc(len);
-        snprintf(buf, len, "%s.%d", parent->id, parent->num_child_bus);
+        snprintf(buf, len, "%s.%d", bus->parent->id, bus->parent->num_child_bus);
         bus->name = buf;
     } else {
         /* no id -> use lowercase bus type for bus name */
         len = strlen(typename) + 16;
         buf = g_malloc(len);
         len = snprintf(buf, len, "%s.%d", typename,
-                       parent ? parent->num_child_bus : 0);
+                       bus->parent ? bus->parent->num_child_bus : 0);
         for (i = 0; i < len; i++)
             buf[i] = qemu_tolower(buf[i]);
         bus->name = buf;
     }
 
-    QTAILQ_INIT(&bus->children);
-    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);
+    if (bus->parent) {
+        QLIST_INSERT_HEAD(&bus->parent->child_bus, bus, sibling);
+        bus->parent->num_child_bus++;
+        object_property_add_child(OBJECT(bus->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. */
@@ -479,7 +474,10 @@ void qbus_create_inplace(BusState *bus, const char *typename,
                          DeviceState *parent, const char *name)
 {
     object_initialize(bus, typename);
-    do_qbus_create_inplace(bus, typename, parent, name);
+
+    bus->parent = parent;
+    bus->name = name ? g_strdup(name) : NULL;
+    qbus_realize(bus);
 }
 
 BusState *qbus_create(const char *typename, DeviceState *parent, const char *name)
@@ -488,7 +486,11 @@ BusState *qbus_create(const char *typename, DeviceState *parent, const char *nam
 
     bus = BUS(object_new(typename));
     bus->qdev_allocated = 1;
-    do_qbus_create_inplace(bus, typename, parent, name);
+
+    bus->parent = parent;
+    bus->name = name ? g_strdup(name) : NULL;
+    qbus_realize(bus);
+
     return bus;
 }
 
@@ -504,22 +506,10 @@ static void main_system_bus_create(void)
 
 void qbus_free(BusState *bus)
 {
-    BusChild *kid;
-
-    while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
-        DeviceState *dev = kid->child;
-        qdev_free(dev);
-    }
-    if (bus->parent) {
-        QLIST_REMOVE(bus, sibling);
-        bus->parent->num_child_bus--;
-    } else {
-        assert(bus != main_system_bus); /* main_system_bus is never freed */
-        qemu_unregister_reset(qbus_reset_all_fn, bus);
-    }
-    g_free((void*)bus->name);
     if (bus->qdev_allocated) {
-        g_free(bus);
+        object_delete(OBJECT(bus));
+    } else {
+        object_finalize(OBJECT(bus));
     }
 }
 
@@ -753,12 +743,40 @@ static TypeInfo device_type_info = {
     .class_size = sizeof(DeviceClass),
 };
 
+static void qbus_initfn(Object *obj)
+{
+    BusState *bus = BUS(obj);
+
+    QTAILQ_INIT(&bus->children);
+}
+
+static void qbus_finalize(Object *obj)
+{
+    BusState *bus = BUS(obj);
+    BusChild *kid;
+
+    while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
+        DeviceState *dev = kid->child;
+        qdev_free(dev);
+    }
+    if (bus->parent) {
+        QLIST_REMOVE(bus, sibling);
+        bus->parent->num_child_bus--;
+    } else {
+        assert(bus != main_system_bus); /* main_system_bus is never freed */
+        qemu_unregister_reset(qbus_reset_all_fn, bus);
+    }
+    g_free((void*)bus->name);
+}
+
 static TypeInfo bus_info = {
     .name = TYPE_BUS,
     .parent = TYPE_OBJECT,
     .instance_size = sizeof(BusState),
     .abstract = true,
     .class_size = sizeof(BusClass),
+    .instance_init = qbus_initfn,
+    .instance_finalize = qbus_finalize,
 };
 
 static void qdev_register_types(void)
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 14/14] qbus: initialize in standard way

[Paolo Bonzini]

Il 01/05/2012 20:18, Anthony Liguori ha scritto:
>      if (bus->qdev_allocated) {
> -        g_free(bus);
> +        object_delete(OBJECT(bus));
> +    } else {
> +        object_finalize(OBJECT(bus));
>      }

Time is ripe to add a more versatile freeing mechanism along the lines
you've set in the past (either a callback or a notifier that object_new
would set to a g_free wrapper).  Anthony, are you going to do this?

Paolo