[libvirt] [PATCH v2 1/4] domain_conf: allow address type='none' to unassign PCI hostdevs

[Daniel P. Berrangé]

On Thu, Nov 21, 2019 at 07:19:14PM -0300, Daniel Henrique Barboza wrote:
> Today, to use a PCI hostdev "A" in a domain, all PCI devices
> that belongs to the same IOMMU group must also be declared in
> the domain XML, meaning that all IOMMU devices are detached
> from the host and all of them are visible to the guest.
> 
> The result is that the guest will have access to all devices,
> but this is not necessarily what the administrator wanted. If only
> the hostdev "A" was intended for guest usage, but hostdevs "B" and
> "C" happened to be in the same IOMMU group of "A", the guest will
> gain access to all 3 devices. This makes the administrator rely
> on alternative solutions, such as use all hostdevs with un-managed
> mode and detached all the IOMMU before the guest starts. If
> use un-managed mode is not an option, the only alternative left is
> an ACS patch to deny guest access to "B" and "C".
> 
> Discussions made in [1] and [2] led to the approach implemented
> here. This patch builds upon the already existing
> VIR_DOMAIN_DEF_FORMAT_ADDRESS_NONE format to use it as a indication
> of whether a hostdev will be owned by a domain, but not visible
> to the guest OS. This allows the administrator to declare all the
> IOMMU while also choosing which hostdevs will be usable by the
> guest. This new mechanic applies to all PCI hostdevs, regardless
> of whether they are a PCI multifunction hostdev or not. Using
> <address type='none'/> in any case other than a PCI hostdev
> will result in error.
> 
> The use of a new 'unassigned' flag in virDomainHostdevDef makes
> it easier to distinguish between the case we're handling here
> versus the case in which a PCI hostdev has no <address> element.
> Both are interpreted as VIR_DOMAIN_DEF_FORMAT_ADDRESS_NONE in
> the existing code, thus reducing the logic to a flag makes
> it easier to handle this new use case. In the next patch we'll
> use the 'unassigned' flag to filter the hostdevs being
> initialized in the QEMU command line.

This paragraph illustrates why I think this approach is a bad
idea. It is overloading ADDRESS_NONE to have 2 separate meanings.
When we've done such things in the past we've usually ended up
regretting it.

We're using ADDRESS_NONE as our signal that the user does not
care about the address and thus libvirt should automatically
assign one. That's consistent across all the different types
of devices that we have.

I don't want to see us add special semantics for this just
for host devices.

The fact that you stil lneeded a "bool unassigned" flag to
be stored in the virDomainHostdevDef is a clear sign that
this must be exposed as a distinct XML attribute.

I see you in fact did exactly this in the previous version of
this patch series, so I'd like to go back to that version.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|