Re: [libvirt] [PATCH v5 2/4] qemu: Add PCI-Express root to ARM virt machine

On Thu, Aug 06, 2015 at 06:37:41PM +0200, Martin Kletzander wrote: >On Fri, Jul 17, 2015 at 02:27:45PM +0300, Pavel Fedin wrote: >>Here we assume that if qemu supports generic PCI host controller, >>it is a part of virt machine and can be used for adding PCI devices. >> >>In qemu this is actually a PCIe bus, so we also declare multibus >>capability so that 0'th bus is specified to qemu correctly as 'pcie.0' >> >>Signed-off-by: Pavel Fedin <p.fedin(a)samsung.com&gt; >>--- >>src/qemu/qemu_capabilities.c | 8 ++++++++ >>src/qemu/qemu_domain.c | 17 +++++++++++++---- >>2 files changed, 21 insertions(+), 4 deletions(-) >> >>diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c >>index d570fdd..f3486c7 100644 >>--- a/src/qemu/qemu_capabilities.c >>+++ b/src/qemu/qemu_capabilities.c >>@@ -2138,6 +2138,14 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, >> return false; >> } >> >>+ if (ARCH_IS_ARM(def->os.arch)) { >>+ /* If 'virt' supports PCI, it supports multibus. >>+ * No extra conditions here for simplicity. >>+ */ > >So every ARM qemu with the "virt" machine type supports both PCI and >multiqueue? How about those "virt-*" for which you check below. That >might not be related, I'm just curious. > >>+ if (STREQ(def->os.machine, "virt")) >>+ return true; >>+ } >>+ >> return false; >>} >> >>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c >>index 8b050a0..c7d14e4 100644 >>--- a/src/qemu/qemu_domain.c >>+++ b/src/qemu/qemu_domain.c >>@@ -981,7 +981,7 @@ virDomainXMLNamespace virQEMUDriverDomainXMLNamespace = { >>static int >>qemuDomainDefPostParse(virDomainDefPtr def, >> virCapsPtr caps, >>- void *opaque ATTRIBUTE_UNUSED) >>+ void *opaque) >>{ >> bool addDefaultUSB = true; >> bool addImplicitSATA = false; >>@@ -1030,12 +1030,21 @@ qemuDomainDefPostParse(virDomainDefPtr def, >> break; >> >> case VIR_ARCH_ARMV7L: >>- addDefaultUSB = false; >>- addDefaultMemballoon = false; >>- break; >> case VIR_ARCH_AARCH64: >> addDefaultUSB = false; >> addDefaultMemballoon = false; >>+ if (STREQ(def->os.machine, "virt") || >>+ STRPREFIX(def->os.machine, "virt-")) { >>+ virQEMUDriverPtr driver = opaque; >>+ >>+ /* This condition is actually a (temporary) hack for test suite which >>+ * does not create capabilities cache */ > >Few questions here. a) how "temporary" is this since you're not >removing it in this series? b) for what tests you need this hack and >what part of the below is the hack? > >Moreover, you cannot use capabilities when defining an XML. The >emulator can change between the domain is defined and started, so you >cannot know with what emulator this will be started. > >I see Michal (Cc'd) just pushed this, I probably just missed the mail Of course I forgot, Cc'ing now.