Re: [Qemu-devel] [PATCHv2 4/8] Store IDE bus id in IDEBus structure for easy access.

[Gleb Natapov]

On Wed, Nov 03, 2010 at 02:39:52PM +0100, Markus Armbruster wrote:
> Here's a generic answer to the question "which of the device's buses is
> this?"
> 
> int qbus_index(BusState *bus)
> {
>     BusState *b;
>     int i, index;
> 
>     index = -1;
>     i = 0;
>     QLIST_FOREACH(b, &bus->parent->child_bus, sibling) {
>         if (b == bus) {
>             index = i;
>         }
>         i++;
>     }
>     assert(0 <= index && index < i);
>     return i - 1 - index;
> }
> 
> The bus created first has index 0.
> 
> Note that the child_bus holds the children in reverse creation order,
> and we can't traverse it backwards.  Same problem also visible with
> makes info qtree:
> 
>       dev: piix3-ide, id ""
> [...]
>         bus: ide.1
>           type IDE
>         bus: ide.0
>           type IDE
Isn't this too implementation dependant? Are you against adding bus_id
to IDEBus? And will it work with ISA? I do not think IDEBus is
added to bus->parent->child_bus in case of ISA otherwise why both
IDEBuses have same name in isapc. Currently I have following patch
queued. It should fix isapc case too.

    Fix isapc IDE bus creation
    
    Currently we create two ide buses with the same name, so it is impossible
    to use -device ide-drive,bus= to address all possible disks in the isapc
    machine. Fix that by giving different names to different ide buses. Also
    store IDE bus id in IDEBus structure for easy access.

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index ff80dd5..b2cbdbc 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -257,8 +257,8 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
     pci_conf[PCI_INTERRUPT_PIN] = 0x01; // interrupt on pin 1
 
     irq = qemu_allocate_irqs(cmd646_set_irq, d, 2);
-    ide_bus_new(&d->bus[0], &d->dev.qdev);
-    ide_bus_new(&d->bus[1], &d->dev.qdev);
+    ide_bus_new(&d->bus[0], &d->dev.qdev, 0);
+    ide_bus_new(&d->bus[1], &d->dev.qdev, 1);
     ide_init2(&d->bus[0], irq[0]);
     ide_init2(&d->bus[1], irq[1]);
 
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 4165543..bde2664 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -448,6 +448,7 @@ struct IDEBus {
     IDEDevice *slave;
     BMDMAState *bmdma;
     IDEState ifs[2];
+    uint8_t bus_id;
     uint8_t unit;
     uint8_t cmd;
     qemu_irq irq;
@@ -564,7 +565,7 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo 
*hd0,
 void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
 
 /* hw/ide/qdev.c */
-void ide_bus_new(IDEBus *idebus, DeviceState *dev);
+void ide_bus_new(IDEBus *idebus, DeviceState *dev, uint8_t bus_id);
 IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
 
 #endif /* HW_IDE_INTERNAL_H */
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 9b94495..b000ab8 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -66,8 +66,9 @@ static const VMStateDescription vmstate_ide_isa = {
 static int isa_ide_initfn(ISADevice *dev)
 {
     ISAIDEState *s = DO_UPCAST(ISAIDEState, dev, dev);
+    static uint8_t bus_id = 0;
 
-    ide_bus_new(&s->bus, &s->dev.qdev);
+    ide_bus_new(&s->bus, &s->dev.qdev, bus_id++);
     ide_init_ioport(&s->bus, s->iobase, s->iobase2);
     isa_init_irq(dev, &s->irq, s->isairq);
     isa_init_ioport_range(dev, s->iobase, 8);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 6206201..e56777f 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -129,8 +129,8 @@ static int pci_piix_ide_initfn(PCIIDEState *d)
 
     vmstate_register(&d->dev.qdev, 0, &vmstate_ide_pci, d);
 
-    ide_bus_new(&d->bus[0], &d->dev.qdev);
-    ide_bus_new(&d->bus[1], &d->dev.qdev);
+    ide_bus_new(&d->bus[0], &d->dev.qdev, 0);
+    ide_bus_new(&d->bus[1], &d->dev.qdev, 1);
     ide_init_ioport(&d->bus[0], 0x1f0, 0x3f6);
     ide_init_ioport(&d->bus[1], 0x170, 0x376);
 
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 336ffe1..220729e 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -29,9 +29,13 @@ static struct BusInfo ide_bus_info = {
     .size  = sizeof(IDEBus),
 };
 
-void ide_bus_new(IDEBus *idebus, DeviceState *dev)
+void ide_bus_new(IDEBus *idebus, DeviceState *dev, uint8_t bus_id)
 {
-    qbus_create_inplace(&idebus->qbus, &ide_bus_info, dev, NULL);
+    char name[10];
+
+    snprintf(name, sizeof(name), "ide.%d", bus_id); 
+    qbus_create_inplace(&idebus->qbus, &ide_bus_info, dev, name);
+    idebus->bus_id = bus_id;
 }
 
 static int ide_qdev_init(DeviceState *qdev, DeviceInfo *base)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index b2c7cad..cc48b2b 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -158,8 +158,8 @@ static int vt82c686b_ide_initfn(PCIDevice *dev)
 
     vmstate_register(&dev->qdev, 0, &vmstate_ide_pci, d);
 
-    ide_bus_new(&d->bus[0], &d->dev.qdev);
-    ide_bus_new(&d->bus[1], &d->dev.qdev);
+    ide_bus_new(&d->bus[0], &d->dev.qdev, 0);
+    ide_bus_new(&d->bus[1], &d->dev.qdev, 1);
     ide_init2(&d->bus[0], isa_reserve_irq(14));
     ide_init2(&d->bus[1], isa_reserve_irq(15));
     ide_init_ioport(&d->bus[0], 0x1f0, 0x3f6);
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 3d07ce5..cec6c9b 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -163,7 +163,7 @@ static void pc_init1(ram_addr_t ram_size,
             ISADevice *dev;
             dev = isa_ide_init(ide_iobase[i], ide_iobase2[i], ide_irq[i],
                                hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
-            idebus[i] = qdev_get_child_bus(&dev->qdev, "ide.0");
+            idebus[i] = qdev_get_child_bus(&dev->qdev, i ? "ide.1" : "ide.0");
         }
     }
 
--
                        Gleb.