qemu: Explicitly add/remove /dev/vfio/vfio to/from NS/CGroups

In near future, the decision what to do with /dev/vfio/vfio with
respect to domain namespace and CGroup is going to be moved out
of qemuDomainGetHostdevPath() because there will be some other
types of devices than hostdevs that need access to VFIO.

All functions that I'm changing (except qemuSetupHostdevCgroup())
assume that hostdev we are adding/removing to VM is not in the
definition yet (because of how qemuDomainNeedsVFIO() is written).
Fortunately, this assumption is true.

For qemuSetupHostdevCgroup(), the worst thing that may happen is
that we allow /dev/vfio/vfio which was already allowed.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 31cf71146b0dc2370b6a0d648dddead8a8ca9034..e451ebdb3afdc340cb474d10f39e515285099ea9 100644 (file)
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -25,6 +25,7 @@
 #include "qemu_domain.h"
 #include "qemu_process.h"
 #include "qemu_extdevice.h"
+#include "qemu_hostdev.h"
 #include "virlog.h"
 #include "viralloc.h"
 #include "virerror.h"
@@ -359,6 +360,16 @@ qemuTeardownInputCgroup(virDomainObjPtr vm,
 }
 
 
+/**
+ * qemuSetupHostdevCgroup:
+ * vm: domain object
+ * @dev: device to allow
+ *
+ * For given host device @dev allow access to in Cgroups.
+ *
+ * Returns: 0 on success,
+ *         -1 otherwise.
+ */
 int
 qemuSetupHostdevCgroup(virDomainObjPtr vm,
                        virDomainHostdevDefPtr dev)
@@ -385,6 +396,16 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
             goto cleanup;
     }
 
+    if (qemuHostdevNeedsVFIO(dev)) {
+        VIR_DEBUG("Cgroup allow %s perms=%d", QEMU_DEV_VFIO, VIR_CGROUP_DEVICE_RW);
+        rv = virCgroupAllowDevicePath(priv->cgroup, QEMU_DEV_VFIO,
+                                      VIR_CGROUP_DEVICE_RW, false);
+        virDomainAuditCgroupPath(vm, priv->cgroup, "allow",
+                                 QEMU_DEV_VFIO, "rw", rv);
+        if (rv < 0)
+            goto cleanup;
+    }
+
     ret = 0;
 
  cleanup:
@@ -395,9 +416,21 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
     return ret;
 }
 
+
+/**
+ * qemuTeardownHostdevCgroup:
+ * @vm: doamin object
+ * @dev: device to tear down
+ *
+ * For given host device @dev deny access to it in CGroups.
+ * Note, @dev must not be in @vm's definition.
+ *
+ * Returns: 0 on success,
+ *         -1 otherwise.
+ */
 int
 qemuTeardownHostdevCgroup(virDomainObjPtr vm,
-                       virDomainHostdevDefPtr dev)
+                          virDomainHostdevDefPtr dev)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
     char **path = NULL;
@@ -421,6 +454,17 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
             goto cleanup;
     }
 
+    if (qemuHostdevNeedsVFIO(dev) &&
+        !qemuDomainNeedsVFIO(vm->def)) {
+        VIR_DEBUG("Cgroup deny " QEMU_DEV_VFIO);
+        rv = virCgroupDenyDevicePath(priv->cgroup, QEMU_DEV_VFIO,
+                                     VIR_CGROUP_DEVICE_RWM, false);
+        virDomainAuditCgroupPath(vm, priv->cgroup, "deny",
+                                 QEMU_DEV_VFIO, "rwm", rv);
+        if (rv < 0)
+            goto cleanup;
+    }
+
     ret = 0;
  cleanup:
     for (i = 0; i < npaths; i++)
@@ -803,6 +847,8 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm)
         goto cleanup;
 
     for (i = 0; i < vm->def->nhostdevs; i++) {
+        /* This may allow /dev/vfio/vfio multiple times, but that
+         * is not a problem. Kernel will have only one record. */
         if (qemuSetupHostdevCgroup(vm, vm->def->hostdevs[i]) < 0)
             goto cleanup;
     }

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4b38da802e6c05da11b647b5e8c6c7bd35be2e41..88509ce9aee4984a77dcef3e7b1210ab933757dc 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -14503,6 +14503,10 @@ qemuDomainSetupHostdev(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED,
             goto cleanup;
     }
 
+    if (qemuHostdevNeedsVFIO(dev) &&
+        qemuDomainCreateDevice(QEMU_DEV_VFIO, data, false) < 0)
+        goto cleanup;
+
     ret = 0;
  cleanup:
     for (i = 0; i < npaths; i++)
@@ -15526,6 +15530,17 @@ qemuDomainNamespaceTeardownDisk(virDomainObjPtr vm G_GNUC_UNUSED,
 }
 
 
+/**
+ * qemuDomainNamespaceSetupHostdev:
+ * @vm: domain object
+ * @hostdev: hostdev to create in @vm's namespace
+ *
+ * For given @hostdev, create its devfs representation (if it has one) in
+ * domain namespace. Note, @hostdev must not be in @vm's definition.
+ *
+ * Returns: 0 on success,
+ *         -1 otherwise.
+ */
 int
 qemuDomainNamespaceSetupHostdev(virDomainObjPtr vm,
                                 virDomainHostdevDefPtr hostdev)
@@ -15540,6 +15555,11 @@ qemuDomainNamespaceSetupHostdev(virDomainObjPtr vm,
     if (qemuDomainNamespaceMknodPaths(vm, (const char **)paths, npaths) < 0)
         goto cleanup;
 
+    if (qemuHostdevNeedsVFIO(hostdev) &&
+        !qemuDomainNeedsVFIO(vm->def) &&
+        qemuDomainNamespaceMknodPath(vm, QEMU_DEV_VFIO) < 0)
+        goto cleanup;
+
     ret = 0;
  cleanup:
     for (i = 0; i < npaths; i++)
@@ -15549,6 +15569,17 @@ qemuDomainNamespaceSetupHostdev(virDomainObjPtr vm,
 }
 
 
+/**
+ * qemuDomainNamespaceTeardownHostdev:
+ * @vm: domain object
+ * @hostdev: hostdev to remove in @vm's namespace
+ *
+ * For given @hostdev, remove its devfs representation (if it has one) in
+ * domain namespace. Note, @hostdev must not be in @vm's definition.
+ *
+ * Returns: 0 on success,
+ *         -1 otherwise.
+ */
 int
 qemuDomainNamespaceTeardownHostdev(virDomainObjPtr vm,
                                    virDomainHostdevDefPtr hostdev)
@@ -15564,6 +15595,11 @@ qemuDomainNamespaceTeardownHostdev(virDomainObjPtr vm,
     if (qemuDomainNamespaceUnlinkPaths(vm, (const char **)paths, npaths) < 0)
         goto cleanup;
 
+    if (qemuHostdevNeedsVFIO(hostdev) &&
+        !qemuDomainNeedsVFIO(vm->def) &&
+        qemuDomainNamespaceUnlinkPath(vm, QEMU_DEV_VFIO) < 0)
+        goto cleanup;
+
     ret = 0;
  cleanup:
     for (i = 0; i < npaths; i++)