From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45992) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFkFT-0007LY-3i for qemu-devel@nongnu.org; Tue, 18 Feb 2014 07:58:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFkFN-0001ms-0G for qemu-devel@nongnu.org; Tue, 18 Feb 2014 07:58:07 -0500 Received: from cantor2.suse.de ([195.135.220.15]:53040 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFkFM-0001mm-Mb for qemu-devel@nongnu.org; Tue, 18 Feb 2014 07:58:00 -0500 Message-ID: <530358D4.3010505@suse.de> Date: Tue, 18 Feb 2014 13:57:56 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1391699295-8381-1-git-send-email-armbru@redhat.com> <87y518a8da.fsf@blackfin.pond.sub.org> In-Reply-To: <87y518a8da.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4] qdev: Keep global allocation counter per bus List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: Paolo Bonzini , Alexander Graf , Anthony Liguori , peter.maydell@linaro.org 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. >=20 > 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 >=20 > Markus Armbruster writes: >=20 >> From: Alexander Graf >> >> 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=3Dide.0 you got a devi= ce >> 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=3Dide.0 to bus=3Dide.1 on the destinat= ion. >> >> 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=3D 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=3D 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 >> CC: Anthony Liguori >> Signed-off-by: Alexander Graf >> Signed-off-by: Markus Armbruster >> --- >> 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 ch= ar *name) >> { >> const char *typename =3D object_get_typename(OBJECT(bus)); >> + BusClass *bc; >> char *buf; >> - int i,len; >> + int i, len, bus_id; >> =20 >> bus->parent =3D parent; >> =20 >> if (name) { >> bus->name =3D 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 =3D bus->parent->num_child_bus; >> + >> len =3D strlen(bus->parent->id) + 16; >> buf =3D 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 =3D 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 =3D BUS_GET_CLASS(bus); >> + bus_id =3D bc->automatic_ids++; >> + >> len =3D strlen(typename) + 16; >> buf =3D g_malloc(len); >> - len =3D snprintf(buf, len, "%s.%d", typename, >> - bus->parent ? bus->parent->num_child_bus : 0); >> - for (i =3D 0; i < len; i++) >> + len =3D snprintf(buf, len, "%s.%d", typename, bus_id); >> + for (i =3D 0; i < len; i++) { >> buf[i] =3D qemu_tolower(buf[i]); >> + } >> bus->name =3D buf; >> } >> =20 >> 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 =3D 0; i < MAX_IDE_BUS; i++) { >> ISADevice *dev; >> + char busname[] =3D "ide.0"; >> dev =3D 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] =3D 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] =3D '0' + i; >> + idebus[i] =3D qdev_get_child_bus(DEVICE(dev), busname); >> } >> } >> =20 >> 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; >> }; >> =20 >> typedef struct BusChild { --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg