[Qemu-devel] [PATCH] pseries: Fix qdev.id handling in the VIO bus code

[Qemu-devel] [PATCH] pseries: Fix qdev.id handling in the VIO bus code

[David Gibson]

From: Michael Ellerman <michael@ellerman.id.au>

When the user creates a device on the command line with -device, they
can specify the id, using id=foo. Currently the VIO bus code overwrites
this id with it's own value. We should only set qdev.id if it is not
already set by the user.

The device tree code uses qdev.id for the device tree node name, however
we can't rely on the user specifiying the id using proper device tree
syntax, ie. device@reg. So separate the device tree node name from the
qdev.id, but use the same syntax, so they will match by default.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 hw/spapr_vio.c |   36 +++++++++++++++++++++++++++++-------
 1 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index b7b3ddd..2dcc036 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -73,20 +73,39 @@ VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg)
     return NULL;
 }
 
+static char *vio_format_dev_name(VIOsPAPRDevice *dev)
+{
+    VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)dev->qdev.info;
+    char *name;
+
+    /* Device tree style name device@reg */
+    if (asprintf(&name, "%s@%x", info->dt_name, dev->reg) < 0) {
+        return NULL;
+    }
+
+    return name;
+}
+
 #ifdef CONFIG_FDT
 static int vio_make_devnode(VIOsPAPRDevice *dev,
                             void *fdt)
 {
     VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)dev->qdev.info;
-    int vdevice_off, node_off;
-    int ret;
+    int vdevice_off, node_off, ret;
+    char *dt_name;
 
     vdevice_off = fdt_path_offset(fdt, "/vdevice");
     if (vdevice_off < 0) {
         return vdevice_off;
     }
 
-    node_off = fdt_add_subnode(fdt, vdevice_off, dev->qdev.id);
+    dt_name = vio_format_dev_name(dev);
+    if (!dt_name) {
+        return -ENOMEM;
+    }
+
+    node_off = fdt_add_subnode(fdt, vdevice_off, dt_name);
+    free(dt_name);
     if (node_off < 0) {
         return node_off;
     }
@@ -608,12 +627,15 @@ static int spapr_vio_busdev_init(DeviceState *qdev, DeviceInfo *qinfo)
     VIOsPAPRDevice *dev = (VIOsPAPRDevice *)qdev;
     char *id;
 
-    if (asprintf(&id, "%s@%x", info->dt_name, dev->reg) < 0) {
-        return -1;
+    /* Don't overwrite ids assigned on the command line */
+    if (!dev->qdev.id) {
+        id = vio_format_dev_name(dev);
+        if (!id) {
+            return -1;
+        }
+        dev->qdev.id = id;
     }
 
-    dev->qdev.id = id;
-
     dev->qirq = spapr_allocate_irq(dev->vio_irq_num, &dev->vio_irq_num);
     if (!dev->qirq) {
         return -1;
-- 
1.7.7.1

Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Fix qdev.id handling in the VIO bus code

[David Gibson]

On Wed, Nov 16, 2011 at 03:53:13PM +1100, David Gibson wrote:
> From: Michael Ellerman <michael@ellerman.id.au>
> 
> When the user creates a device on the command line with -device, they
> can specify the id, using id=foo. Currently the VIO bus code overwrites
> this id with it's own value. We should only set qdev.id if it is not
> already set by the user.
> 
> The device tree code uses qdev.id for the device tree node name, however
> we can't rely on the user specifiying the id using proper device tree
> syntax, ie. device@reg. So separate the device tree node name from the
> qdev.id, but use the same syntax, so they will match by default.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

This is again a bugfix which I hope will go into 1.0.
-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Fix qdev.id handling in the VIO bus code

[Alexander Graf]

On 16.11.2011, at 06:25, David Gibson wrote:

> On Wed, Nov 16, 2011 at 03:53:13PM +1100, David Gibson wrote:
>> From: Michael Ellerman <michael@ellerman.id.au>
>> 
>> When the user creates a device on the command line with -device, they
>> can specify the id, using id=foo. Currently the VIO bus code overwrites
>> this id with it's own value. We should only set qdev.id if it is not
>> already set by the user.
>> 
>> The device tree code uses qdev.id for the device tree node name, however
>> we can't rely on the user specifiying the id using proper device tree
>> syntax, ie. device@reg. So separate the device tree node name from the
>> qdev.id, but use the same syntax, so they will match by default.
>> 
>> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> 
> This is again a bugfix which I hope will go into 1.0.

Me too. Queued for 1.0.

Btw, next time please put your sob on it as well.


Alex