Patchwork [06/14] qdev: add ability to do QOM-style derived naming

login
register
mail settings
Submitter Anthony Liguori
Date Sept. 16, 2011, 4 p.m.
Message ID <1316188834-13675-7-git-send-email-aliguori@us.ibm.com>
Download mbox | patch
Permalink /patch/114974/
State New
Headers show

Comments

Anthony Liguori - Sept. 16, 2011, 4 p.m.
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.

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 <aliguori@us.ibm.com>
---
 hw/qdev.c |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 hw/qdev.h |    8 ++++-
 2 files changed, 78 insertions(+), 9 deletions(-)
Blue Swirl - Sept. 17, 2011, 6:39 p.m.
On Fri, Sep 16, 2011 at 4:00 PM, Anthony Liguori <aliguori@us.ibm.com> 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 <aliguori@us.ibm.com>
> ---
>  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
>
>
>

Patch

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;