[RFC,10/11] qdev: Add do_info_qbus and friends.

Places information about a bus and the devices on into a QObject.

Signed-off-by: Nathan Baum <nathan@parenthephobia.org.uk>
---
 hw/qdev.c |   73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 73 insertions(+), 0 deletions(-)

Nathan Baum <nathan@parenthephobia.org.uk> writes:

> Places information about a bus and the devices on into a QObject.
>
> Signed-off-by: Nathan Baum <nathan@parenthephobia.org.uk>
> ---
>  hw/qdev.c |   73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 73 insertions(+), 0 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index b6bd4ae..f5d68c6 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -30,6 +30,8 @@
>  #include "sysemu.h"
>  #include "monitor.h"
>  #include "qerror.h"
> +#include "qint.h"
> +#include "qbool.h"
>  
>  static int qdev_hotplug = 0;
>  
> @@ -654,6 +656,77 @@ void qbus_free(BusState *bus)
>      }
>  }
>  
> +static void do_info_qbus(Monitor *mon, BusState *bus, QObject **ret_data);
> +
> +static void do_info_qdev_props(Monitor *mon, DeviceState *dev, Property *props, const char *prefix,

Long line, please break; there's plenty of space left in the next one :)

> +                               QDict *qdict)
> +{
> +    char name[64];
> +    char value[64];
> +
> +    if (!props)
> +        return;
> +    while (props->name) {
> +        if (props->info->print) {
> +            props->info->print(dev, props, value, sizeof(value));
> +            snprintf(name, sizeof(name), "%s-%s", prefix, props->name);

Funny naming convention PREFIX-PROPNAME, because do_info_qdev_props()
below puts properties directly in the qdict.

Maybe it would be cleaner to have them in their own dictionary "props".

However, because there are both device properties and bus properties
(really: device properties common to all devices on this bus), their
names can clash.  Device properties take precedence (see
qdev_prop_find()).  Hmm, qdev_printf() prints even overridden bus
properties, not sure that's appropriate.  Gerd?

> +            qdict_put(qdict, name, qstring_from_str(value));
> +        }
> +        props++;
> +    }
> +}
> +
> +static void do_info_qdev(Monitor *mon, DeviceState *dev, QObject **ret_data)
> +{
> +    BusState *child;
> +    QDict *qdict;
> +    QList *children;
> +
> +    qdict = qdict_new();
> +    qdict_put(qdict, "name", qstring_from_str(dev->info->name));
> +    if (dev->id)
> +        qdict_put(qdict, "id", qstring_from_str(dev->id));
> +    qdict_put(qdict, "gpio-in", qint_from_int(dev->num_gpio_in));
> +    qdict_put(qdict, "gpio-out", qint_from_int(dev->num_gpio_out));
> +    do_info_qdev_props(mon, dev, dev->info->props, "dev", qdict);
> +    do_info_qdev_props(mon, dev, dev->parent_bus->info->props, "bus", qdict);
> +    children = qlist_new();
> +    QLIST_FOREACH(child, &dev->child_bus, sibling) {
> +        QObject *data = NULL;
> +        do_info_qbus(mon, child, &data);
> +        if (data)
> +            qlist_append_obj(children, data);
> +    }
> +    if (!qlist_empty(children))
> +        qdict_put(qdict, "children", children);
> +    if (dev->parent_bus->info->info_dev) {
> +        qdict_put_obj(qdict, "info", dev->parent_bus->info->info_dev(mon, dev));
> +    }
> +    *ret_data = (QObject *) qdict;

Use QOBJECT().

> +}
> +
> +static void do_info_qbus(Monitor *mon, BusState *bus, QObject **ret_data)
> +{
> +    struct DeviceState *dev;
> +    QDict *qdict;
> +    QList *children;
> +
> +    qdict = qdict_new();
> +    qdict_put(qdict, "bus", qstring_from_str(bus->name));
> +    qdict_put(qdict, "type", qstring_from_str(bus->info->name));
> +    qdict_put(qdict, "allow_hotplug", qbool_from_int(bus->allow_hotplug));
> +    children = qlist_new();
> +    qdict_put(qdict, "children", children);
> +    QLIST_FOREACH(dev, &bus->children, sibling) {
> +        QObject *data = NULL;
> +        do_info_qdev(mon, dev, &data);
> +        if (data)
> +            qlist_append_obj(children, data);
> +    }
> +    
> +    *ret_data = (QObject *) qdict;

Use QOBJECT().

> +}
> +
>  #define qdev_printf(fmt, ...) monitor_printf(mon, "%*s" fmt, indent, "", ## __VA_ARGS__)
>  static void qbus_print(Monitor *mon, BusState *bus, int indent);

> However, because there are both device properties and bus properties
> (really: device properties common to all devices on this bus), their
> names can clash.  Device properties take precedence (see
> qdev_prop_find()).  Hmm, qdev_printf() prints even overridden bus
> properties, not sure that's appropriate.  Gerd?

IMHO they must not clash.  This isn't enforced in any way though.

cheers,
   Gerd

Gerd Hoffmann <kraxel@redhat.com> writes:

>> However, because there are both device properties and bus properties
>> (really: device properties common to all devices on this bus), their
>> names can clash.  Device properties take precedence (see
>> qdev_prop_find()).  Hmm, qdev_printf() prints even overridden bus
>> properties, not sure that's appropriate.  Gerd?
>
> IMHO they must not clash.  This isn't enforced in any way though.

If they must not clash, then it makes no sense to invent a fancy prefix
to cope with clashes, I think.

Alternatively, we could declare devices overriding properties inherited
from the bus a feature.  Does it make any sense to show the shadowed
properties then?

On 01/18/10 13:34, Markus Armbruster wrote:
>>> However, because there are both device properties and bus properties
>>> (really: device properties common to all devices on this bus), their
>>> names can clash.  Device properties take precedence (see
>>> qdev_prop_find()).  Hmm, qdev_printf() prints even overridden bus
>>> properties, not sure that's appropriate.  Gerd?
>>
>> IMHO they must not clash.  This isn't enforced in any way though.
>
> If they must not clash, then it makes no sense to invent a fancy prefix
> to cope with clashes, I think.

I've added the bus- and dev- prefixes to make clear where the properties 
come from (BusInfo or DeviceInfo) for informational purposes, not to 
avoid clashes.  We could just drop that ...

> Alternatively, we could declare devices overriding properties inherited
> from the bus a feature.

No, this is just asking for trouble IMHO.

cheers,
   Gerd