[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4] qdev: Keep global allocation counter per bus
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH v4] qdev: Keep global allocation counter per bus |
Date: |
Tue, 18 Feb 2014 13:57:56 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
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 <address@hidden> writes:
>
>> From: Alexander Graf <address@hidden>
>>
>> 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 <address@hidden>
>> CC: Anthony Liguori <address@hidden>
>> Signed-off-by: Alexander Graf <address@hidden>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> 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