Bug 1729675

*** Description of problem:
SEV requires negotiating VIRTIO_F_IOMMU_PLATFORM, between the virtio
device and the guest driver. For that to work, the device has to offer
the feature. In the libvirt domain XML, this is achieved by adding

      <driver iommu='on'/>

to the individual virtio device elements. (Documented at
<https://libvirt.org/formatdomain.html#elementsVirtio>.)

When this is done, "virsh edit" exits with the following error:

> error: XML document failed to validate against schema: Unable to
> validate doc against /usr/share/libvirt/schemas/domain.rng
> Extra element devices in interleave
> Element domain failed to validate content


*** Version-Release number of selected component (if applicable):
libvirt-4.5.0-30.module+el8.1.0+3574+3a63752b
(Brew build 924456)


*** How reproducible:
always


*** Steps to Reproduce:
1. invoke "virsh edit" on a suitable domain
2. add <driver iommu='on'/>
3. save and exit


*** Actual results:
See error message under "Description".

(Note: the error can be ignored with the "i" action, and the domain
launches correctly afterwards.)


*** Expected results:
No error message should be printed; the domain XML should be saved
without an interactive override.


*** Additional info:

(1) This issue occurs because

  /usr/share/libvirt/schemas/domaincommon.rng

contains:

  <define name="virtioOptions">
    <optional>
      <attribute name="iommu">
        <ref name="virOnOff"/>
      </attribute>
      <attribute name="ats">
        <ref name="virOnOff"/>
      </attribute>
    </optional>
  </define>

meaning that "iommu" and "ats" can only be specified together.

This is an invalid requirement, since "iommu" controls the
VIRTIO_F_IOMMU_PLATFORM virtio feature, while "ats" controls the PCI
Express ATS (address translation services) extended capability.

It is valid for the virtio device to advertise VIRTIO_F_IOMMU_PLATFORM
(~DMA API support), independently of the actual IOMMU implementation,
and *not* to offer ATS. Refer to:

- QEMU commit 615c4ed20598 ("virtio-pci: address space translation
  service (ATS) support", 2017-01-10)

- https://libvirt.org/formatdomain.html#elementsIommu

The use case described there (IOTLB / IOMMU translation caching, vhost,
vIOMMU) does not apply to SEV. SEV needs VIRTIO_F_IOMMU_PLATFORM, but
not ATS.


(2) The issue has been fixed in upstream libvirt v5.0.0 already, so the
ask is effectively to backport commit 87b4e1cd7e7e ("docs: schemas:
Decouple the virtio options from each other", 2019-01-04).


(3) In general, it is quite difficult to determine where exactly
the bug is, in the libvirt Relax-NG schema, whenever "virsh edit"
exits with a validation failure.

The "virt-xml-validate" tool isn't of much help either:

$ virsh dumpxml --inactive DOMAIN >DOMAIN.xml
$ virt-xml-validate DOMAIN.xml

> Relax-NG validity error : Extra element devices in interleave
> DOMAIN.xml:27: element devices: Relax-NG validity error : Element
> domain failed to validate content
> DOMAIN.xml fails to validate

This kind of error has frequently been reported in Bugzilla (I've now
read a few examples from the past), and it is never explained *how* the
issue was analyzed -- only the fix is posted every time.

The article at <https://wiki.libvirt.org/page/Common_XML_errors> is
silent on "Extra element FOOBAR in interleave" as well.

For the record, here's how I tracked down the problem:

- Install the "rnv" package (available on Fedora only; although it can
  be built from source elsewhere). RNV stands for "Relax NG Validator";
  the home page is at <http://www.davidashen.net/rnv.html>.

- Install the "trang" package (built from the "jing-trang" SRPM). Also
  exclusive to Fedora, in package format.

- Copy all the *.rng files from "/usr/share/libvirt/schemas/" to a
  temporary directory.

- In that directory, convert the RNG schema to "compact syntax",
  starting with the top-level <domain> element:

  $ trang -I rng -O rnc domain.rng domain.rnc

- Validate the domain XML dumped earlier against the "compact" schema:

  $ rnv domain.rnc DOMAIN.xml

- The output was (in the present case):

> DOMAIN.xml
> DOMAIN.xml:80:6: error: missing attributes of ^driver
> required:
>         attribute ^ats
> allowed:
>         attribute ^queues
>         attribute ^cmd_per_lun
>         attribute ^max_sectors
>         attribute ^ioeventfd
>         attribute ^iothread

  Based on the line number logged, I could locate the related element:

      <driver iommu='on'/>

It would be *really* nice if "virsh edit" could do this at once, when it
reports a schema validation error. Anyway, that's a separate (upstream)
feature request.

Thanks!