[PATCH 2/4] virDevMapperGetTargets: Use a linked list as return type

Peter Krempa pkrempa at redhat.com
Mon Sep 20 14:00:44 UTC 2021


Of the two callers one simply iterates over the returned paths and the
second one appends the returned paths to another linked list. Simplify
all of this by directly returning a linked list.

Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
 src/qemu/qemu_cgroup.c    | 11 ++++++-----
 src/qemu/qemu_namespace.c |  9 +++------
 src/util/virdevmapper.c   | 38 +++++++++++++++++---------------------
 src/util/virdevmapper.h   |  2 +-
 tests/qemuhotplugmock.c   | 10 ++++------
 5 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 6d4a82b3cd..471cbc3b8f 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -38,6 +38,7 @@
 #include "virnuma.h"
 #include "virdevmapper.h"
 #include "virutil.h"
+#include "virglibutil.h"

 #define VIR_FROM_THIS VIR_FROM_QEMU

@@ -60,8 +61,8 @@ qemuSetupImagePathCgroup(virDomainObj *vm,
 {
     qemuDomainObjPrivate *priv = vm->privateData;
     int perms = VIR_CGROUP_DEVICE_READ;
-    g_auto(GStrv) targetPaths = NULL;
-    size_t i;
+    g_autoptr(virGSListString) targetPaths = NULL;
+    GSList *n;
     int rv;

     if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
@@ -94,10 +95,10 @@ qemuSetupImagePathCgroup(virDomainObj *vm,
         return -1;
     }

-    for (i = 0; targetPaths && targetPaths[i]; i++) {
-        rv = virCgroupAllowDevicePath(priv->cgroup, targetPaths[i], perms, false);
+    for (n = targetPaths; n; n = n->next) {
+        rv = virCgroupAllowDevicePath(priv->cgroup, n->data, perms, false);

-        virDomainAuditCgroupPath(vm, priv->cgroup, "allow", targetPaths[i],
+        virDomainAuditCgroupPath(vm, priv->cgroup, "allow", n->data,
                                  virCgroupGetDevicePermsString(perms),
                                  rv);
         if (rv < 0)
diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index 728d77fc61..f1aaca86b1 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -251,8 +251,7 @@ qemuDomainSetupDisk(virStorageSource *src,
             if (!(tmpPath = virPCIDeviceAddressGetIOMMUGroupDev(&next->nvme->pciAddr)))
                 return -1;
         } else {
-            g_auto(GStrv) targetPaths = NULL;
-            GStrv tmp;
+            GSList *targetPaths;

             if (virStorageSourceIsEmpty(next) ||
                 !virStorageSourceIsLocalStorage(next)) {
@@ -270,10 +269,8 @@ qemuDomainSetupDisk(virStorageSource *src,
                 return -1;
             }

-            if (targetPaths) {
-                for (tmp = targetPaths; *tmp; tmp++)
-                    *paths = g_slist_prepend(*paths, g_steal_pointer(tmp));
-            }
+            if (targetPaths)
+                *paths = g_slist_concat(g_slist_reverse(targetPaths), *paths);
         }

         *paths = g_slist_prepend(*paths, g_steal_pointer(&tmpPath));
diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
index 301c8f3ba7..e42324fd19 100644
--- a/src/util/virdevmapper.c
+++ b/src/util/virdevmapper.c
@@ -36,6 +36,7 @@
 # include "virstring.h"
 # include "virfile.h"
 # include "virlog.h"
+# include "virglibutil.h"

 # define VIR_FROM_THIS VIR_FROM_STORAGE

@@ -217,18 +218,16 @@ virDMSanitizepath(const char *path)
 static int
 virDevMapperGetTargetsImpl(int controlFD,
                            const char *path,
-                           char ***devPaths_ret,
+                           GSList **devPaths,
                            unsigned int ttl)
 {
     g_autofree char *sanitizedPath = NULL;
     g_autofree char *buf = NULL;
     struct dm_ioctl dm;
     struct dm_target_deps *deps = NULL;
-    g_auto(GStrv) devPaths = NULL;
     size_t i;

     memset(&dm, 0, sizeof(dm));
-    *devPaths_ret = NULL;

     if (ttl == 0) {
         errno = ELOOP;
@@ -258,24 +257,17 @@ virDevMapperGetTargetsImpl(int controlFD,
         return -1;
     }

-    devPaths = g_new0(char *, deps->count + 1);
     for (i = 0; i < deps->count; i++) {
-        devPaths[i] = g_strdup_printf("/dev/block/%u:%u",
-                                      major(deps->dev[i]),
-                                      minor(deps->dev[i]));
-    }
-
-    for (i = 0; i < deps->count; i++) {
-        g_auto(GStrv) tmpPaths = NULL;
+        char *curpath = g_strdup_printf("/dev/block/%u:%u",
+                                        major(deps->dev[i]),
+                                        minor(deps->dev[i]));

-        if (virDevMapperGetTargetsImpl(controlFD, devPaths[i], &tmpPaths, ttl - 1) < 0)
-            return -1;
+        *devPaths = g_slist_prepend(*devPaths, curpath);

-        if (virStringListMerge(&devPaths, &tmpPaths) < 0)
+        if (virDevMapperGetTargetsImpl(controlFD, curpath, devPaths, ttl - 1) < 0)
             return -1;
     }

-    *devPaths_ret = g_steal_pointer(&devPaths);
     return 0;
 }

@@ -283,11 +275,10 @@ virDevMapperGetTargetsImpl(int controlFD,
 /**
  * virDevMapperGetTargets:
  * @path: devmapper target
- * @devPaths: returned string list of devices
+ * @devPaths: filled in by a GSList containing the paths
  *
  * For given @path figure out its targets, and store them in
- * @devPaths array. Note, @devPaths is a string list so it's NULL
- * terminated.
+ * @devPaths.
  *
  * If @path is not a devmapper device, @devPaths is set to NULL and
  * success is returned.
@@ -301,10 +292,11 @@ virDevMapperGetTargetsImpl(int controlFD,
  */
 int
 virDevMapperGetTargets(const char *path,
-                       char ***devPaths)
+                       GSList **devPaths)
 {
     VIR_AUTOCLOSE controlFD = -1;
     const unsigned int ttl = 32;
+    g_autoptr(virGSListString) paths = NULL;

     /* Arbitrary limit on recursion level. A devmapper target can
      * consist of devices or yet another targets. If that's the
@@ -321,7 +313,11 @@ virDevMapperGetTargets(const char *path,
         return -1;
     }

-    return virDevMapperGetTargetsImpl(controlFD, path, devPaths, ttl);
+    if (virDevMapperGetTargetsImpl(controlFD, path, &paths, ttl) < 0)
+        return -1;
+
+    *devPaths = g_slist_reverse(g_steal_pointer(&paths));
+    return 0;
 }


@@ -346,7 +342,7 @@ virIsDevMapperDevice(const char *dev_name)

 int
 virDevMapperGetTargets(const char *path G_GNUC_UNUSED,
-                       char ***devPaths G_GNUC_UNUSED)
+                       GSlist **devPaths G_GNUC_UNUSED)
 {
     errno = ENOSYS;
     return -1;
diff --git a/src/util/virdevmapper.h b/src/util/virdevmapper.h
index 834900692e..4d8d3ccdb8 100644
--- a/src/util/virdevmapper.h
+++ b/src/util/virdevmapper.h
@@ -24,7 +24,7 @@

 int
 virDevMapperGetTargets(const char *path,
-                       char ***devPaths) G_GNUC_NO_INLINE;
+                       GSList **devPaths) G_GNUC_NO_INLINE;

 bool
 virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1);
diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
index d1fc50c5f0..3b5f858044 100644
--- a/tests/qemuhotplugmock.c
+++ b/tests/qemuhotplugmock.c
@@ -54,16 +54,14 @@ qemuDomainGetUnplugTimeout(virDomainObj *vm)

 int
 virDevMapperGetTargets(const char *path,
-                       char ***devPaths)
+                       GSList **devPaths)
 {
     *devPaths = NULL;

     if (STREQ(path, "/dev/mapper/virt")) {
-        *devPaths = g_new0(char *, 4);
-        (*devPaths)[0] = g_strdup("/dev/block/8:0");  /* /dev/sda */
-        (*devPaths)[1] = g_strdup("/dev/block/8:16"); /* /dev/sdb */
-        (*devPaths)[2] = g_strdup("/dev/block/8:32"); /* /dev/sdc */
-        (*devPaths)[3] = NULL;
+        *devPaths = g_slist_prepend(*devPaths, g_strdup("/dev/block/8:32")); /* /dev/sdc */
+        *devPaths = g_slist_prepend(*devPaths, g_strdup("/dev/block/8:16")); /* /dev/sdb */
+        *devPaths = g_slist_prepend(*devPaths, g_strdup("/dev/block/8:0")); /* /dev/sda */
     }

     return 0;
-- 
2.31.1




More information about the libvir-list mailing list