Patchwork [v4,04/23] qdev: Allow device addressing via 'driver.instance'

login
register
mail settings
Submitter Jan Kiszka
Date June 15, 2010, 10:38 p.m.
Message ID <f3ad1adfd8cd9070df036f970826652734a7e5d6.1276641524.git.jan.kiszka@web.de>
Download mbox | patch
Permalink /patch/55806/
State New
Headers show

Comments

Jan Kiszka - June 15, 2010, 10:38 p.m.
From: Jan Kiszka <jan.kiszka@siemens.com>

Extend qbus_find_dev to allow addressing of devices without an unique id
via an optional per-bus instance number. The new formats are
'driver.instance' and 'alias.instance'. Attach this name extension
whenever an instantiated device is printed.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 docs/qdev-device-use.txt |   13 +++++++++++-
 hw/qdev.c                |   48 +++++++++++++++++++++++++++++++++++++--------
 2 files changed, 51 insertions(+), 10 deletions(-)
Markus Armbruster - June 23, 2010, 9:10 a.m.
[cc: kraxel]

Jan Kiszka <jan.kiszka@web.de> writes:

> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Extend qbus_find_dev to allow addressing of devices without an unique id
> via an optional per-bus instance number. The new formats are
> 'driver.instance' and 'alias.instance'. Attach this name extension
> whenever an instantiated device is printed.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

The instance number repairs the specific problem created by the previous
patch.  My objection to it on general principles stands.

That said, I'm neutral on the instance number feature.  Instance numbers
are not suitable where address stability is needed (see debate on
canonical addresses), but they might be convenient for interactive use.

There is some functional overlap to the [driver-name]@bus-address syntax
proposed elsewhere.  Especially visible for buses where the bus-address
is a small integer.  Could be confusing.

There is some conceptual overlap with the .NUMBER in default bus names
(see qbus_create_inplace()).  Instance numbers count for each name
independently.  Numbers in default bus names count for all names.  But I
think we agree the latter sucks.  Paul wants us to get rid of it.

> ---
>  docs/qdev-device-use.txt |   13 +++++++++++-
>  hw/qdev.c                |   48 +++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 51 insertions(+), 10 deletions(-)
>
> diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
> index f252c8e..58f2630 100644
> --- a/docs/qdev-device-use.txt
> +++ b/docs/qdev-device-use.txt
> @@ -1,6 +1,6 @@
>  = How to convert to -device & friends =
>  
> -=== Specifying Bus and Address on Bus ===
> +=== Specifying Bus, Address on Bus, and Devices ===
>  
>  In qdev, each device has a parent bus.  Some devices provide one or
>  more buses for children.  You can specify a device's parent bus with
> @@ -20,6 +20,17 @@ bus named pci.0.  To put a FOO device into its slot 4, use -device
>  FOO,bus=/i440FX-pcihost/pci.0,addr=4.  The abbreviated form bus=pci.0
>  also works as long as the bus name is unique.
>  
> +Existing devices can be addressed either via a unique ID if it was
> +assigned during creation or via the device tree path:
> +
> +/full_bus_address/driver_name[.instance_number]
> +
> +The instance number counts devices managed by the same driver on a
> +specifc bus. It is zero-based.
   ~~~~~~~ typo

> +
> +Example: /i440FX-pcihost/pci.0/e1000.1 addresses the second e1000
> +adapter on the bus 'pci.0'.
> +
>  Note: the USB device address can't be controlled at this time.
>  
>  === Block Devices ===
[...]

Patch

diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
index f252c8e..58f2630 100644
--- a/docs/qdev-device-use.txt
+++ b/docs/qdev-device-use.txt
@@ -1,6 +1,6 @@ 
 = How to convert to -device & friends =
 
-=== Specifying Bus and Address on Bus ===
+=== Specifying Bus, Address on Bus, and Devices ===
 
 In qdev, each device has a parent bus.  Some devices provide one or
 more buses for children.  You can specify a device's parent bus with
@@ -20,6 +20,17 @@  bus named pci.0.  To put a FOO device into its slot 4, use -device
 FOO,bus=/i440FX-pcihost/pci.0,addr=4.  The abbreviated form bus=pci.0
 also works as long as the bus name is unique.
 
+Existing devices can be addressed either via a unique ID if it was
+assigned during creation or via the device tree path:
+
+/full_bus_address/driver_name[.instance_number]
+
+The instance number counts devices managed by the same driver on a
+specifc bus. It is zero-based.
+
+Example: /i440FX-pcihost/pci.0/e1000.1 addresses the second e1000
+adapter on the bus 'pci.0'.
+
 Note: the USB device address can't be controlled at this time.
 
 === Block Devices ===
diff --git a/hw/qdev.c b/hw/qdev.c
index aa25155..f4ae4a6 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -492,12 +492,29 @@  static DeviceState *qdev_find_recursive(BusState *bus, const char *id)
     return NULL;
 }
 
+static int qdev_instance_no(DeviceState *dev)
+{
+    struct DeviceState *sibling;
+    int instance = 0;
+
+    QLIST_FOREACH(sibling, &dev->parent_bus->children, sibling) {
+        if (sibling->info == dev->info) {
+            if (sibling == dev) {
+                break;
+            }
+            instance++;
+        }
+    }
+    return instance;
+}
+
 static void qbus_list_bus(DeviceState *dev)
 {
     BusState *child;
     const char *sep = " ";
 
-    error_printf("child busses at \"%s\":", dev->info->name);
+    error_printf("child busses at \"%s.%d\":",
+                 dev->info->name, qdev_instance_no(dev));
     QLIST_FOREACH(child, &dev->child_bus, sibling) {
         error_printf("%s\"%s\"", sep, child->name);
         sep = ", ";
@@ -512,7 +529,8 @@  static void qbus_list_dev(BusState *bus)
 
     error_printf("devices at \"%s\":", bus->name);
     QLIST_FOREACH(dev, &bus->children, sibling) {
-        error_printf("%s\"%s\"", sep, dev->info->name);
+        error_printf("%s\"%s.%d\"", sep, dev->info->name,
+                     qdev_instance_no(dev));
         sep = ", ";
     }
     error_printf("\n");
@@ -530,23 +548,35 @@  static BusState *qbus_find_bus(DeviceState *dev, char *elem)
     return NULL;
 }
 
-static DeviceState *qbus_find_dev(BusState *bus, char *elem)
+static DeviceState *qbus_find_dev(BusState *bus, const char *elem)
 {
     DeviceState *dev;
+    int instance, n;
+    char buf[128];
 
     /*
      * try to match in order:
-     *   (1) driver name
-     *   (2) driver alias, if present
+     *   (1) driver name [.instance]
+     *   (2) driver alias [.instance], if present
      */
 
+    if (sscanf(elem, "%127[^.].%u", buf, &instance) == 2) {
+        elem = buf;
+    } else {
+        instance = 0;
+    }
+
+    n = 0;
     QLIST_FOREACH(dev, &bus->children, sibling) {
-        if (strcmp(dev->info->name, elem) == 0) {
+        if (strcmp(dev->info->name, elem) == 0 && n++ == instance) {
             return dev;
         }
     }
+
+    n = 0;
     QLIST_FOREACH(dev, &bus->children, sibling) {
-        if (dev->info->alias && strcmp(dev->info->alias, elem) == 0) {
+        if (dev->info->alias && strcmp(dev->info->alias, elem) == 0 &&
+            n++ == instance) {
             return dev;
         }
     }
@@ -710,8 +740,8 @@  static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
 static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
 {
     BusState *child;
-    qdev_printf("dev: %s, id \"%s\"\n", dev->info->name,
-                dev->id ? dev->id : "");
+    qdev_printf("dev: %s.%d, id \"%s\"\n", dev->info->name,
+                qdev_instance_no(dev), dev->id ? dev->id : "");
     indent += 2;
     if (dev->num_gpio_in) {
         qdev_printf("gpio-in %d\n", dev->num_gpio_in);