From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56483) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VuK1K-0002n9-QQ for qemu-devel@nongnu.org; Sat, 21 Dec 2013 05:43:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VuK1B-0000Zf-TS for qemu-devel@nongnu.org; Sat, 21 Dec 2013 05:42:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49144) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VuK1B-0000ZL-Kn for qemu-devel@nongnu.org; Sat, 21 Dec 2013 05:42:49 -0500 From: Markus Armbruster References: <1387503694-11203-1-git-send-email-agraf@suse.de> Date: Sat, 21 Dec 2013 11:42:43 +0100 In-Reply-To: <1387503694-11203-1-git-send-email-agraf@suse.de> (Alexander Graf's message of "Fri, 20 Dec 2013 02:41:34 +0100") Message-ID: <87lhzelc2k.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v2] qdev: Keep global allocation counter per bus List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Paolo Bonzini , QEMU Developers , Anthony Liguori Alexander Graf writes: > 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. Suggest to add: ", killing migration." Which boards are affected? Should be listed in the commit message! I ran a quick test for all boards that actually make it to the monitor without special parameters or files, and survive "info qtree". 164 boards can do that, 59 refuse to start, one crashes on start, 10 make it to the monitor but crash in info qtree. Not nice. If there's a way to start *any* board to the monitor, please let me know. Anyway, I found the following machines sporting non-unique bus names before your patch: target machine bus id times aarch64 n800 i2c-bus.0 2 aarch64 n810 i2c-bus.0 2 aarch64 nuri i2c 9 aarch64 smdkc210 i2c 9 aarch64 vexpress-a15 virtio-mmio-bus.0 4 aarch64 vexpress-a9 virtio-mmio-bus.0 4 aarch64 virt virtio-mmio-bus.0 32 aarch64 xilinx-zynq-a9 usb-bus.0 2 aarch64 xilinx-zynq-a9 spi0 3 arm n800 i2c-bus.0 2 arm n810 i2c-bus.0 2 arm nuri i2c 9 arm smdkc210 i2c 9 arm vexpress-a15 virtio-mmio-bus.0 4 arm vexpress-a9 virtio-mmio-bus.0 4 arm virt virtio-mmio-bus.0 32 arm xilinx-zynq-a9 usb-bus.0 2 arm xilinx-zynq-a9 spi0 3 i386 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 x86_64 isapc ide.0 2 The isapc crash with v1 demonstrates that such machines need testing with your patch. Not yet covered: target machine reason aarch64 akita info qtree crashes aarch64 borzoi info qtree crashes aarch64 canon-a1100 refuses to start aarch64 cheetah refuses to start aarch64 connex refuses to start aarch64 lm3s6965evb refuses to start aarch64 lm3s811evb refuses to start aarch64 mainstone refuses to start aarch64 spitz info qtree crashes aarch64 sx1 refuses to start aarch64 sx1-v1 refuses to start aarch64 terrier info qtree crashes aarch64 tosa info qtree crashes aarch64 verdex refuses to start aarch64 z2 refuses to start arm akita info qtree crashes arm borzoi info qtree crashes arm canon-a1100 refuses to start arm cheetah refuses to start arm connex refuses to start arm lm3s6965evb refuses to start arm lm3s811evb refuses to start arm mainstone refuses to start arm spitz info qtree crashes arm sx1 refuses to start arm sx1-v1 refuses to start arm terrier info qtree crashes arm tosa info qtree crashes arm verdex refuses to start arm z2 refuses to start cris axis-dev88 refuses to start i386 xenfv refuses to start i386 xenpv refuses to start lm32 milkymist refuses to start m68k an5206 refuses to start m68k mcf5208evb refuses to start mips magnum refuses to start mips malta refuses to start mips mipssim refuses to start mips pica61 refuses to start mips64 magnum refuses to start mips64 malta refuses to start mips64 mipssim refuses to start mips64 pica61 refuses to start mips64el fulong2e refuses to start mips64el magnum refuses to start mips64el malta refuses to start mips64el mipssim refuses to start mips64el pica61 refuses to start mipsel magnum refuses to start mipsel malta refuses to start mipsel mipssim refuses to start mipsel pica61 refuses to start ppc ref405ep refuses to start ppc taihu refuses to start ppc64 ref405ep refuses to start ppc64 taihu refuses to start ppcemb g3beige refuses to start ppcemb mac99 refuses to start ppcemb mpc8544ds refuses to start ppcemb ppce500 refuses to start ppcemb prep refuses to start ppcemb ref405ep refuses to start ppcemb taihu refuses to start sh4 shix refuses to start sh4eb shix refuses to start sparc leon3_generic refuses to start unicore32 puv3 crashes on startup x86_64 xenfv refuses to start x86_64 xenpv refuses to start > > This is intended and makes the bus enumeration work as expected. > > CC: Paolo Bonzini > CC: Markus Armbruster > CC: Anthony Liguori > Signed-off-by: Alexander Graf > > --- > > v1 -> v2: > > - add fix for isapc which was searching for 2 buses called "ide.0" Yes, this version no longer crashes on startup. The secondary controller's bus gets renamed to "ide.1". > - explain the semantic change more in the commit message Patch looks good to me, but I'd recommend more thorough testing, as outlined above.