qemuDomainAttachDeviceMknodHelper: Create more files in a single go

So far, when attaching a device needs two or more /dev nodes
created into a domain, we fork off and run the helper for every
node separately. For majority of devices this is okay, because
they need no or one node created anyway. But the idea is to use
this attach code to build the namespace when starting a domain,
in which case there will be way more nodes than one.

To achieve this, the recursive approach for handling symlinks has
to be turned into an iterative one.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index 7b0469114820515505a2ae01e0e63e363c6f09fb..7e8f8d2f2ce4b14409a0b3ba553583180e73763f 100644 (file)
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -1098,26 +1098,58 @@ qemuDomainNamespaceAvailable(qemuDomainNamespace ns G_GNUC_UNUSED)
 }
 
 
-struct qemuNamespaceMkondData {
-    virQEMUDriverPtr driver;
-    virDomainObjPtr vm;
+typedef struct _qemuNamespaceMknodItem qemuNamespaceMknodItem;
+typedef qemuNamespaceMknodItem *qemuNamespaceMknodItemPtr;
+struct _qemuNamespaceMknodItem {
     const char *file;
     char *target;
+    bool bindmounted;
     GStatBuf sb;
     void *acl;
-#ifdef WITH_SELINUX
     char *tcon;
-#endif
 };
 
+typedef struct _qemuNamespaceMknodData qemuNamespaceMknodData;
+typedef qemuNamespaceMknodData *qemuNamespaceMknodDataPtr;
+struct _qemuNamespaceMknodData {
+    virQEMUDriverPtr driver;
+    virDomainObjPtr vm;
+    qemuNamespaceMknodItemPtr items;
+    size_t nitems;
+};
+
+
+static void
+qemuNamespaceMknodItemClear(qemuNamespaceMknodItemPtr item)
+{
+    VIR_FREE(item->target);
+    virFileFreeACLs(&item->acl);
+#ifdef WITH_SELINUX
+    freecon(item->tcon);
+#endif
+}
+
+
+static void
+qemuNamespaceMknodDataClear(qemuNamespaceMknodDataPtr data)
+{
+    size_t i;
+
+    for (i = 0; i < data->nitems; i++) {
+        qemuNamespaceMknodItemPtr item = &data->items[i];
+
+        qemuNamespaceMknodItemClear(item);
+    }
+
+    VIR_FREE(data->items);
+}
+
 
 /* Our way of creating devices is highly linux specific */
 #if defined(__linux__)
 static int
-qemuDomainAttachDeviceMknodHelper(pid_t pid G_GNUC_UNUSED,
-                                  void *opaque)
+qemuNamespaceMknodOne(qemuNamespaceMknodItemPtr data)
 {
-    struct qemuNamespaceMkondData *data = opaque;
     int ret = -1;
     bool delDevice = false;
     bool isLink = S_ISLNK(data->sb.st_mode);
@@ -1125,8 +1157,6 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid G_GNUC_UNUSED,
     bool isReg = S_ISREG(data->sb.st_mode) || S_ISFIFO(data->sb.st_mode) || S_ISSOCK(data->sb.st_mode);
     bool isDir = S_ISDIR(data->sb.st_mode);
 
-    qemuSecurityPostFork(data->driver->securityManager);
-
     if (virFileMakeParentPath(data->file) < 0) {
         virReportSystemError(errno,
                              _("Unable to create %s"), data->file);
@@ -1244,11 +1274,6 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid G_GNUC_UNUSED,
         else
             unlink(data->file);
     }
-# ifdef WITH_SELINUX
-    freecon(data->tcon);
-# endif
-    virFileFreeACLs(&data->acl);
-    VIR_FREE(data->target);
     return ret;
 }
 
@@ -1265,63 +1290,66 @@ qemuNamespaceMknodItemNeedsBindMount(mode_t st_mode)
 
 
 static int
-qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver,
-                                     virDomainObjPtr vm,
-                                     const char *file,
-                                     char * const *devMountsPath,
-                                     size_t ndevMountsPath,
-                                     unsigned int ttl)
+qemuNamespaceMknodHelper(pid_t pid G_GNUC_UNUSED,
+                         void *opaque)
 {
-    g_autoptr(virQEMUDriverConfig) cfg = NULL;
-    struct qemuNamespaceMkondData data;
+    qemuNamespaceMknodDataPtr data = opaque;
+    size_t i;
     int ret = -1;
-    g_autofree char *target = NULL;
-    bool isLink;
-    bool needsBindMount;
 
-    if (!ttl) {
-        virReportSystemError(ELOOP,
-                             _("Too many levels of symbolic links: %s"),
-                             file);
-        return ret;
+    qemuSecurityPostFork(data->driver->securityManager);
+
+    for (i = 0; i < data->nitems; i++) {
+        if (qemuNamespaceMknodOne(&data->items[i]) < 0)
+            goto cleanup;
     }
 
-    memset(&data, 0, sizeof(data));
+    ret = 0;
+ cleanup:
+    qemuNamespaceMknodDataClear(data);
+    return ret;
+}
 
-    data.driver = driver;
-    data.vm = vm;
-    data.file = file;
 
-    if (g_lstat(file, &data.sb) < 0) {
+static int
+qemuNamespaceMknodItemInit(qemuNamespaceMknodItemPtr item,
+                           virQEMUDriverConfigPtr cfg,
+                           virDomainObjPtr vm,
+                           const char *file)
+{
+    g_autofree char *target = NULL;
+    bool isLink;
+    bool needsBindMount;
+
+    item->file = file;
+
+    if (g_lstat(file, &item->sb) < 0) {
         virReportSystemError(errno,
                              _("Unable to access %s"), file);
-        return ret;
+        return -1;
     }
 
-    isLink = S_ISLNK(data.sb.st_mode);
-    needsBindMount = qemuNamespaceMknodItemNeedsBindMount(data.sb.st_mode);
+    isLink = S_ISLNK(item->sb.st_mode);
+    needsBindMount = qemuNamespaceMknodItemNeedsBindMount(item->sb.st_mode);
 
     if (needsBindMount && STRPREFIX(file, QEMU_DEVPREFIX)) {
-        cfg = virQEMUDriverGetConfig(driver);
         if (!(target = qemuDomainGetPreservedMountPath(cfg, vm, file)))
-            goto cleanup;
-
-        if (virFileBindMountDevice(file, target) < 0)
-            goto cleanup;
+            return -1;
 
-        data.target = target;
+        item->target = g_steal_pointer(&target);
     } else if (isLink) {
         g_autoptr(GError) gerr = NULL;
 
         if (!(target = g_file_read_link(file, &gerr))) {
             virReportError(VIR_ERR_SYSTEM_ERROR,
                            _("failed to resolve symlink %s: %s"), file, gerr->message);
-            return ret;
+            return -1;
         }
 
         if (!g_path_is_absolute(target)) {
             g_autofree char *fileTmp = g_strdup(file);
-            char *c = NULL, *tmp = NULL;
+            char *c = NULL;
+            char *tmp = NULL;
 
             if ((c = strrchr(fileTmp, '/')))
                 *(c + 1) = '\0';
@@ -1331,68 +1359,128 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver,
             target = g_steal_pointer(&tmp);
         }
 
-        data.target = target;
+        item->target = g_steal_pointer(&target);
     }
 
     /* Symlinks don't have ACLs. */
     if (!isLink &&
-        virFileGetACLs(file, &data.acl) < 0 &&
+        virFileGetACLs(file, &item->acl) < 0 &&
         errno != ENOTSUP) {
         virReportSystemError(errno,
                              _("Unable to get ACLs on %s"), file);
-        goto cleanup;
+        return -1;
     }
 
 # ifdef WITH_SELINUX
-    if (lgetfilecon_raw(file, &data.tcon) < 0 &&
+    if (lgetfilecon_raw(file, &item->tcon) < 0 &&
         (errno != ENOTSUP && errno != ENODATA)) {
         virReportSystemError(errno,
                              _("Unable to get SELinux label from %s"), file);
-        goto cleanup;
+        return -1;
     }
 # endif
 
-    if (STRPREFIX(file, QEMU_DEVPREFIX)) {
-        size_t i;
+    return 0;
+}
 
-        for (i = 0; i < ndevMountsPath; i++) {
-            if (STREQ(devMountsPath[i], "/dev"))
-                continue;
-            if (STRPREFIX(file, devMountsPath[i]))
-                break;
+
+static int
+qemuNamespaceAttachDeviceMknodOne(qemuNamespaceMknodDataPtr data,
+                                  virQEMUDriverConfigPtr cfg,
+                                  virDomainObjPtr vm,
+                                  const char *file,
+                                  char * const *devMountsPath,
+                                  size_t ndevMountsPath)
+{
+    long ttl = sysconf(_SC_SYMLOOP_MAX);
+    const char *next = file;
+    size_t i;
+
+    while (1) {
+        qemuNamespaceMknodItem item = { 0 };
+
+        if (qemuNamespaceMknodItemInit(&item, cfg, vm, next) < 0)
+            return -1;
+
+        if (STRPREFIX(next, QEMU_DEVPREFIX)) {
+            for (i = 0; i < ndevMountsPath; i++) {
+                if (STREQ(devMountsPath[i], "/dev"))
+                    continue;
+                if (STRPREFIX(next, devMountsPath[i]))
+                    break;
+            }
+
+            if (i == ndevMountsPath &&
+                VIR_APPEND_ELEMENT_COPY(data->items, data->nitems, item) < 0)
+                return -1;
         }
 
-        if (i == ndevMountsPath) {
-            if (qemuSecurityPreFork(driver->securityManager) < 0)
-                goto cleanup;
+        if (!S_ISLNK(item.sb.st_mode))
+            break;
 
-            if (virProcessRunInMountNamespace(vm->pid,
-                                              qemuDomainAttachDeviceMknodHelper,
-                                              &data) < 0) {
-                qemuSecurityPostFork(driver->securityManager);
+        if (ttl-- == 0) {
+            virReportSystemError(ELOOP,
+                                 _("Too many levels of symbolic links: %s"),
+                                 next);
+            return -1;
+        }
+
+        next = item.target;
+    }
+
+    return 0;
+}
+
+
+static int
+qemuDomainAttachDeviceMknod(virQEMUDriverPtr driver,
+                            virDomainObjPtr vm,
+                            const char *file,
+                            char * const *devMountsPath,
+                            size_t ndevMountsPath)
+{
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+    qemuNamespaceMknodData data = { 0 };
+    size_t i;
+    int ret = -1;
+
+    data.driver = driver;
+    data.vm = vm;
+
+    if (qemuNamespaceAttachDeviceMknodOne(&data, cfg, vm, file,
+                                          devMountsPath, ndevMountsPath) < 0)
+        return -1;
+
+    for (i = 0; i < data.nitems; i++) {
+        qemuNamespaceMknodItemPtr item = &data.items[i];
+        if (item->target &&
+            qemuNamespaceMknodItemNeedsBindMount(item->sb.st_mode)) {
+            if (virFileBindMountDevice(item->file, item->target) < 0)
                 goto cleanup;
-            }
-            qemuSecurityPostFork(driver->securityManager);
-        } else {
-            VIR_DEBUG("Skipping dev %s because of %s mount point",
-                      file, devMountsPath[i]);
+            item->bindmounted = true;
         }
     }
 
-    if (isLink &&
-        qemuDomainAttachDeviceMknodRecursive(driver, vm, target,
-                                             devMountsPath, ndevMountsPath,
-                                             ttl -1) < 0)
+    if (qemuSecurityPreFork(driver->securityManager) < 0)
+        goto cleanup;
+
+    if (virProcessRunInMountNamespace(vm->pid,
+                                      qemuNamespaceMknodHelper,
+                                      &data) < 0) {
+        qemuSecurityPostFork(driver->securityManager);
         goto cleanup;
+    }
+    qemuSecurityPostFork(driver->securityManager);
 
     ret = 0;
  cleanup:
-# ifdef WITH_SELINUX
-    freecon(data.tcon);
-# endif
-    virFileFreeACLs(&data.acl);
-    if (needsBindMount && target)
-        umount(target);
+    for (i = 0; i < data.nitems; i++) {
+        if (data.items[i].bindmounted &&
+            umount(data.items[i].target) < 0) {
+            VIR_WARN("Unable to unmount %s", data.items[i].target);
+        }
+    }
+    qemuNamespaceMknodDataClear(&data);
     return ret;
 }
 
@@ -1401,12 +1489,11 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver,
 
 
 static int
-qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver G_GNUC_UNUSED,
-                                     virDomainObjPtr vm G_GNUC_UNUSED,
-                                     const char *file G_GNUC_UNUSED,
-                                     char * const *devMountsPath G_GNUC_UNUSED,
-                                     size_t ndevMountsPath G_GNUC_UNUSED,
-                                     unsigned int ttl G_GNUC_UNUSED)
+qemuDomainAttachDeviceMknod(virQEMUDriverPtr driver G_GNUC_UNUSED,
+                            virDomainObjPtr vm G_GNUC_UNUSED,
+                            const char *file G_GNUC_UNUSED,
+                            char * const *devMountsPath G_GNUC_UNUSED,
+                            size_t ndevMountsPath G_GNUC_UNUSED)
 {
     virReportSystemError(ENOSYS, "%s",
                          _("Namespaces are not supported on this platform."));
@@ -1417,21 +1504,6 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver G_GNUC_UNUSED,
 #endif /* !defined(__linux__) */
 
 
-static int
-qemuDomainAttachDeviceMknod(virQEMUDriverPtr driver,
-                            virDomainObjPtr vm,
-                            const char *file,
-                            char * const *devMountsPath,
-                            size_t ndevMountsPath)
-{
-    long symloop_max = sysconf(_SC_SYMLOOP_MAX);
-
-    return qemuDomainAttachDeviceMknodRecursive(driver, vm, file,
-                                                devMountsPath, ndevMountsPath,
-                                                symloop_max);
-}
-
-
 static int
 qemuDomainDetachDeviceUnlinkHelper(pid_t pid G_GNUC_UNUSED,
                                    void *opaque)