On 08/10/2015 11:09 AM, Daniel P. Berrange wrote:
> On Thu, Aug 06, 2015 at 07:46:58PM +0200, Martin Kletzander wrote:
>> 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@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
>>
>> Of course I forgot, Cc'ing now.
>
> I agree with your core statement that we should not be using the QEMU
> capabilities when defining the XML. With all existing scenarios we have
> been able to determine whether to add the implicit PCI controller based
> on the machine type name only, because with every other QEMU arch when
> doing such a major change as adding a PCI bus, they have created a new
> machine type. The problem is that arm 'virt' machine type is not stable,
> it is being changed arbitrarily in new QEMU releases :-(
>
> So AFAIK, that leaves us with 3 choices
>
> - Never add PCI controller at time the XML is defined, on the basis
> that we have to be conservative in what we add to cope with old QEMU
>
> - Always add PCI controller at time the XML is defined, on the basis
> that most people will have new enough QEMU because ARM 'virt' machine
> type is very much still in development, so no one will serously stick
> with the older QEMU versions which lack PCI.
>
> - Use the capabilities in XML post-parse to conditionally add the
> PCI controller. This is what was currently merged
>
> I don't think option 1 makes much sense as it'll harm ARM arch forever
> more, to cope with QEMU versions that will almost never be used in
> practice.
>
> I'd be inclined to go with option 2, and then if any PCI devices are
> actually used with the guest, check the capability at start time when
> we are doing auto-address assignment.
>
Another option: add versioned 'virt' machine types to the next qemu release
(like virt-2.5 etc.), and key libvirt enabling pci off of that.
_Eventually_ we are going to need versioned 'virt' machine types for migration
compatibility like we already do with x86 -M pc-*. Might as well make the
change early so libvirt can actually use it.
- Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list