Re: [libvirt] [PATCH v2 11/11] qemu: auto-add pci-root controller for pc machine types

On 04/18/2013 08:09 AM, Ján Tomko wrote: > On 04/18/2013 07:22 AM, Laine Stump wrote: >> On 04/17/2013 03:00 PM, Ján Tomko wrote: >>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >>> index 68518a7..a2179aa 100644 >>> --- a/src/conf/domain_conf.c >>> +++ b/src/conf/domain_conf.c >>> @@ -10849,9 +10849,15 @@ virDomainDefParseXML(xmlDocPtr xml, >>> >>> if (def->virtType == VIR_DOMAIN_VIRT_QEMU || >>> def->virtType == VIR_DOMAIN_VIRT_KQEMU || >>> - def->virtType == VIR_DOMAIN_VIRT_KVM) >>> + def->virtType == VIR_DOMAIN_VIRT_KVM) { >>> if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_USB, 0, -1) < 0) >>> goto error; >>> + if (STRPREFIX(def->os.machine,"pc")) { >> >> You also need to do this for all rhel-* machine types. Even though that >> machine type only shows up in RHEL-specific versions of qemu, it is >> possible for someone with a stock RHEL qemu to install upstream libvirt, >> and we don't want to break that. > It seems these are not the only machines with a PCI bus (PPC for example > has one), which makes me wonder how many things this will break. Well, we're going to need to figure those out. We can either do it for all, and let people interested in particular machinetypes explicitly exclude theirs, or we can do it only for those machinetypes that we know have a PCI bus, and wait for the others to complain; I guess it all depends on if we want to minimize pain, or minimize the amount of time that passes before we get it right. I'm not sure if the presence of "PCI" in the output of qemu-system-* -help is an accurate indicator of which platforms have a PCI bus, but I tried running that on all the qemu emulators on my Fedora 18 system and got the following list of qemu binaries that have devices claiming to live on a PCI bus: for e in qemu-system-*; do if $e -device \? 2>&1| grep -q PCI; then echo "YES - $e"; else echo "NO - $e"; fi; done YES - qemu-system-alpha YES - qemu-system-arm NO - qemu-system-cris YES - qemu-system-i386 NO - qemu-system-lm32 YES - qemu-system-m68k NO - qemu-system-microblaze NO - qemu-system-microblazeel YES - qemu-system-mips YES - qemu-system-mips64 YES - qemu-system-mips64el YES - qemu-system-mipsel NO - qemu-system-or32 YES - qemu-system-ppc YES - qemu-system-ppc64 YES - qemu-system-ppcemb NO - qemu-system-s390x YES - qemu-system-sh4 YES - qemu-system-sh4eb NO - qemu-system-sparc YES - qemu-system-sparc64 NO - qemu-system-unicore32 YES - qemu-system-x86_64 NO - qemu-system-xtensa NO - qemu-system-xtensaeb So within any binary that supports PCI devices, does that necessarily mean it's supported for *all* machinetypes? If so, we've got a pretty long list of machinetypes to check for: ( for e in qemu-system-*; do if $e -device \? 2>&1| grep -q PCI; then $e -M ? | cut -f1 -d' ' | grep -v Supported | grep -v none; fi; done ) | sort | uniq | wc 72 72 590 (note that it's bad enough we have to eliminate the opacity of machinetype; we *certainly* don't want to start trying to make decisions based on the emulator binary name!) And of course, just because a machinetype can support pci devices, doesn't necessarily mean that it has a builtin pci-root bus :-O