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

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

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@xxxxxxxxxxx>
> ---
> 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
from him.  Anyway, I don't think this is how this should be handled.
I know this is bit of a chicken-and-egg problem due to the fact that
we generate addresses during definitions.  But I think we could figure
something out instead of this hack.

> +           if (driver->qemuCapsCache) {
> +               virQEMUCapsPtr qemuCaps =
> +                   virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);
> +               addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX);
> +           }
> +       }
>        break;
>
>     case VIR_ARCH_PPC64:
> --
> 1.9.5.msysgit.0
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list