From 7ca06472a3169d7c6b3816ad06b2d393298514bc Mon Sep 17 00:00:00 2001

Message-Id: <7ca06472a3169d7c6b3816ad06b2d393298514bc.1386348946.git.jdenemar@redhat.com>

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

Date: Mon, 2 Dec 2013 13:39:11 +0000

Subject: [PATCH] Pull lxcContainerGetSubtree out into shared virfile module

For

  https://bugzilla.redhat.com/show_bug.cgi?id=1035403

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@redhat.com>

(cherry picked from commit d45b833d14477b46e084ef8c629522afa36c7322)

Conflicts:

	tests/Makefile.am

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 .gitignore                    |   1 +

 src/libvirt_private.syms      |   2 +

 src/lxc/lxc_container.c       |  51 +----------------

 src/util/virfile.c            | 112 ++++++++++++++++++++++++++++++++++++++

 src/util/virfile.h            |   9 +++

 tests/Makefile.am             |   6 ++

 tests/virfiledata/mounts1.txt |  31 +++++++++++

 tests/virfiletest.c           | 124 ++++++++++++++++++++++++++++++++++++++++++

 8 files changed, 288 insertions(+), 48 deletions(-)

 create mode 100644 tests/virfiledata/mounts1.txt

 create mode 100644 tests/virfiletest.c

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms

index fa36b31..22df5a9 100644

--- a/src/libvirt_private.syms

+++ b/src/libvirt_private.syms

@@ -1380,6 +1380,8 @@ virFileExists;

 virFileFclose;

 virFileFdopen;

 virFileFindMountPoint;

+virFileGetMountReverseSubtree;

+virFileGetMountSubtree;

 virFileHasSuffix;

 virFileIsAbsPath;

 virFileIsDir;

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c

index c2cbfb4..3f54f82 100644

--- a/src/lxc/lxc_container.c

+++ b/src/lxc/lxc_container.c

@@ -499,50 +499,6 @@ extern int pivot_root(const char * new_root,const char * put_old);

 # 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]),

-              virStringSortRevCompare);

-

-    ret = 0;

-cleanup:

-    *mountsret = mounts;

-    *nmountsret = nmounts;

-    endmntent(procmnt);

-    return ret;

-}

 static int lxcContainerUnmountSubtree(const char *prefix,

                                       bool isOldRootFS)

@@ -556,7 +512,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]);

@@ -592,9 +549,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;

 }

diff --git a/src/util/virfile.c b/src/util/virfile.c

index b396b16..e2a5f7a 100644

--- a/src/util/virfile.c

+++ b/src/util/virfile.c

@@ -1492,6 +1492,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 ? nmounts - 1 : 0;

+    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 ff84719..f6e087e 100644

--- a/src/util/virfile.h

+++ b/src/util/virfile.h

@@ -158,6 +158,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/tests/Makefile.am b/tests/Makefile.am

index 3d5ac72..4141c47 100644

--- a/tests/Makefile.am

+++ b/tests/Makefile.am

@@ -102,6 +102,7 @@ EXTRA_DIST =		\

 	sysinfodata \

 	test-lib.sh	\

 	virsh-uriprecedence \

+	virfiledata \

 	vmx2xmldata \

 	xencapsdata \

 	xmconfigdata \

@@ -121,6 +122,7 @@ test_programs = virshtest sockettest \

 	virbitmaptest \

 	vircgrouptest \

 	virendiantest \

+	virfiletest \

 	viridentitytest \

 	virkeycodetest \

 	virlockspacetest \

@@ -759,6 +761,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..fd6761d

--- /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@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, 1, 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.4.5