Re: [Qemu-devel] [PATCH 06/14] qdev: add ability to do QOM-style derived naming

[Blue Swirl]

On Fri, Sep 16, 2011 at 4:00 PM, Anthony Liguori <address@hidden> wrote:
> By using a prefix of "::" in the name, we can safely derive the composed 
> device
> name from the parent device and busses name.  For instance, if the "::i440fx"
> device created a device named "piix3", it would look like this:
>
>  static void i440fx_initfn(...)
>  {
>    s->piix3 = qdev_create("PIIX3", "::piix3");
>    ...
>
> The resulting device would be named "::i440fx::i440fx.0::piix3".  The reason 
> for
> the middle "::i440fx.0" blob is that there are two levels of the tree 
> hierarchy
> here and the bus level already has it's name derived from the parent device.

It could make sense to name the intermediate level by bus type, like
::i440fx::pci.0::piix3.

> We'll eliminate the bus level of the hierarchy in due time, but for now we 
> have
> to just live with the ugly names.
>
> This patch lets qdev names be specified as a printf style format string which 
> is
> convenient for creating devices like "::smbus-eeprom[%d]".
>
> Signed-off-by: Anthony Liguori <address@hidden>
> ---
>  hw/qdev.c |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  hw/qdev.h |    8 ++++-
>  2 files changed, 78 insertions(+), 9 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 3096667..6bf6650 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -88,9 +88,10 @@ static DeviceInfo *qdev_find_info(BusInfo *bus_info, const 
> char *name)
>     return NULL;
>  }
>
> -static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info, 
> const char *id)
> +static DeviceState *qdev_create_from_infov(BusState *bus, DeviceInfo *info, 
> const char *id, va_list ap)
>  {
>     DeviceState *dev;
> +    char *name = NULL;
>
>     assert(bus->info == info->bus_info);
>     dev = g_malloc0(info->size);
> @@ -107,18 +108,50 @@ static DeviceState *qdev_create_from_info(BusState 
> *bus, DeviceInfo *info, const
>     }
>     dev->instance_id_alias = -1;
>     dev->state = DEV_STATE_CREATED;
> -    dev->id = g_strdup(id);
> +
> +    if (id) {
> +        name = g_strdup_vprintf(id, ap);
> +        if (name[0] == ':' && name[1] == ':') {
> +            const char *parent_bus, *parent_device;
> +            char *full_name;
> +
> +            if (dev->parent_bus && dev->parent_bus->parent) {
> +                parent_device = dev->parent_bus->parent->id;
> +                parent_bus = dev->parent_bus->name;
> +
> +                full_name = g_strdup_printf("%s%s%s",
> +                                            dev->parent_bus->parent->id,
> +                                            dev->parent_bus->name,
> +                                            name);
> +                g_free(name);
> +                name = full_name;
> +            }
> +        }
> +    }
> +    dev->id = name;
> +    return dev;
> +}
> +
> +static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info, 
> const char *id, ...)
> +{
> +    DeviceState *dev;
> +    va_list ap;
> +
> +    va_start(ap, id);
> +    dev = qdev_create_from_infov(bus, info, id, ap);
> +    va_end(ap);
> +
>     return dev;
>  }
>
>  /* Create a new device.  This only initializes the device state structure
>    and allows properties to be set.  qdev_init should be called to
>    initialize the actual device emulation.  */
> -DeviceState *qdev_create(BusState *bus, const char *name, const char *id)
> +DeviceState *qdev_createv(BusState *bus, const char *name, const char *id, 
> va_list ap)
>  {
>     DeviceState *dev;
>
> -    dev = qdev_try_create(bus, name, id);
> +    dev = qdev_try_createv(bus, name, id, ap);
>     if (!dev) {
>         if (bus) {
>             hw_error("Unknown device '%s' for bus '%s'\n", name,
> @@ -131,7 +164,19 @@ DeviceState *qdev_create(BusState *bus, const char 
> *name, const char *id)
>     return dev;
>  }
>
> -DeviceState *qdev_try_create(BusState *bus, const char *name, const char *id)
> +DeviceState *qdev_create(BusState *bus, const char *name, const char *id, 
> ...)
> +{
> +    DeviceState *dev;
> +    va_list ap;
> +
> +    va_start(ap, id);
> +    dev = qdev_createv(bus, name, id, ap);
> +    va_end(ap);
> +
> +    return dev;
> +}
> +
> +DeviceState *qdev_try_createv(BusState *bus, const char *name, const char 
> *id, va_list ap)
>  {
>     DeviceInfo *info;
>
> @@ -144,7 +189,19 @@ DeviceState *qdev_try_create(BusState *bus, const char 
> *name, const char *id)
>         return NULL;
>     }
>
> -    return qdev_create_from_info(bus, info, id);
> +    return qdev_create_from_infov(bus, info, id, ap);
> +}
> +
> +DeviceState *qdev_try_create(BusState *bus, const char *name, const char 
> *id, ...)
> +{
> +    DeviceState *dev;
> +    va_list ap;
> +
> +    va_start(ap, id);
> +    dev = qdev_try_createv(bus, name, id, ap);
> +    va_end(ap);
> +
> +    return dev;
>  }
>
>  static void qdev_print_devinfo(DeviceInfo *info)
> @@ -231,6 +288,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>     DeviceInfo *info;
>     DeviceState *qdev;
>     BusState *bus;
> +    const char *id;
>
>     driver = qemu_opt_get(opts, "driver");
>     if (!driver) {
> @@ -271,8 +329,15 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>         return NULL;
>     }
>
> +    id = qemu_opts_id(opts);
> +
>     /* create device, set properties */
> -    qdev = qdev_create_from_info(bus, info, qemu_opts_id(opts));
> +    if (id) {
> +        qdev = qdev_create_from_info(bus, info, "%s", id);
> +    } else {
> +        qdev = qdev_create_from_info(bus, info, NULL);
> +    }
> +
>     if (qemu_opt_foreach(opts, set_property, qdev, 1) != 0) {
>         qdev_free(qdev);
>         return NULL;
> diff --git a/hw/qdev.h b/hw/qdev.h
> index d15a47e..a2b7a08 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -120,8 +120,12 @@ typedef struct GlobalProperty {
>
>  /*** Board API.  This should go away once we have a machine config file.  
> ***/
>
> -DeviceState *qdev_create(BusState *bus, const char *name, const char *id);
> -DeviceState *qdev_try_create(BusState *bus, const char *name, const char 
> *id);
> +DeviceState *qdev_create(BusState *bus, const char *name, const char *id, 
> ...)
> +    GCC_FMT_ATTR(3, 4);
> +DeviceState *qdev_createv(BusState *bus, const char *name, const char *id, 
> va_list ap);
> +DeviceState *qdev_try_create(BusState *bus, const char *name, const char 
> *id, ...)
> +    GCC_FMT_ATTR(3, 4);
> +DeviceState *qdev_try_createv(BusState *bus, const char *name, const char 
> *id, va_list ap);
>  int qdev_device_help(QemuOpts *opts);
>  DeviceState *qdev_device_add(QemuOpts *opts);
>  int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
> --
> 1.7.4.1
>
>
>