Am 18.02.2014 13:54, schrieb Markus Armbruster: > Peter, can you merge this patch? v2 is from December 20, and the > changes since then have been limited to the commit message. > > In case Peter doesn't want to take it directly: Andreas, would you be > willing to take it through your tree? It's not really QOM, but your > tree is the closest fit I can find.
Last thing I read was some open discussions related to migration problems, so I didn't take it yet. If those are resolved, as a qdev patch I would take it through qom-next, if mst acks for the PC part. Regards, Andreas > > Markus Armbruster <arm...@redhat.com> writes: > >> From: Alexander Graf <ag...@suse.de> >> >> When we have 2 separate qdev devices that both create a qbus of the >> same type without specifying a bus name or device name, we end up >> with two buses of the same name, such as ide.0 on the Mac machines: >> >> dev: macio-ide, id "" >> bus: ide.0 >> type IDE >> dev: macio-ide, id "" >> bus: ide.0 >> type IDE >> >> If we now spawn a device that connects to a ide.0 the last created >> bus gets the device, with the first created bus inaccessible to the >> command line. >> >> After some discussion on IRC we concluded that the best quick fix way >> forward for this is to make automated bus-class type based allocation >> count a global counter. That's what this patch implements. With this >> we instead get >> >> dev: macio-ide, id "" >> bus: ide.1 >> type IDE >> dev: macio-ide, id "" >> bus: ide.0 >> type IDE >> >> on the example mentioned above. >> >> This also means that if you did -device ...,bus=ide.0 you got a device >> on the first bus (the last created one) before this patch and get that >> device on the second one (the first created one) now. Breaks >> migration unless you change bus=ide.0 to bus=ide.1 on the destination. >> >> This is intended and makes the bus enumeration work as expected. >> >> As per review request follows a list of otherwise affected boards and >> the reasoning for the conclusion that they are ok: >> >> target machine bus id times >> ------ ------- ------ ----- >> >> aarch64 n800 i2c-bus.0 2 >> aarch64 n810 i2c-bus.0 2 >> arm n800 i2c-bus.0 2 >> arm n810 i2c-bus.0 2 >> >> -> Devices are only created explicitly on one of the two buses, using >> s->mpu->i2c[0], so no change to the guest. >> >> aarch64 vexpress-a15 virtio-mmio-bus.0 4 >> aarch64 vexpress-a9 virtio-mmio-bus.0 4 >> aarch64 virt virtio-mmio-bus.0 32 >> arm vexpress-a15 virtio-mmio-bus.0 4 >> arm vexpress-a9 virtio-mmio-bus.0 4 >> arm virt virtio-mmio-bus.0 32 >> >> -> Makes -device bus= work for all virtio-mmio buses. Breaks >> migration. Workaround for migration from old to new: specify >> virtio-mmio-bus.4 or .32 respectively rather than .0 on the >> destination. >> >> aarch64 xilinx-zynq-a9 usb-bus.0 2 >> arm xilinx-zynq-a9 usb-bus.0 2 >> mips64el fulong2e usb-bus.0 2 >> >> -> Normal USB operation not affected. Migration driver needs command >> line to use the other bus. >> >> i386 isapc ide.0 2 >> x86_64 isapc ide.0 2 >> mips mips ide.0 2 >> mips64 mips ide.0 2 >> mips64el mips ide.0 2 >> mipsel mips ide.0 2 >> ppc g3beige ide.0 2 >> ppc mac99 ide.0 2 >> ppc prep ide.0 2 >> ppc64 g3beige ide.0 2 >> ppc64 mac99 ide.0 2 >> ppc64 prep ide.0 2 >> >> -> Makes -device bus= work for all IDE buses. Breaks migration. >> Workaround for migration from old to new: specify ide.1 rather than >> ide.0 on the destination. >> >> CC: Paolo Bonzini <pbonz...@redhat.com> >> CC: Anthony Liguori <aligu...@amazon.com> >> Signed-off-by: Alexander Graf <ag...@suse.de> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> v1 -> v2: >> >> - add fix for isapc which was searching for 2 buses called "ide.0" >> - explain the semantic change more in the commit message >> >> v2 -> v3: >> >> - add board list to commit message >> >> v3 -> v4: >> >> - Explain impact on migration more clearly in commit message. >> >> hw/core/qdev.c | 20 +++++++++++++------- >> hw/i386/pc_piix.c | 8 +++++++- >> include/hw/qdev-core.h | 2 ++ >> 3 files changed, 22 insertions(+), 8 deletions(-) >> >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >> index 82a9123..e7985fe 100644 >> --- a/hw/core/qdev.c >> +++ b/hw/core/qdev.c >> @@ -430,27 +430,33 @@ DeviceState *qdev_find_recursive(BusState *bus, const >> char *id) >> static void qbus_realize(BusState *bus, DeviceState *parent, const char >> *name) >> { >> const char *typename = object_get_typename(OBJECT(bus)); >> + BusClass *bc; >> char *buf; >> - int i,len; >> + int i, len, bus_id; >> >> bus->parent = parent; >> >> if (name) { >> bus->name = g_strdup(name); >> } else if (bus->parent && bus->parent->id) { >> - /* parent device has id -> use it for bus name */ >> + /* parent device has id -> use it plus parent-bus-id for bus name */ >> + bus_id = bus->parent->num_child_bus; >> + >> len = strlen(bus->parent->id) + 16; >> buf = g_malloc(len); >> - snprintf(buf, len, "%s.%d", bus->parent->id, >> bus->parent->num_child_bus); >> + snprintf(buf, len, "%s.%d", bus->parent->id, bus_id); >> bus->name = buf; >> } else { >> - /* no id -> use lowercase bus type for bus name */ >> + /* no id -> use lowercase bus type plus global bus-id for bus name >> */ >> + bc = BUS_GET_CLASS(bus); >> + bus_id = bc->automatic_ids++; >> + >> len = strlen(typename) + 16; >> buf = g_malloc(len); >> - len = snprintf(buf, len, "%s.%d", typename, >> - bus->parent ? bus->parent->num_child_bus : 0); >> - for (i = 0; i < len; i++) >> + len = snprintf(buf, len, "%s.%d", typename, bus_id); >> + for (i = 0; i < len; i++) { >> buf[i] = qemu_tolower(buf[i]); >> + } >> bus->name = buf; >> } >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index a327d71..62a9a22 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -221,10 +221,16 @@ static void pc_init1(QEMUMachineInitArgs *args, >> } else { >> for(i = 0; i < MAX_IDE_BUS; i++) { >> ISADevice *dev; >> + char busname[] = "ide.0"; >> dev = isa_ide_init(isa_bus, 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(DEVICE(dev), "ide.0"); >> + /* >> + * The ide bus name is ide.0 for the first bus and ide.1 for the >> + * second one. >> + */ >> + busname[4] = '0' + i; >> + idebus[i] = qdev_get_child_bus(DEVICE(dev), busname); >> } >> } >> >> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h >> index 2c4f140..a299bc6 100644 >> --- a/include/hw/qdev-core.h >> +++ b/include/hw/qdev-core.h >> @@ -172,6 +172,8 @@ struct BusClass { >> void (*reset)(BusState *bus); >> /* maximum devices allowed on the bus, 0: no limit. */ >> int max_dev; >> + /* number of automatically allocated bus ids (e.g. ide.0) */ >> + int automatic_ids; >> }; >> >> typedef struct BusChild { -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg