[libvirt] [PATCH v2 3/7] util/virhostdev: enhance VFIO device is in use detection

[Michal Privoznik]

On 7/25/19 8:09 PM, Daniel Henrique Barboza wrote:
> The current virHostdevPreparePCIDevices code fails to detect an unmanaged
> VFIO device that is in the activePCIHostdevs, and active in the same
> domain name as dom_name, as a device in use. Considering a call to this
> function, when activePCIHostdevs has a VFIO deviceA in the list, and
> deviceA is active in domain 'dom_name', this is what happens:
> 
> - At step 1, the code goes to the 'if (usesVFIO)' block. All devices in
> the same IOMMU group of deviceA are used as argument of
> virHostdevIsPCINodeDeviceUsed, via the IOMMUGroupIterate function;
> 
> - Inside virHostdevIsPCINodeDeviceUsed, an 'usesVFIO' verification is
> made, following to an 'iommu_owner' jump that sets ret=0. This will
> happen to all devices of the IOMMU group that belongs to the same domain
> as dom_name, including deviceA;
> 
> - Step 2 starts, thinking that all devices inside hostdevs are available,
> which is not the case.
> 
> This error was detected when changing tests/virhostdev.c to use
> vfio-pci instead of pci-stub (a change that will follow this one).
> The side effect observed was a test failure in
> testVirHostdevPreparePCIHostdevs_unmanaged, result of the behavior
> mentioned above:
> 
> Unexpected count of items in mgr->inactivePCIHostdevs: 1, expecting 0
> 
> This patch fixes virHostdevIsPCINodeDeviceUsed to consider the case of
> a VFIO device that is already active in the domain. First, check if
> the device is in the activePCIHostdev list and bail immediately if
> true. Otherwise, in case of VFIO, check for IOMMU group ownership
> of the domain. This is done by a new callback function for
> IOMMUGroupIterate. If the VFIO device isn't in the active list and
> its domain has ownership of the IOMMU, then it is unused.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
> ---
>   src/util/virhostdev.c | 74 +++++++++++++++++++++++++++++++++----------
>   1 file changed, 57 insertions(+), 17 deletions(-)

As you pointed out, I've sent a patch for the same issue. And if you 
don't mind, I'd like to go with mine, because it's shorter and simpler.

Michal