[libvirt] [PATCH 2/3] Pull lxcContainerGetSubtree out into shared virfile module

Daniel P. Berrange berrange at redhat.com
Wed Nov 27 16:31:51 UTC 2013


From: "Daniel P. Berrange" <berrange at redhat.com>

Move the code for lxcContainerGetSubtree into the virfile
module creating 2 new functions

  int virFileGetMountSubtree(const char *mtabpath,
                             const char *prefix,
                             char ***mountsret,
                             size_t *nmountsret);
  int virFileGetMountReverseSubtree(const char *mtabpath,
                                    const char *prefix,
                                    char ***mountsret,
                                    size_t *nmountsret);

Add a new virfiletest.c test case to validate the new code.

Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
---
 .gitignore                    |   1 +
 src/libvirt_private.syms      |   4 ++
 src/lxc/lxc_container.c       |  63 ++-------------------
 src/util/virfile.c            | 112 ++++++++++++++++++++++++++++++++++++++
 src/util/virfile.h            |   9 +++
 src/util/virstring.c          |  29 ++++++++++
 src/util/virstring.h          |   3 +
 tests/Makefile.am             |   6 ++
 tests/virfiledata/mounts1.txt |  31 +++++++++++
 tests/virfiletest.c           | 124 ++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 323 insertions(+), 59 deletions(-)
 create mode 100644 tests/virfiledata/mounts1.txt
 create mode 100644 tests/virfiletest.c

diff --git a/.gitignore b/.gitignore
index e4395ac..8de0b26 100644
--- a/.gitignore
+++ b/.gitignore
@@ -195,6 +195,7 @@
 /tests/virdbustest
 /tests/virdrivermoduletest
 /tests/virendiantest
+/tests/virfiletest
 /tests/virhashtest
 /tests/viridentitytest
 /tests/virkeycodetest
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a705c56..2bb5525 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1192,6 +1192,8 @@ virFileFclose;
 virFileFdopen;
 virFileFindMountPoint;
 virFileHasSuffix;
+virFileGetMountSubtree;
+virFileGetMountReverseSubtree;
 virFileIsAbsPath;
 virFileIsDir;
 virFileIsExecutable;
@@ -1747,6 +1749,8 @@ virStringArrayHasString;
 virStringFreeList;
 virStringJoin;
 virStringListLength;
+virStringSortCompare;
+virStringSortRevCompare;
 virStringSplit;
 virStrncpy;
 virStrndup;
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index c24e7fb..0a763cc 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -486,16 +486,6 @@ error_out:
 /*_syscall2(int, pivot_root, char *, newroot, const char *, oldroot)*/
 extern int pivot_root(const char * new_root, const char * put_old);
 
-static int lxcContainerChildMountSort(const void *a, const void *b)
-{
-  const char **sa = (const char**)a;
-  const char **sb = (const char**)b;
-
-  /* Deliberately reversed args - we need to unmount deepest
-     children first */
-  return strcmp(*sb, *sa);
-}
-
 #ifndef MS_REC
 # define MS_REC          16384
 #endif
@@ -512,50 +502,6 @@ static int lxcContainerChildMountSort(const void *a, const void *b)
 # define MS_SLAVE                (1<<19)
 #endif
 
-static int lxcContainerGetSubtree(const char *prefix,
-                                  char ***mountsret,
-                                  size_t *nmountsret)
-{
-    FILE *procmnt;
-    struct mntent mntent;
-    char mntbuf[1024];
-    int ret = -1;
-    char **mounts = NULL;
-    size_t nmounts = 0;
-
-    VIR_DEBUG("prefix=%s", prefix);
-
-    *mountsret = NULL;
-    *nmountsret = 0;
-
-    if (!(procmnt = setmntent("/proc/mounts", "r"))) {
-        virReportSystemError(errno, "%s",
-                             _("Failed to read /proc/mounts"));
-        return -1;
-    }
-
-    while (getmntent_r(procmnt, &mntent, mntbuf, sizeof(mntbuf)) != NULL) {
-        if (!STRPREFIX(mntent.mnt_dir, prefix))
-            continue;
-
-        if (VIR_REALLOC_N(mounts, nmounts+1) < 0)
-            goto cleanup;
-        if (VIR_STRDUP(mounts[nmounts], mntent.mnt_dir) < 0)
-            goto cleanup;
-        nmounts++;
-    }
-
-    if (mounts)
-        qsort(mounts, nmounts, sizeof(mounts[0]),
-              lxcContainerChildMountSort);
-
-    ret = 0;
-cleanup:
-    *mountsret = mounts;
-    *nmountsret = nmounts;
-    endmntent(procmnt);
-    return ret;
-}
 
 static int lxcContainerUnmountSubtree(const char *prefix,
                                       bool isOldRootFS)
@@ -569,7 +515,8 @@ static int lxcContainerUnmountSubtree(const char *prefix,
 
     VIR_DEBUG("Unmount subtreee from %s", prefix);
 
-    if (lxcContainerGetSubtree(prefix, &mounts, &nmounts) < 0)
+    if (virFileGetMountReverseSubtree("/proc/mounts", prefix,
+                                      &mounts, &nmounts) < 0)
         goto cleanup;
     for (i = 0; i < nmounts; i++) {
         VIR_DEBUG("Umount %s", mounts[i]);
@@ -605,9 +552,7 @@ static int lxcContainerUnmountSubtree(const char *prefix,
     ret = 0;
 
 cleanup:
-    for (i = 0; i < nmounts; i++)
-        VIR_FREE(mounts[i]);
-    VIR_FREE(mounts);
+    virStringFreeList(mounts);
 
     return ret;
 }
@@ -816,7 +761,7 @@ static int lxcContainerSetReadOnly(void)
 
     if (mounts)
         qsort(mounts, nmounts, sizeof(mounts[0]),
-              lxcContainerChildMountSort);
+              virStringSortRevCompare);
 
     for (i = 0; i < nmounts; i++) {
         VIR_DEBUG("Bind readonly %s", mounts[i]);
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 0e8d52d..9e421eb 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1591,6 +1591,118 @@ int virFileIsMountPoint(const char *file)
     return ret;
 }
 
+
+#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
+static int
+virFileGetMountSubtreeImpl(const char *mtabpath,
+                           const char *prefix,
+                           char ***mountsret,
+                           size_t *nmountsret,
+                           bool reverse)
+{
+    FILE *procmnt;
+    struct mntent mntent;
+    char mntbuf[1024];
+    int ret = -1;
+    char **mounts = NULL;
+    size_t nmounts = 0;
+
+    VIR_DEBUG("prefix=%s", prefix);
+
+    *mountsret = NULL;
+    *nmountsret = 0;
+
+    if (!(procmnt = setmntent(mtabpath, "r"))) {
+        virReportSystemError(errno,
+                             _("Failed to read %s"), mtabpath);
+        return -1;
+    }
+
+    while (getmntent_r(procmnt, &mntent, mntbuf, sizeof(mntbuf)) != NULL) {
+        if (!STRPREFIX(mntent.mnt_dir, prefix))
+            continue;
+
+        if (VIR_EXPAND_N(mounts, nmounts, nmounts ? 1 : 2) < 0)
+            goto cleanup;
+        if (VIR_STRDUP(mounts[nmounts - 2], mntent.mnt_dir) < 0)
+            goto cleanup;
+    }
+
+    if (mounts)
+        qsort(mounts, nmounts - 1, sizeof(mounts[0]),
+              reverse ? virStringSortRevCompare : virStringSortCompare);
+
+    *mountsret = mounts;
+    *nmountsret = nmounts - 1;
+    ret = 0;
+
+cleanup:
+    if (ret < 0)
+        virStringFreeList(mounts);
+    endmntent(procmnt);
+    return ret;
+}
+#else /* ! defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R */
+static int
+virFileGetMountSubtreeImpl(const char *mtabpath ATTRIBUTE_UNUSED,
+                           const char *prefix ATTRIBUTE_UNUSED,
+                           char ***mountsret ATTRIBUTE_UNUSED,
+                           size_t *nmountsret ATTRIBUTE_UNUSED,
+                           bool reverse ATTRIBUTE_UNUSED)
+{
+    virReportSystemError(ENOSYS, "%s",
+                         _("Unable to determine mount table on this platform"));
+    return -1;
+}
+#endif /* ! defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R */
+
+/**
+ * virFileGetMountSubtree:
+ * @mtabpath: mount file to parser (eg /proc/mounts)
+ * @prefix: mount path prefix to match
+ * @mountsret: allocated and filled with matching mounts
+ * @nmountsret: filled with number of matching mounts, not counting NULL terminator
+ *
+ * Return the list of mounts from @mtabpath which contain
+ * the path @prefix, sorted from shortest to longest path.
+ *
+ * The @mountsret array will be NULL terminated and should
+ * be freed with virStringFreeList
+ *
+ * Returns 0 on success, -1 on error
+ */
+int virFileGetMountSubtree(const char *mtabpath,
+                           const char *prefix,
+                           char ***mountsret,
+                           size_t *nmountsret)
+{
+    return virFileGetMountSubtreeImpl(mtabpath, prefix, mountsret, nmountsret, false);
+}
+
+/**
+ * virFileGetMountReverseSubtree:
+ * @mtabpath: mount file to parser (eg /proc/mounts)
+ * @prefix: mount path prefix to match
+ * @mountsret: allocated and filled with matching mounts
+ * @nmountsret: filled with number of matching mounts, not counting NULL terminator
+ *
+ * Return the list of mounts from @mtabpath which contain
+ * the path @prefix, sorted from longest to shortest path.
+ * ie opposite order to which they appear in @mtabpath
+ *
+ * The @mountsret array will be NULL terminated and should
+ * be freed with virStringFreeList
+ *
+ * Returns 0 on success, -1 on error
+ */
+int virFileGetMountReverseSubtree(const char *mtabpath,
+                                  const char *prefix,
+                                  char ***mountsret,
+                                  size_t *nmountsret)
+{
+    return virFileGetMountSubtreeImpl(mtabpath, prefix, mountsret, nmountsret, true);
+}
+
 #ifndef WIN32
 /* Check that a file is accessible under certain
  * user & gid.
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 10cf8bd..0d20cdb 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -161,6 +161,15 @@ bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1);
 
 int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1);
 
+int virFileGetMountSubtree(const char *mtabpath,
+                           const char *prefix,
+                           char ***mountsret,
+                           size_t *nmountsret) ATTRIBUTE_RETURN_CHECK;
+int virFileGetMountReverseSubtree(const char *mtabpath,
+                                  const char *prefix,
+                                  char ***mountsret,
+                                  size_t *nmountsret) ATTRIBUTE_RETURN_CHECK;
+
 char *virFileSanitizePath(const char *path);
 
 enum {
diff --git a/src/util/virstring.c b/src/util/virstring.c
index d11db5c..8d0ca70 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -616,3 +616,32 @@ size_t virStringListLength(char **strings)
 
     return i;
 }
+
+
+/**
+ * virStringSortCompare:
+ *
+ * A comparator function for sorting strings in
+ * normal order with qsort().
+ */
+int virStringSortCompare(const void *a, const void *b)
+{
+    const char **sa = (const char**)a;
+    const char **sb = (const char**)b;
+
+    return strcmp(*sa, *sb);
+}
+
+/**
+ * virStringSortRevCompare:
+ *
+ * A comparator function for sorting strings in
+ * reverse order with qsort().
+ */
+int virStringSortRevCompare(const void *a, const void *b)
+{
+    const char **sa = (const char**)a;
+    const char **sb = (const char**)b;
+
+    return strcmp(*sb, *sa);
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index b390150..13a6e5a 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -223,4 +223,7 @@ size_t virStringListLength(char **strings);
     virAsprintfInternal(false, 0, NULL, NULL, 0, \
                         strp, __VA_ARGS__)
 
+int virStringSortCompare(const void *a, const void *b);
+int virStringSortRevCompare(const void *a, const void *b);
+
 #endif /* __VIR_STRING_H__ */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 520fd2a..0900448 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -109,6 +109,7 @@ EXTRA_DIST =		\
 	sysinfodata \
 	test-lib.sh	\
 	virsh-uriprecedence \
+	virfiledata \
 	virpcitestdata \
 	vmx2xmldata \
 	xencapsdata \
@@ -131,6 +132,7 @@ test_programs = virshtest sockettest \
 	vircgrouptest \
 	virpcitest \
 	virendiantest \
+	virfiletest \
 	viridentitytest \
 	virkeycodetest \
 	virlockspacetest \
@@ -851,6 +853,10 @@ virendiantest_SOURCES = \
 	virendiantest.c testutils.h testutils.c
 virendiantest_LDADD = $(LDADDS)
 
+virfiletest_SOURCES = \
+	virfiletest.c testutils.h testutils.c
+virfiletest_LDADD = $(LDADDS)
+
 jsontest_SOURCES = \
 	jsontest.c testutils.h testutils.c
 jsontest_LDADD = $(LDADDS)
diff --git a/tests/virfiledata/mounts1.txt b/tests/virfiledata/mounts1.txt
new file mode 100644
index 0000000..79434cc
--- /dev/null
+++ b/tests/virfiledata/mounts1.txt
@@ -0,0 +1,31 @@
+rootfs / rootfs rw 0 0
+proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0
+sysfs /sys sysfs rw,seclabel,nosuid,nodev,noexec,relatime 0 0
+devtmpfs /dev devtmpfs rw,seclabel,nosuid,size=8057768k,nr_inodes=2014442,mode=755 0 0
+securityfs /sys/kernel/security securityfs rw,nosuid,nodev,noexec,relatime 0 0
+selinuxfs /sys/fs/selinux selinuxfs rw,relatime 0 0
+tmpfs /dev/shm tmpfs rw,seclabel,nosuid,nodev 0 0
+devpts /dev/pts devpts rw,seclabel,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0
+tmpfs /run tmpfs rw,seclabel,nosuid,nodev,mode=755 0 0
+tmpfs /sys/fs/cgroup tmpfs rw,seclabel,nosuid,nodev,noexec,mode=755 0 0
+cgroup /sys/fs/cgroup/systemd cgroup rw,nosuid,nodev,noexec,relatime,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd 0 0
+pstore /sys/fs/pstore pstore rw,nosuid,nodev,noexec,relatime 0 0
+cgroup /sys/fs/cgroup/cpuset cgroup rw,nosuid,nodev,noexec,relatime,cpuset 0 0
+cgroup /sys/fs/cgroup/cpu,cpuacct cgroup rw,nosuid,nodev,noexec,relatime,cpuacct,cpu 0 0
+cgroup /sys/fs/cgroup/memory cgroup rw,nosuid,nodev,noexec,relatime,memory 0 0
+cgroup /sys/fs/cgroup/devices cgroup rw,nosuid,nodev,noexec,relatime,devices 0 0
+cgroup /sys/fs/cgroup/freezer cgroup rw,nosuid,nodev,noexec,relatime,freezer 0 0
+cgroup /sys/fs/cgroup/net_cls cgroup rw,nosuid,nodev,noexec,relatime,net_cls 0 0
+cgroup /sys/fs/cgroup/blkio cgroup rw,nosuid,nodev,noexec,relatime,blkio 0 0
+cgroup /sys/fs/cgroup/perf_event cgroup rw,nosuid,nodev,noexec,relatime,perf_event 0 0
+/dev/mapper/fedora-root / ext4 rw,seclabel,relatime,data=ordered 0 0
+systemd-1 /proc/sys/fs/binfmt_misc autofs rw,relatime,fd=33,pgrp=1,timeout=300,minproto=5,maxproto=5,direct 0 0
+configfs /sys/kernel/config configfs rw,relatime 0 0
+debugfs /sys/kernel/debug debugfs rw,relatime 0 0
+mqueue /dev/mqueue mqueue rw,seclabel,relatime 0 0
+tmpfs /tmp tmpfs rw,seclabel 0 0
+hugetlbfs /dev/hugepages hugetlbfs rw,seclabel,relatime 0 0
+binfmt_misc /proc/sys/fs/binfmt_misc binfmt_misc rw,relatime 0 0
+/dev/sda1 /boot ext4 rw,seclabel,relatime,stripe=4,data=ordered 0 0
+fusectl /sys/fs/fuse/connections fusectl rw,relatime 0 0
+gvfsd-fuse /run/user/501/gvfs fuse.gvfsd-fuse rw,nosuid,nodev,relatime,user_id=501,group_id=501 0 0
diff --git a/tests/virfiletest.c b/tests/virfiletest.c
new file mode 100644
index 0000000..9a64565
--- /dev/null
+++ b/tests/virfiletest.c
@@ -0,0 +1,124 @@
+/*
+ * Copyright (C) 2013 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Daniel P. Berrange <berrange at redhat.com>
+ */
+
+#include <config.h>
+
+#include <stdlib.h>
+
+#include "testutils.h"
+#include "virfile.h"
+#include "virstring.h"
+
+
+#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
+static int testFileCheckMounts(const char *prefix,
+                               char **gotmounts,
+                               size_t gotnmounts,
+                               const char *const*wantmounts,
+                               size_t wantnmounts)
+{
+    size_t i;
+    if (gotnmounts != wantnmounts) {
+        fprintf(stderr, "Expected %zu mounts under %s, but got %zu\n",
+                wantnmounts, prefix, gotnmounts);
+        return -1;
+    }
+    for (i = 0; i < gotnmounts; i++) {
+        if (STRNEQ(gotmounts[i], wantmounts[i])) {
+            fprintf(stderr, "Expected mount[%zu] '%s' but got '%s'\n",
+                    i, wantmounts[i], gotmounts[i]);
+            return -1;
+        }
+    }
+    return 0;
+}
+
+struct testFileGetMountSubtreeData {
+    const char *path;
+    const char *prefix;
+    const char *const *mounts;
+    size_t nmounts;
+    bool rev;
+};
+
+static int testFileGetMountSubtree(const void *opaque)
+{
+    int ret = -1;
+    char **gotmounts = NULL;
+    size_t gotnmounts = 0;
+    const struct testFileGetMountSubtreeData *data = opaque;
+
+    if (data->rev) {
+        if (virFileGetMountReverseSubtree(data->path,
+                                          data->prefix,
+                                          &gotmounts,
+                                          &gotnmounts) < 0)
+            goto cleanup;
+    } else {
+        if (virFileGetMountSubtree(data->path,
+                                   data->prefix,
+                                   &gotmounts,
+                                   &gotnmounts) < 0)
+            goto cleanup;
+    }
+
+    ret = testFileCheckMounts(data->prefix,
+                              gotmounts, gotnmounts,
+                              data->mounts, data->nmounts);
+
+ cleanup:
+    virStringFreeList(gotmounts);
+    return ret;
+}
+#endif /* ! defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R */
+
+static int
+mymain(void)
+{
+    int ret = 0;
+
+#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
+# define MTAB_PATH1 abs_srcdir "/virfiledata/mounts1.txt"
+# define MTAB_PATH2 abs_srcdir "/virfiledata/mounts2.txt"
+
+    static const char *wantmounts1[] = {
+        "/proc", "/proc/sys/fs/binfmt_misc", "/proc/sys/fs/binfmt_misc",
+    };
+    static const char *wantmounts1rev[] = {
+        "/proc/sys/fs/binfmt_misc", "/proc/sys/fs/binfmt_misc", "/proc"
+    };
+
+# define DO_TEST_MOUNT_SUBTREE(name, path, prefix, mounts, rev)    \
+    do {                                                           \
+        struct testFileGetMountSubtreeData data = {                \
+            path, prefix, mounts, ARRAY_CARDINALITY(mounts), rev   \
+        };                                                         \
+        if (virtTestRun(name, testFileGetMountSubtree, &data) < 0) \
+            ret = -1;                                              \
+    } while (0)
+
+    DO_TEST_MOUNT_SUBTREE("/proc normal", MTAB_PATH1, "/proc", wantmounts1, false);
+    DO_TEST_MOUNT_SUBTREE("/proc reverse", MTAB_PATH1, "/proc", wantmounts1rev, true);
+#endif /* ! defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R */
+
+    return ret != 0 ? EXIT_FAILURE : EXIT_SUCCESS;
+}
+
+VIRT_TEST_MAIN(mymain)
-- 
1.8.3.1




More information about the libvir-list mailing list