Re: [libvirt] [PATCH 6/7] qemuDomainGetHostdevPath: Report /dev/vfio/vfio less frequently

So far, qemuDomainGetHostdevPath has no knowledge of the reasong

it is called and thus reports /dev/vfio/vfio for every VFIO backed device. This is suboptimal, as we want it to: a) report /dev/vfio/vfio on every addition or domain startup b) report /dev/vfio/vfio only on last VFIO device being unplugged If a domain is being stopped then namespace and CGroup die with it so no need to worry about that. I mean, even when a domain that's exiting has more than one VFIO devices assigned to it, this function does not clean /dev/vfio/vfio in CGroup nor in the namespace. But that doesn't matter. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt;

--- src/qemu/qemu_cgroup.c | 87 ++++++++++++-------------------------------------- src/qemu/qemu_domain.c | 38 ++++++++++++++++------ src/qemu/qemu_domain.h | 4 ++- 3 files changed, 52 insertions(+), 77 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 944e8dc87..209cbc275 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -52,7 +52,6 @@ const char *const defaultDeviceACL[] = { #define DEVICE_PTY_MAJOR 136 #define DEVICE_SND_MAJOR 116 -#define DEV_VFIO "/dev/vfio/vfio" static int qemuSetupImagePathCgroup(virDomainObjPtr vm, @@ -271,7 +270,7 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm, size_t i, npaths = 0; int rv, ret = -1; - if (qemuDomainGetHostdevPath(dev, &npaths, &path, &perms) < 0) + if (qemuDomainGetHostdevPath(NULL, dev, false, &npaths, &path, &perms) < 0) goto cleanup; for (i = 0; i < npaths; i++) { @@ -298,11 +297,10 @@ int qemuTeardownHostdevCgroup(virDomainObjPtr vm, virDomainHostdevDefPtr dev) { - int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; - virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; - virPCIDevicePtr pci = NULL; - char *path = NULL; + char **path = NULL; + size_t i, npaths = 0; + int rv, ret = -1; /* currently this only does something for PCI devices using vfio * for device assignment, but it is called for *all* hostdev @@ -312,70 +310,27 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm, if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) return 0; - if (dev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { - - switch ((virDomainHostdevSubsysType) dev->source.subsys.type) { - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: - if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { - int rv; - size_t i, vfios = 0; - - pci = virPCIDeviceNew(pcisrc->addr.domain, - pcisrc->addr.bus, - pcisrc->addr.slot, - pcisrc->addr.function); - if (!pci) - goto cleanup; - - if (!(path = virPCIDeviceGetIOMMUGroupDev(pci))) - goto cleanup; - - VIR_DEBUG("Cgroup deny %s for PCI device assignment", path); - rv = virCgroupDenyDevicePath(priv->cgroup, path, - VIR_CGROUP_DEVICE_RWM, false); - virDomainAuditCgroupPath(vm, priv->cgroup, - "deny", path, "rwm", rv == 0); - if (rv < 0) - goto cleanup; - - /* If this is the last hostdev with VFIO backend deny - * /dev/vfio/vfio too. */ - for (i = 0; i < vm->def->nhostdevs; i++) { - virDomainHostdevDefPtr tmp = vm->def->hostdevs[i]; - if (tmp->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - tmp->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - tmp->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) - vfios++; - } - - if (vfios == 0) { - VIR_DEBUG("Cgroup deny " DEV_VFIO " for PCI device assignment"); - rv = virCgroupDenyDevicePath(priv->cgroup, DEV_VFIO, - VIR_CGROUP_DEVICE_RWM, false); - virDomainAuditCgroupPath(vm, priv->cgroup, - "deny", DEV_VFIO, "rwm", rv == 0); - if (rv < 0) - goto cleanup; - } - } - break; - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: - /* nothing to tear down for USB */ - break; - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: - /* nothing to tear down for SCSI */ - break; - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: - /* nothing to tear down for scsi_host */ - break; - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: - break; - } + if (dev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + dev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && + dev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO && + qemuDomainGetHostdevPath(vm->def, dev, true, + &npaths, &path, NULL) < 0) + goto cleanup; + + for (i = 0; i < npaths; i++) { + VIR_DEBUG("Cgroup deny %s", path[i]); + rv = virCgroupDenyDevicePath(priv->cgroup, path[i], + VIR_CGROUP_DEVICE_RWM, false); + virDomainAuditCgroupPath(vm, priv->cgroup, + "deny", path[i], "rwm", rv == 0); + if (rv < 0) + goto cleanup; } ret = 0; cleanup: - virPCIDeviceFree(pci); + for (i = 0; i < npaths; i++) + VIR_FREE(path[i]); VIR_FREE(path); return ret; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 530eced33..515e0052e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6834,7 +6834,9 @@ qemuDomainSupportsVideoVga(virDomainVideoDefPtr video, /** * qemuDomainGetHostdevPath: + * @def: domain definition * @dev: host device definition + * @teardown: true if device will be removed * @npaths: number of items in @path and @perms arrays * @path: resulting path to @dev * @perms: Optional pointer to VIR_CGROUP_DEVICE_* perms @@ -6849,7 +6851,9 @@ qemuDomainSupportsVideoVga(virDomainVideoDefPtr video, * Returns 0 on success, -1 otherwise. */ int -qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev, +qemuDomainGetHostdevPath(virDomainDefPtr def, + virDomainHostdevDefPtr dev, + bool teardown, size_t *npaths, char ***path, int **perms) @@ -6890,7 +6894,21 @@ qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev, freeTmpPath = true; perm = VIR_CGROUP_DEVICE_RW; - includeVFIO = true; + if (teardown) { + size_t nvfios = 0; + for (i = 0; i < def->nhostdevs; i++) { + virDomainHostdevDefPtr tmp = def->hostdevs[i]; + if (tmp->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + tmp->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && + tmp->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) + nvfios++; + } + + if (nvfios == 0) + includeVFIO = true; + } else { + includeVFIO = true; + } } break; @@ -7389,7 +7407,7 @@ qemuDomainSetupHostdev(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, char **path = NULL; size_t i, npaths = 0; - if (qemuDomainGetHostdevPath(dev, &npaths, &path, NULL) < 0) + if (qemuDomainGetHostdevPath(NULL, dev, false, &npaths, &path, NULL) < 0) goto cleanup; for (i = 0; i < npaths; i++) { @@ -8024,7 +8042,7 @@ qemuDomainNamespaceSetupHostdev(virQEMUDriverPtr driver, if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) return 0; - if (qemuDomainGetHostdevPath(hostdev, &npaths, &path, NULL) < 0) + if (qemuDomainGetHostdevPath(NULL, hostdev, false, &npaths, &path, NULL) < 0) goto cleanup; for (i = 0; i < npaths; i++) { @@ -8055,14 +8073,14 @@ qemuDomainNamespaceTeardownHostdev(virQEMUDriverPtr driver, if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) return 0; - if (qemuDomainGetHostdevPath(hostdev, &npaths, &path, NULL) < 0) + if (qemuDomainGetHostdevPath(vm->def, hostdev, true, + &npaths, &path, NULL) < 0) goto cleanup; - /* Don't remove other paths than for the @hostdev itself. - * They might be still in use by other devices. */ - if (npaths > 0 && - qemuDomainDetachDeviceUnlink(driver, vm, path[0]) < 0) - goto cleanup; + for (i = 0; i < npaths; i++) { + if (qemuDomainDetachDeviceUnlink(driver, vm, path[i]) < 0) + goto cleanup; + } ret = 0; cleanup: diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index e64aa25ba..80de50fbe 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -802,7 +802,9 @@ int qemuDomainCheckMonitor(virQEMUDriverPtr driver, bool qemuDomainSupportsVideoVga(virDomainVideoDefPtr video, virQEMUCapsPtr qemuCaps); -int qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev, +int qemuDomainGetHostdevPath(virDomainDefPtr def, + virDomainHostdevDefPtr dev, + bool teardown, size_t *npaths, char ***path, int **perms); -- 2.11.0 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list