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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
 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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux