* [Qemu-devel] [PATCH v4] qdev: Keep global allocation counter per bus
@ 2014-02-06 15:08 Markus Armbruster
2014-02-18 12:54 ` Markus Armbruster
2014-02-18 16:22 ` Andreas Färber
0 siblings, 2 replies; 5+ messages in thread
From: Markus Armbruster @ 2014-02-06 15:08 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Alexander Graf, Anthony Liguori
From: Alexander Graf <agraf@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 <pbonzini@redhat.com>
CC: Anthony Liguori <aliguori@amazon.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Markus Armbruster <armbru@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 {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v4] qdev: Keep global allocation counter per bus
2014-02-06 15:08 [Qemu-devel] [PATCH v4] qdev: Keep global allocation counter per bus Markus Armbruster
@ 2014-02-18 12:54 ` Markus Armbruster
2014-02-18 12:57 ` Andreas Färber
2014-02-18 16:22 ` Andreas Färber
1 sibling, 1 reply; 5+ messages in thread
From: Markus Armbruster @ 2014-02-18 12:54 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Andreas Färber, Alexander Graf,
Anthony Liguori, peter.maydell
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.
Markus Armbruster <armbru@redhat.com> writes:
> From: Alexander Graf <agraf@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 <pbonzini@redhat.com>
> CC: Anthony Liguori <aliguori@amazon.com>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Markus Armbruster <armbru@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 {
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v4] qdev: Keep global allocation counter per bus
2014-02-18 12:54 ` Markus Armbruster
@ 2014-02-18 12:57 ` Andreas Färber
2014-02-18 13:17 ` Markus Armbruster
0 siblings, 1 reply; 5+ messages in thread
From: Andreas Färber @ 2014-02-18 12:57 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
Cc: Paolo Bonzini, Alexander Graf, Anthony Liguori, peter.maydell
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 <armbru@redhat.com> writes:
>
>> From: Alexander Graf <agraf@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 <pbonzini@redhat.com>
>> CC: Anthony Liguori <aliguori@amazon.com>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Markus Armbruster <armbru@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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v4] qdev: Keep global allocation counter per bus
2014-02-18 12:57 ` Andreas Färber
@ 2014-02-18 13:17 ` Markus Armbruster
0 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2014-02-18 13:17 UTC (permalink / raw)
To: Andreas Färber
Cc: peter.maydell, Michael S. Tsirkin, qemu-devel, Alexander Graf,
Anthony Liguori, Paolo Bonzini
Andreas Färber <afaerber@suse.de> writes:
> 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
Yes, we discussed migration at some length, but the problem to resolve
was documenting the breakage, which we did in v4.
We break -device bus=FOO.0 in the case where we have multiple FOO.0
before the patch (all but one unusable with -device), and orderly FOO.0,
FOO.1, ... after. We (Paolo, Alex and I) agreed that avoiding this
breakage isn't worthwhile. The commit message enumerates the affected
buses, and explains how to work around the migration breakage.
> patch I would take it through qom-next, if mst acks for the PC part.
Thanks. Michael, please review :)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v4] qdev: Keep global allocation counter per bus
2014-02-06 15:08 [Qemu-devel] [PATCH v4] qdev: Keep global allocation counter per bus Markus Armbruster
2014-02-18 12:54 ` Markus Armbruster
@ 2014-02-18 16:22 ` Andreas Färber
1 sibling, 0 replies; 5+ messages in thread
From: Andreas Färber @ 2014-02-18 16:22 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel, Alexander Graf
Cc: Paolo Bonzini, Anthony Liguori, Michael S. Tsirkin
Am 06.02.2014 16:08, schrieb Markus Armbruster:
> From: Alexander Graf <agraf@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 <pbonzini@redhat.com>
> CC: Anthony Liguori <aliguori@amazon.com>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Since Alex has it in his ppc queue,
Reviewed-by: Andreas Färber <afaerber@suse.de>
The PC change looks straightforward too and if I'm interpreting the "if
(pci_enabled) { ... } else" correctly it should be covered by qom-test
running -M isapc, so I'll queue it if testing succeeds.
Reviews/acks/nacks still appreciated.
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-02-18 16:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-06 15:08 [Qemu-devel] [PATCH v4] qdev: Keep global allocation counter per bus Markus Armbruster
2014-02-18 12:54 ` Markus Armbruster
2014-02-18 12:57 ` Andreas Färber
2014-02-18 13:17 ` Markus Armbruster
2014-02-18 16:22 ` Andreas Färber
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).