On Mon, Aug 10, 2015 at 07:23:07PM -0400, Cole Robinson wrote:
> 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.
I've raised the idea of versioned 'virt' machine type several times on
qemu-devel and its been clearly rejected saying they're not willing to
guarantee migration compatibility between releases on arm yet.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list