[libvirt] [PATCH v2] qemu: fix command line building for iommu devices

[Daniel P. Berrange]

On Mon, Oct 03, 2016 at 05:18:40PM +0200, Ján Tomko wrote:
> On Mon, Oct 03, 2016 at 02:37:57PM +0100, Daniel P. Berrange wrote:
> > The intel-iommu device has existed since QEMU 2.2.0, but
> > it was only possible to create it with -device since
> > QEMU 2.7.0, thanks to:
> > 
> >  commit 621d983a1f9051f4cfc3f402569b46b77d8449fc
> >  Author: Marcel Apfelbaum <marcel at redhat.com>
> >  Date:   Mon Jun 27 18:38:34 2016 +0300
> > 
> >    hw/iommu: enable iommu with -device
> > 
> >    Use the standard '-device intel-iommu' to create the IOMMU device.
> >    The legacy '-machine,iommu=on' can still be used.
> > 
> > The libvirt capability check & command line formatting code
> > is thus broken for all QEMU versions 2.2.0 -> 2.6.0 inclusive.
> > 
> > This fixes it to use iommu=on instead.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> > ---
> > src/qemu/qemu_capabilities.c                       | 13 ++++++++
> > src/qemu/qemu_capabilities.h                       |  3 ++
> > src/qemu/qemu_command.c                            | 22 +++++++++++++
> > tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml   |  2 +-
> > tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml   |  2 +-
> > tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   |  2 +-
> > tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   |  2 +-
> > .../qemuxml2argv-intel-iommu-machine.args          | 21 ++++++++++++
> > .../qemuxml2argv-intel-iommu-machine.xml           | 37 ++++++++++++++++++++++
> > tests/qemuxml2argvtest.c                           |  2 ++
> > 10 files changed, 102 insertions(+), 4 deletions(-)
> > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.args
> > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-intel-iommu-machine.xml
> > 
> 
> This patch won't apply for me:
> Applying: qemu: fix command line building for iommu devices
> error: src/qemu/qemu_capabilities.c: does not match index
> error: patch failed: src/qemu/qemu_capabilities.h:376
> error: src/qemu/qemu_capabilities.h: patch does not apply
> error: patch failed:
> tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml:190
> error: tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml: patch does not
> apply
> Patch failed at 0001 qemu: fix command line building for iommu devices

Opps, I based it on the wrong branch - libvirt 2.2.0 instead of master

> 
> > @@ -3633,6 +3635,17 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
> >     if (virQEMUCapsProbeQMPMigrationCapabilities(qemuCaps, mon) < 0)
> >         goto cleanup;
> > 
> > +    /* 'intel-iommu' shows up as a device since 2.2.0, but can
> > +     * not be used with -device until 2.7.0. Before that it
> > +     * requires -machine iommu=on. So we must clear the device
> > +     * capability we detected on older QEMUs
> > +     */
> > +    if (qemuCaps->version < 2007000 &&
> > +        virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU)) {
> > +        virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU);
> > +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU);
> 
> Works for upstream QEMU, but not for distros that have backported the
> fix.

That's only a problem for distros that backported the patch
that *removes* the iommu attribute against -machine. If distros
backport code that removes features, then its their responsibility
to fix their downstream libvirt too.

> > diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
> > index 7a54040..b8b79ab 100644
> > --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
> > +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
> > @@ -190,10 +190,10 @@
> >   <flag name='drive-detect-zeroes'/>
> >   <flag name='tls-creds-x509'/>
> >   <flag name='display'/>
> > -  <flag name='intel-iommu'/>
> 
> Strange. For 2.7.0 we should have kept the original capability,
> according to the code above.

Opps, yes, that's not right.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|