From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Define 'abs_srcdir' in AM_CFLAGS, just as we do for 'abs_builddir'. This lets test files write code like abs_srcdir "/some/file" and thus avoid the tedium of virAsprintf() to build paths in some places. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- tests/Makefile.am | 1 + tests/testutils.c | 12 ------------ tests/testutils.h | 1 - tests/virpcimock.c | 5 ----- 4 files changed, 1 insertion(+), 18 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index e46d5f7..520fd2a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,6 +29,7 @@ INCLUDES = \ AM_CFLAGS = \ -Dabs_builddir="\"`pwd`\"" \ + -Dabs_srcdir="\"`cd '$(srcdir)'; pwd`\"" \ $(LIBXML_CFLAGS) \ $(GNUTLS_CFLAGS) \ $(SASL_CFLAGS) \ diff --git a/tests/testutils.c b/tests/testutils.c index 5d634b4..2a75f6c 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -69,7 +69,6 @@ static size_t testStart = 0; static size_t testEnd = 0; char *progname; -char *abs_srcdir; void virtTestResult(const char *name, int ret, const char *msg, ...) { @@ -535,17 +534,8 @@ int virtTestMain(int argc, int (*func)(void)) { int ret; - bool abs_srcdir_cleanup = false; char *testRange = NULL; - abs_srcdir = getenv("abs_srcdir"); - if (!abs_srcdir) { - abs_srcdir = getcwd(NULL, 0); - abs_srcdir_cleanup = true; - } - if (!abs_srcdir) - return EXIT_AM_HARDFAIL; - progname = last_component(argv[0]); if (STRPREFIX(progname, "lt-")) progname += 3; @@ -599,8 +589,6 @@ int virtTestMain(int argc, ret = (func)(); - if (abs_srcdir_cleanup) - VIR_FREE(abs_srcdir); virResetLastError(); if (!virTestGetVerbose() && ret != EXIT_AM_SKIP) { if (testCounter == 0 || testCounter % 40) diff --git a/tests/testutils.h b/tests/testutils.h index 478b53c..66f25b0 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -38,7 +38,6 @@ # endif extern char *progname; -extern char *abs_srcdir; void virtTestResult(const char *name, int ret, const char *msg, ...) ATTRIBUTE_FMT_PRINTF(3,4); diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 19062c3..a0b9643 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -305,11 +305,6 @@ pci_device_new_from_stub(const struct pciDevice *data) char *configSrc, *configDst; char tmp[32]; struct stat sb; - char *abs_srcdir; - - abs_srcdir = getenv("abs_srcdir"); - if (!abs_srcdir) - abs_srcdir = getcwd(NULL, 0); if (VIR_ALLOC_QUIET(dev) < 0 || virAsprintfQuiet(&configSrc, "%s/virpcitestdata/%s.config", abs_srcdir, data->id) < 0 || -- 1.8.3.1

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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(a)redhat.com&gt; --- .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(a)redhat.com&gt; + */ + +#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

On Wed, Nov 27, 2013 at 03:25:28PM -0700, Eric Blake wrote: Actually virsh can't use it - it sorts virDomainPtr instances rather than just the name strings. That said I've split it off anyway, and added a test case to virstringtest.c Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|