On Thu, Jul 11, 2019 at 17:54:10 +0200, Michal Privoznik wrote: > Couple of places in the QEMU driver will want to know what paths > are associated with NVMe disks (for instance CGroup code or > namespaces code). Introduce helpers which return desired paths > (for instance /dev/vfio/vfio and /dev/vfio/N). > > Signed-off-by: Michal Privoznik <[email protected]> > --- > src/qemu/qemu_domain.c | 44 ++++++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_domain.h | 6 ++++++ > 2 files changed, 50 insertions(+) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 2a7f09ce24..949bbace88 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -11723,6 +11723,50 @@ qemuDomainSupportsVideoVga(virDomainVideoDefPtr > video, > } > > > +char * > +qemuDomainGetNVMeDiskPath(const virStorageSourceNVMeDef *nvme)
Name of this function is VERY misleading. It returns path of the IOMMU
group associated with the host portion of the NVMe device, but the name
implies smething completely different.
Include IOMMUGroup in the name and add a comment.
> +{
> + VIR_AUTOPTR(virPCIDevice) pci = NULL;
> +
> + /* All NVMe devices are VFIO PCI devices */
> + if (!(pci = virPCIDeviceNew(nvme->pciAddr.domain,
> + nvme->pciAddr.bus,
> + nvme->pciAddr.slot,
> + nvme->pciAddr.function)))
> + return NULL;
> +
> + return virPCIDeviceGetIOMMUGroupDev(pci);
> +}
> +
> +
> +char **
> +qemuDomainGetDiskNVMePaths(const virDomainDef *def,
Also this name looks troubling.
> + const virStorageSource *src,
> + bool teardown)
> +{
> + VIR_AUTOFREE(char *) iommuGroup = NULL;
> + VIR_AUTOSTRINGLIST paths = NULL;
> + bool includeVFIO = !teardown;
> +
> + if (!(iommuGroup = qemuDomainGetNVMeDiskPath(src->nvme)))
> + return NULL;
> +
> + if (virStringListAdd(&paths, iommuGroup) < 0)
> + return NULL;
> +
> + if (teardown && def &&
> + !virDomainDefHasNVMeDisk(def) &&
> + !virDomainDefHasVFIOHostdev(def))
> + includeVFIO = true;
> +
> + if (includeVFIO &&
> + virStringListAdd(&paths, QEMU_DEV_VFIO) < 0)
> + return NULL;
I don't like this. It's hiding the stuff necessary to detach VFIO groups
in random function and the hostdev code will require exactly the same
treatment. Additionally any further possible VFIO based device would
require 3 places.
Can't we consolidate that somehow?
signature.asc
Description: PGP signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
