[libvirt] [PATCH 0/7] tests: Always fake root directory
When mocking filesystem access in the test suite, we have always assumed it to be some subdirectory of /sys depending on the needs of the specific test case. This limits our flexibility, and will be a problem once we need to start mocking eg. /dev as well, or simply two different parts of the /sys filesystem at the same time[1]. Solve the issue by always using the temporary directory as the root directory for the mocked filesystem. The series is organized as follows: 1-2: Tiny cleanups 3-4: Change the mock libraries so that they build the proper directory structure 5: Change the name of the environment variable used to pass the temporary directory name to the mock libraries 6: Make it so we'll be able to mock different parts of the /sys filesystem at the same time 7: Update scsihosttest to use the same directory structure Cheers. [1] https://www.redhat.com/archives/libvir-list/2015-November/msg00532.html is an example of why we would need to do that Andrea Bolognani (7): tests: scsihost: Don't set LIBVIRT_FAKE_SYSFS_DIR tests: pcimock: Remove check for fakesysfsdir tests: pcimock: Use the temporary directory as fake root tests: cgroupmock: Use the temporary directory as fake root tests: Rename LIBVIRT_FAKE_SYSFS_DIR to LIBVIRT_FAKE_ROOT_DIR tests: Use more specific names for variables tests: scsihost: Use fakerootdir instead of fakesysfsdir tests/scsihosttest.c | 17 ++++++--- tests/vircgroupmock.c | 99 ++++++++++++++++++++++++++++---------------------- tests/vircgrouptest.c | 16 ++++---- tests/virhostdevtest.c | 16 ++++---- tests/virpcimock.c | 69 +++++++++++++++++++---------------- tests/virpcitest.c | 16 ++++---- 6 files changed, 129 insertions(+), 104 deletions(-) -- 2.5.0
The test program is not preloading any of the mock libraries that read that environment variable, so setting it is pointless. --- tests/scsihosttest.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/scsihosttest.c b/tests/scsihosttest.c index eecb1c3..eb5f522 100644 --- a/tests/scsihosttest.c +++ b/tests/scsihosttest.c @@ -262,8 +262,6 @@ mymain(void) goto cleanup; } - setenv("LIBVIRT_FAKE_SYSFS_DIR", fakesysfsdir, 1); - if (init_scsihost_sysfs(fakesysfsdir) < 0) { fprintf(stderr, "Failed to create fakesysfs='%s'\n", fakesysfsdir); goto cleanup; -- 2.5.0
init_env() will return right away if fakesysfsdir is already initialized, so this check is redundant. --- tests/virpcimock.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 0b49290..6d00a4f 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -229,8 +229,7 @@ static int getrealpath(char **newpath, const char *path) { - if (!fakesysfsdir) - init_env(); + init_env(); if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { if (virAsprintfQuiet(newpath, "%s/%s", -- 2.5.0
We might need to mock files living outside PCI_SYSFS_PREFIX later on, so it's better to treat the temporary directory we are passed via the environment as the root of the fake filesystem and create PCI_SYSFS_PREFIX inside it. The environment variable name will be changed to reflect the new use we're making of it in a later commit. --- tests/virpcimock.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 6d00a4f..f1517ce 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -49,6 +49,7 @@ static DIR * (*realopendir)(const char *name); * when passed as an arg to virAsprintf() * vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in an inline function with external linkage [-Werror,-Wstatic-in-inline] */ +char *fakerootdir; char *fakesysfsdir; # define PCI_SYSFS_PREFIX "/sys/bus/pci/" @@ -800,12 +801,19 @@ init_syms(void) static void init_env(void) { - if (fakesysfsdir) + if (fakerootdir && fakesysfsdir) return; - if (!(fakesysfsdir = getenv("LIBVIRT_FAKE_SYSFS_DIR"))) + if (!(fakerootdir = getenv("LIBVIRT_FAKE_SYSFS_DIR"))) ABORT("Missing LIBVIRT_FAKE_SYSFS_DIR env variable\n"); + if (virAsprintfQuiet(&fakesysfsdir, "%s%s", + fakerootdir, PCI_SYSFS_PREFIX) < 0) + ABORT_OOM(); + + if (virFileMakePath(fakesysfsdir) < 0) + ABORT("Unable to create: %s", fakesysfsdir); + make_file(fakesysfsdir, "drivers_probe", NULL, -1); # define MAKE_PCI_DRIVER(name, ...) \ -- 2.5.0
We might need to mock files living outside SYSFS_PREFIX later on, so it's better to treat the temporary directory we are passed via the environment as the root of the fake filesystem and create SYSFS_PREFIX inside it. The environment variable name will be changed to reflect the new use we're making of it in a later commit. --- tests/vircgroupmock.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index f2fee7d..ead18b6 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -31,6 +31,8 @@ # include <sys/stat.h> # include <stdarg.h> # include "testutilslxc.h" +# include "virstring.h" +# include "virfile.h" static int (*realopen)(const char *path, int flags, ...); static FILE *(*realfopen)(const char *path, const char *mode); @@ -45,6 +47,7 @@ static int (*realmkdir)(const char *path, mode_t mode); * when passed as an arg to asprintf() * vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in an inline function with external linkage [-Werror,-Wstatic-in-inline] */ +char *fakerootdir; char *fakesysfsdir; const char *fakedevicedir0 = FAKEDEVDIR0; const char *fakedevicedir1 = FAKEDEVDIR1; @@ -414,14 +417,23 @@ static void init_syms(void) static void init_sysfs(void) { - if (fakesysfsdir) + if (fakerootdir && fakesysfsdir) return; - if (!(fakesysfsdir = getenv("LIBVIRT_FAKE_SYSFS_DIR"))) { + if (!(fakerootdir = getenv("LIBVIRT_FAKE_SYSFS_DIR"))) { fprintf(stderr, "Missing LIBVIRT_FAKE_SYSFS_DIR env variable\n"); abort(); } + if (virAsprintfQuiet(&fakesysfsdir, "%s%s", + fakerootdir, SYSFS_PREFIX) < 0) + abort(); + + if (virFileMakePath(fakesysfsdir) < 0) { + fprintf(stderr, "Cannot create %s\n", fakesysfsdir); + abort(); + } + # define MAKE_CONTROLLER(subpath) \ do { \ char *path; \ -- 2.5.0
The old name is no longer accurate, since now we're using its value as the root of the fake filesystem. No functional changes. --- tests/vircgroupmock.c | 10 +++++----- tests/vircgrouptest.c | 16 ++++++++-------- tests/virhostdevtest.c | 16 ++++++++-------- tests/virpcimock.c | 6 +++--- tests/virpcitest.c | 16 ++++++++-------- 5 files changed, 32 insertions(+), 32 deletions(-) diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index ead18b6..fb33af1 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -67,9 +67,9 @@ const char *fakedevicedir1 = FAKEDEVDIR1; * * In any open/acces/mkdir calls we look at path and if * it starts with /not/really/sys/fs/cgroup, we rewrite - * the path to point at a temporary directory referred - * to by LIBVIRT_FAKE_SYSFS_DIR env variable that is - * set by the main test suite + * the path to point at a subdirectory of the temporary + * directory referred to by LIBVIRT_FAKE_ROOT_DIR env + * variable that is set by the main test suite * * In mkdir() calls, we simulate the cgroups behaviour * whereby creating the directory auto-creates a bunch @@ -420,8 +420,8 @@ static void init_sysfs(void) if (fakerootdir && fakesysfsdir) return; - if (!(fakerootdir = getenv("LIBVIRT_FAKE_SYSFS_DIR"))) { - fprintf(stderr, "Missing LIBVIRT_FAKE_SYSFS_DIR env variable\n"); + if (!(fakerootdir = getenv("LIBVIRT_FAKE_ROOT_DIR"))) { + fprintf(stderr, "Missing LIBVIRT_FAKE_ROOT_DIR env variable\n"); abort(); } diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 731ecc4..7ea6e13 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -847,25 +847,25 @@ static int testCgroupGetBlkioIoDeviceServiced(const void *args ATTRIBUTE_UNUSED) return ret; } -# define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX" +# define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XXXXXX" static int mymain(void) { int ret = 0; - char *fakesysfsdir; + char *fakerootdir; - if (VIR_STRDUP_QUIET(fakesysfsdir, FAKESYSFSDIRTEMPLATE) < 0) { + if (VIR_STRDUP_QUIET(fakerootdir, FAKEROOTDIRTEMPLATE) < 0) { fprintf(stderr, "Out of memory\n"); abort(); } - if (!mkdtemp(fakesysfsdir)) { - fprintf(stderr, "Cannot create fakesysfsdir"); + if (!mkdtemp(fakerootdir)) { + fprintf(stderr, "Cannot create fakerootdir"); abort(); } - setenv("LIBVIRT_FAKE_SYSFS_DIR", fakesysfsdir, 1); + setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, 1); # define DETECT_MOUNTS(file) \ do { \ @@ -937,9 +937,9 @@ mymain(void) unsetenv("VIR_CGROUP_MOCK_MODE"); if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) - virFileDeleteTree(fakesysfsdir); + virFileDeleteTree(fakerootdir); - VIR_FREE(fakesysfsdir); + VIR_FREE(fakerootdir); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c index faebdd4..2a74976 100644 --- a/tests/virhostdevtest.c +++ b/tests/virhostdevtest.c @@ -410,25 +410,25 @@ testVirHostdevUpdateActivePCIHostdevs(const void *oaque ATTRIBUTE_UNUSED) return ret; } -# define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX" +# define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XXXXXX" static int mymain(void) { int ret = 0; - char *fakesysfsdir; + char *fakerootdir; - if (VIR_STRDUP_QUIET(fakesysfsdir, FAKESYSFSDIRTEMPLATE) < 0) { + if (VIR_STRDUP_QUIET(fakerootdir, FAKEROOTDIRTEMPLATE) < 0) { fprintf(stderr, "Out of memory\n"); abort(); } - if (!mkdtemp(fakesysfsdir)) { - fprintf(stderr, "Cannot create fakesysfsdir"); + if (!mkdtemp(fakerootdir)) { + fprintf(stderr, "Cannot create fakerootdir"); abort(); } - setenv("LIBVIRT_FAKE_SYSFS_DIR", fakesysfsdir, 1); + setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, 1); # define DO_TEST(fnc) \ do { \ @@ -458,9 +458,9 @@ mymain(void) myCleanup(); if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) - virFileDeleteTree(fakesysfsdir); + virFileDeleteTree(fakerootdir); - VIR_FREE(fakesysfsdir); + VIR_FREE(fakerootdir); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/virpcimock.c b/tests/virpcimock.c index f1517ce..9778783 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -71,7 +71,7 @@ char *fakesysfsdir; * The plan: * * Mock some file handling functions. Redirect them into a stub tree passed via - * LIBVIRT_FAKE_SYSFS_DIR env variable. All files and links within stub tree is + * LIBVIRT_FAKE_ROOT_DIR env variable. All files and links within stub tree is * created by us. There are some actions that we must take if some special * files are written to. Here's the list of files we watch: * @@ -804,8 +804,8 @@ init_env(void) if (fakerootdir && fakesysfsdir) return; - if (!(fakerootdir = getenv("LIBVIRT_FAKE_SYSFS_DIR"))) - ABORT("Missing LIBVIRT_FAKE_SYSFS_DIR env variable\n"); + if (!(fakerootdir = getenv("LIBVIRT_FAKE_ROOT_DIR"))) + ABORT("Missing LIBVIRT_FAKE_ROOT_DIR env variable\n"); if (virAsprintfQuiet(&fakesysfsdir, "%s%s", fakerootdir, PCI_SYSFS_PREFIX) < 0) diff --git a/tests/virpcitest.c b/tests/virpcitest.c index d4d3253..f1c5369 100644 --- a/tests/virpcitest.c +++ b/tests/virpcitest.c @@ -350,25 +350,25 @@ testVirPCIDeviceUnbind(const void *opaque) return ret; } -# define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX" +# define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XXXXXX" static int mymain(void) { int ret = 0; - char *fakesysfsdir; + char *fakerootdir; - if (VIR_STRDUP_QUIET(fakesysfsdir, FAKESYSFSDIRTEMPLATE) < 0) { + if (VIR_STRDUP_QUIET(fakerootdir, FAKEROOTDIRTEMPLATE) < 0) { VIR_TEST_DEBUG("Out of memory\n"); abort(); } - if (!mkdtemp(fakesysfsdir)) { - VIR_TEST_DEBUG("Cannot create fakesysfsdir"); + if (!mkdtemp(fakerootdir)) { + VIR_TEST_DEBUG("Cannot create fakerootdir"); abort(); } - setenv("LIBVIRT_FAKE_SYSFS_DIR", fakesysfsdir, 1); + setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, 1); # define DO_TEST(fnc) \ do { \ @@ -445,9 +445,9 @@ mymain(void) DO_TEST_PCI_DRIVER(0, 0x0a, 3, 0, NULL); if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) - virFileDeleteTree(fakesysfsdir); + virFileDeleteTree(fakerootdir); - VIR_FREE(fakesysfsdir); + VIR_FREE(fakerootdir); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.5.0
Instead of fakesysfsdir, which is very generic, use fakesysfspcidir and fakesysfscgroupdir. This makes it explicit what part of the fake sysfs filesystem they're referring to, and also leaves open the possibility of handling files in two unrelated parts of the fake sysfs filesystem. No functional changes. --- tests/vircgroupmock.c | 85 ++++++++++++++++++++++++++------------------------- tests/virpcimock.c | 60 ++++++++++++++++++------------------ 2 files changed, 73 insertions(+), 72 deletions(-) diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index fb33af1..9ce7d41 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -48,12 +48,12 @@ static int (*realmkdir)(const char *path, mode_t mode); * vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in an inline function with external linkage [-Werror,-Wstatic-in-inline] */ char *fakerootdir; -char *fakesysfsdir; +char *fakesysfscgroupdir; const char *fakedevicedir0 = FAKEDEVDIR0; const char *fakedevicedir1 = FAKEDEVDIR1; -# define SYSFS_PREFIX "/not/really/sys/fs/cgroup/" +# define SYSFS_CGROUP_PREFIX "/not/really/sys/fs/cgroup/" # define SYSFS_CPU_PRESENT "/sys/devices/system/cpu/present" # define SYSFS_CPU_PRESENT_MOCKED "devices_system_cpu_present" @@ -186,11 +186,11 @@ static int make_controller(const char *path, mode_t mode) int ret = -1; const char *controller; - if (!STRPREFIX(path, fakesysfsdir)) { + if (!STRPREFIX(path, fakesysfscgroupdir)) { errno = EINVAL; return -1; } - controller = path + strlen(fakesysfsdir) + 1; + controller = path + strlen(fakesysfscgroupdir) + 1; if (STREQ(controller, "cpu")) return symlink("cpu,cpuacct", path); @@ -417,7 +417,7 @@ static void init_syms(void) static void init_sysfs(void) { - if (fakerootdir && fakesysfsdir) + if (fakerootdir && fakesysfscgroupdir) return; if (!(fakerootdir = getenv("LIBVIRT_FAKE_ROOT_DIR"))) { @@ -425,24 +425,24 @@ static void init_sysfs(void) abort(); } - if (virAsprintfQuiet(&fakesysfsdir, "%s%s", - fakerootdir, SYSFS_PREFIX) < 0) + if (virAsprintfQuiet(&fakesysfscgroupdir, "%s%s", + fakerootdir, SYSFS_CGROUP_PREFIX) < 0) abort(); - if (virFileMakePath(fakesysfsdir) < 0) { - fprintf(stderr, "Cannot create %s\n", fakesysfsdir); + if (virFileMakePath(fakesysfscgroupdir) < 0) { + fprintf(stderr, "Cannot create %s\n", fakesysfscgroupdir); abort(); } -# define MAKE_CONTROLLER(subpath) \ - do { \ - char *path; \ - if (asprintf(&path, "%s/%s", fakesysfsdir, subpath) < 0)\ - abort(); \ - if (make_controller(path, 0755) < 0) { \ - fprintf(stderr, "Cannot initialize %s\n", path); \ - abort(); \ - } \ +# define MAKE_CONTROLLER(subpath) \ + do { \ + char *path; \ + if (asprintf(&path, "%s/%s", fakesysfscgroupdir, subpath) < 0) \ + abort(); \ + if (make_controller(path, 0755) < 0) { \ + fprintf(stderr, "Cannot initialize %s\n", path); \ + abort(); \ + } \ } while (0) MAKE_CONTROLLER("cpu"); @@ -454,7 +454,8 @@ static void init_sysfs(void) MAKE_CONTROLLER("memory"); MAKE_CONTROLLER("freezer"); - if (make_file(fakesysfsdir, SYSFS_CPU_PRESENT_MOCKED, "8-23,48-159\n") < 0) + if (make_file(fakesysfscgroupdir, + SYSFS_CPU_PRESENT_MOCKED, "8-23,48-159\n") < 0) abort(); } @@ -528,12 +529,12 @@ int access(const char *path, int mode) init_syms(); - if (STRPREFIX(path, SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); char *newpath; if (asprintf(&newpath, "%s/%s", - fakesysfsdir, - path + strlen(SYSFS_PREFIX)) < 0) { + fakesysfscgroupdir, + path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; return -1; } @@ -559,12 +560,12 @@ int __lxstat(int ver, const char *path, struct stat *sb) init_syms(); - if (STRPREFIX(path, SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); char *newpath; if (asprintf(&newpath, "%s/%s", - fakesysfsdir, - path + strlen(SYSFS_PREFIX)) < 0) { + fakesysfscgroupdir, + path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; return -1; } @@ -590,12 +591,12 @@ int lstat(const char *path, struct stat *sb) init_syms(); - if (STRPREFIX(path, SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); char *newpath; if (asprintf(&newpath, "%s/%s", - fakesysfsdir, - path + strlen(SYSFS_PREFIX)) < 0) { + fakesysfscgroupdir, + path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; return -1; } @@ -621,12 +622,12 @@ int __xstat(int ver, const char *path, struct stat *sb) init_syms(); - if (STRPREFIX(path, SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); char *newpath; if (asprintf(&newpath, "%s/%s", - fakesysfsdir, - path + strlen(SYSFS_PREFIX)) < 0) { + fakesysfscgroupdir, + path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; return -1; } @@ -656,16 +657,16 @@ int stat(const char *path, struct stat *sb) if (STREQ(path, SYSFS_CPU_PRESENT)) { init_sysfs(); if (asprintf(&newpath, "%s/%s", - fakesysfsdir, + fakesysfscgroupdir, SYSFS_CPU_PRESENT_MOCKED) < 0) { errno = ENOMEM; return -1; } - } else if (STRPREFIX(path, SYSFS_PREFIX)) { + } else if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); if (asprintf(&newpath, "%s/%s", - fakesysfsdir, - path + strlen(SYSFS_PREFIX)) < 0) { + fakesysfscgroupdir, + path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; return -1; } @@ -692,12 +693,12 @@ int mkdir(const char *path, mode_t mode) init_syms(); - if (STRPREFIX(path, SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); char *newpath; if (asprintf(&newpath, "%s/%s", - fakesysfsdir, - path + strlen(SYSFS_PREFIX)) < 0) { + fakesysfscgroupdir, + path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; return -1; } @@ -719,18 +720,18 @@ int open(const char *path, int flags, ...) if (STREQ(path, SYSFS_CPU_PRESENT)) { init_sysfs(); if (asprintf(&newpath, "%s/%s", - fakesysfsdir, + fakesysfscgroupdir, SYSFS_CPU_PRESENT_MOCKED) < 0) { errno = ENOMEM; return -1; } } - if (STRPREFIX(path, SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); if (asprintf(&newpath, "%s/%s", - fakesysfsdir, - path + strlen(SYSFS_PREFIX)) < 0) { + fakesysfscgroupdir, + path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; return -1; } diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 9778783..cfe42a6 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -50,9 +50,9 @@ static DIR * (*realopendir)(const char *name); * vircgroupmock.c:462:22: error: static variable 'fakesysfsdir' is used in an inline function with external linkage [-Werror,-Wstatic-in-inline] */ char *fakerootdir; -char *fakesysfsdir; +char *fakesysfspcidir; -# define PCI_SYSFS_PREFIX "/sys/bus/pci/" +# define SYSFS_PCI_PREFIX "/sys/bus/pci/" # define STDERR(...) \ fprintf(stderr, "%s %zu: ", __FUNCTION__, (size_t) __LINE__); \ @@ -201,8 +201,8 @@ pci_read_file(const char *path, char *newpath; if (virAsprintfQuiet(&newpath, "%s/%s", - fakesysfsdir, - path + strlen(PCI_SYSFS_PREFIX)) < 0) { + fakesysfspcidir, + path + strlen(SYSFS_PCI_PREFIX)) < 0) { errno = ENOMEM; goto cleanup; } @@ -232,10 +232,10 @@ getrealpath(char **newpath, { init_env(); - if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_PCI_PREFIX)) { if (virAsprintfQuiet(newpath, "%s/%s", - fakesysfsdir, - path + strlen(PCI_SYSFS_PREFIX)) < 0) { + fakesysfspcidir, + path + strlen(SYSFS_PCI_PREFIX)) < 0) { errno = ENOMEM; return -1; } @@ -343,7 +343,7 @@ pci_device_new_from_stub(const struct pciDevice *data) if (VIR_ALLOC_QUIET(dev) < 0 || virAsprintfQuiet(&configSrc, "%s/virpcitestdata/%s.config", abs_srcdir, id) < 0 || - virAsprintfQuiet(&devpath, "%s/devices/%s", fakesysfsdir, data->id) < 0) + virAsprintfQuiet(&devpath, "%s/devices/%s", fakesysfspcidir, data->id) < 0) ABORT_OOM(); VIR_FREE(id); @@ -449,7 +449,7 @@ pci_driver_new(const char *name, int fail, ...) if (VIR_ALLOC_QUIET(driver) < 0 || VIR_STRDUP_QUIET(driver->name, name) < 0 || - virAsprintfQuiet(&driverpath, "%s/drivers/%s", fakesysfsdir, name) < 0) + virAsprintfQuiet(&driverpath, "%s/drivers/%s", fakesysfspcidir, name) < 0) ABORT_OOM(); driver->fail = fail; @@ -532,9 +532,9 @@ pci_driver_bind(struct pciDriver *driver, /* Make symlink under device tree */ if (virAsprintfQuiet(&devpath, "%s/devices/%s/driver", - fakesysfsdir, dev->id) < 0 || + fakesysfspcidir, dev->id) < 0 || virAsprintfQuiet(&driverpath, "%s/drivers/%s", - fakesysfsdir, driver->name) < 0) { + fakesysfspcidir, driver->name) < 0) { errno = ENOMEM; goto cleanup; } @@ -546,9 +546,9 @@ pci_driver_bind(struct pciDriver *driver, VIR_FREE(devpath); VIR_FREE(driverpath); if (virAsprintfQuiet(&devpath, "%s/devices/%s", - fakesysfsdir, dev->id) < 0 || + fakesysfspcidir, dev->id) < 0 || virAsprintfQuiet(&driverpath, "%s/drivers/%s/%s", - fakesysfsdir, driver->name, dev->id) < 0) { + fakesysfspcidir, driver->name, dev->id) < 0) { errno = ENOMEM; goto cleanup; } @@ -579,9 +579,9 @@ pci_driver_unbind(struct pciDriver *driver, /* Make symlink under device tree */ if (virAsprintfQuiet(&devpath, "%s/devices/%s/driver", - fakesysfsdir, dev->id) < 0 || + fakesysfspcidir, dev->id) < 0 || virAsprintfQuiet(&driverpath, "%s/drivers/%s/%s", - fakesysfsdir, driver->name, dev->id) < 0) { + fakesysfspcidir, driver->name, dev->id) < 0) { errno = ENOMEM; goto cleanup; } @@ -801,20 +801,20 @@ init_syms(void) static void init_env(void) { - if (fakerootdir && fakesysfsdir) + if (fakerootdir && fakesysfspcidir) return; if (!(fakerootdir = getenv("LIBVIRT_FAKE_ROOT_DIR"))) ABORT("Missing LIBVIRT_FAKE_ROOT_DIR env variable\n"); - if (virAsprintfQuiet(&fakesysfsdir, "%s%s", - fakerootdir, PCI_SYSFS_PREFIX) < 0) + if (virAsprintfQuiet(&fakesysfspcidir, "%s%s", + fakerootdir, SYSFS_PCI_PREFIX) < 0) ABORT_OOM(); - if (virFileMakePath(fakesysfsdir) < 0) - ABORT("Unable to create: %s", fakesysfsdir); + if (virFileMakePath(fakesysfspcidir) < 0) + ABORT("Unable to create: %s", fakesysfspcidir); - make_file(fakesysfsdir, "drivers_probe", NULL, -1); + make_file(fakesysfspcidir, "drivers_probe", NULL, -1); # define MAKE_PCI_DRIVER(name, ...) \ pci_driver_new(name, 0, __VA_ARGS__, -1, -1) @@ -861,7 +861,7 @@ access(const char *path, int mode) init_syms(); - if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_PCI_PREFIX)) { char *newpath; if (getrealpath(&newpath, path) < 0) return -1; @@ -880,7 +880,7 @@ __lxstat(int ver, const char *path, struct stat *sb) init_syms(); - if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_PCI_PREFIX)) { char *newpath; if (getrealpath(&newpath, path) < 0) return -1; @@ -899,7 +899,7 @@ lstat(const char *path, struct stat *sb) init_syms(); - if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_PCI_PREFIX)) { char *newpath; if (getrealpath(&newpath, path) < 0) return -1; @@ -918,7 +918,7 @@ __xstat(int ver, const char *path, struct stat *sb) init_syms(); - if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_PCI_PREFIX)) { char *newpath; if (getrealpath(&newpath, path) < 0) return -1; @@ -937,7 +937,7 @@ stat(const char *path, struct stat *sb) init_syms(); - if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_PCI_PREFIX)) { char *newpath; if (getrealpath(&newpath, path) < 0) return -1; @@ -956,7 +956,7 @@ canonicalize_file_name(const char *path) init_syms(); - if (STRPREFIX(path, PCI_SYSFS_PREFIX)) { + if (STRPREFIX(path, SYSFS_PCI_PREFIX)) { char *newpath; if (getrealpath(&newpath, path) < 0) return NULL; @@ -976,7 +976,7 @@ open(const char *path, int flags, ...) init_syms(); - if (STRPREFIX(path, PCI_SYSFS_PREFIX) && + if (STRPREFIX(path, SYSFS_PCI_PREFIX) && getrealpath(&newpath, path) < 0) return -1; @@ -993,7 +993,7 @@ open(const char *path, int flags, ...) /* Catch both: /sys/bus/pci/drivers/... and * /sys/bus/pci/device/.../driver/... */ - if (ret >= 0 && STRPREFIX(path, PCI_SYSFS_PREFIX) && + if (ret >= 0 && STRPREFIX(path, SYSFS_PCI_PREFIX) && strstr(path, "driver") && add_fd(ret, path) < 0) { realclose(ret); ret = -1; @@ -1011,7 +1011,7 @@ opendir(const char *path) init_syms(); - if (STRPREFIX(path, PCI_SYSFS_PREFIX) && + if (STRPREFIX(path, SYSFS_PCI_PREFIX) && getrealpath(&newpath, path) < 0) return NULL; -- 2.5.0
This updates the test program to make it consistent with recent changes to the mock libraries, and also opens up the possibility of mocking more than just /sys in the future. --- tests/scsihosttest.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/scsihosttest.c b/tests/scsihosttest.c index eb5f522..ab3b0ae 100644 --- a/tests/scsihosttest.c +++ b/tests/scsihosttest.c @@ -244,21 +244,27 @@ testVirFindSCSIHostByPCI(const void *data ATTRIBUTE_UNUSED) return ret; } -# define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX" +# define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XXXXXX" static int mymain(void) { int ret = -1; + char *fakerootdir = NULL; char *fakesysfsdir = NULL; - if (VIR_STRDUP_QUIET(fakesysfsdir, FAKESYSFSDIRTEMPLATE) < 0) { + if (VIR_STRDUP_QUIET(fakerootdir, FAKEROOTDIRTEMPLATE) < 0) { fprintf(stderr, "Out of memory\n"); goto cleanup; } - if (!mkdtemp(fakesysfsdir)) { - fprintf(stderr, "Cannot create fakesysfsdir"); + if (!mkdtemp(fakerootdir)) { + fprintf(stderr, "Cannot create fakerootdir"); + goto cleanup; + } + + if (virAsprintfQuiet(&fakesysfsdir, "%s/sys", fakerootdir) < 0) { + fprintf(stderr, "Out of memory\n"); goto cleanup; } @@ -290,7 +296,8 @@ mymain(void) cleanup: if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) - virFileDeleteTree(fakesysfsdir); + virFileDeleteTree(fakerootdir); + VIR_FREE(fakerootdir); VIR_FREE(fakesysfsdir); VIR_FREE(scsihost_class_path); return ret; -- 2.5.0
On 04.12.2015 15:34, Andrea Bolognani wrote:
When mocking filesystem access in the test suite, we have always assumed it to be some subdirectory of /sys depending on the needs of the specific test case.
This limits our flexibility, and will be a problem once we need to start mocking eg. /dev as well, or simply two different parts of the /sys filesystem at the same time[1].
Solve the issue by always using the temporary directory as the root directory for the mocked filesystem.
The series is organized as follows:
1-2: Tiny cleanups 3-4: Change the mock libraries so that they build the proper directory structure 5: Change the name of the environment variable used to pass the temporary directory name to the mock libraries 6: Make it so we'll be able to mock different parts of the /sys filesystem at the same time 7: Update scsihosttest to use the same directory structure
Cheers.
[1] https://www.redhat.com/archives/libvir-list/2015-November/msg00532.html is an example of why we would need to do that
Andrea Bolognani (7): tests: scsihost: Don't set LIBVIRT_FAKE_SYSFS_DIR tests: pcimock: Remove check for fakesysfsdir tests: pcimock: Use the temporary directory as fake root tests: cgroupmock: Use the temporary directory as fake root tests: Rename LIBVIRT_FAKE_SYSFS_DIR to LIBVIRT_FAKE_ROOT_DIR tests: Use more specific names for variables tests: scsihost: Use fakerootdir instead of fakesysfsdir
tests/scsihosttest.c | 17 ++++++--- tests/vircgroupmock.c | 99 ++++++++++++++++++++++++++++---------------------- tests/vircgrouptest.c | 16 ++++---- tests/virhostdevtest.c | 16 ++++---- tests/virpcimock.c | 69 +++++++++++++++++++---------------- tests/virpcitest.c | 16 ++++---- 6 files changed, 129 insertions(+), 104 deletions(-)
ACK, although I'd rather wait until after release. If you (or somebody else) feels otherwise, do push it any time you want. Michal
On Fri, 2015-12-04 at 16:31 +0100, Michal Privoznik wrote:
ACK, although I'd rather wait until after release. If you (or somebody else) feels otherwise, do push it any time you want.
No need to rush, I'll push after release. Thanks for the review! -- Andrea Bolognani Software Engineer - Virtualization Team
On Fri, 2015-12-04 at 17:07 +0100, Andrea Bolognani wrote:
On Fri, 2015-12-04 at 16:31 +0100, Michal Privoznik wrote:
ACK, although I'd rather wait until after release. If you (or somebody else) feels otherwise, do push it any time you want.
No need to rush, I'll push after release.
Thanks for the review!
Now pushed. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
participants (2)
-
Andrea Bolognani -
Michal Privoznik