On Fri, Feb 10, 2017 at 6:57 PM Michal Privoznik <mprivozn@xxxxxxxxxx> wrote:
So far, qemuDomainGetHostdevPath has no knowledge of the reasong
reasong/reason
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@xxxxxxxxxx>
---
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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
--
Marc-André Lureau
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
