On Fri, Apr 12, 2013 at 03:24:04PM -0600, Eric Blake wrote:
On 04/10/2013 04:08 AM, Daniel P. Berrange wrote:
+++ b/tests/Makefile.am @@ -97,7 +97,9 @@ test_programs = virshtest sockettest \ utiltest shunloadtest \ virtimetest viruritest virkeyfiletest \ virauthconfigtest \ - virbitmaptest virendiantest \ + virbitmaptest \ + vircgrouptest \ + virendiantest \ viridentitytest \ virkeycodetest \ virlockspacetest \ @@ -247,6 +249,7 @@ EXTRA_DIST += $(test_scripts)
test_libraries = libshunload.la \ libvirportallocatormock.la \ + vircgroupmock.la \ $(NULL)
shunload.c is guarded by #ifdef linux; you should do the same for vircgroupmock.c.
Yep, put #ifdef __linux__ around the test source files
+vircgrouptest_SOURCES = \ + vircgrouptest.c testutils.h testutils.c +vircgrouptest_LDADD = $(LDADDS) + +vircgroupmock_la_SOURCES = \ + vircgroupmock.c +vircgroupmock_la_CFLAGS = $(AM_CFLAGS) -DMOCK_HELPER=1
Why do you need -DMOCK_HELPER=1? Too much copy and past from virportallocatortest.c, which crammed both the mock library and the test into the same file rather than your approach here of using two files?
Yes, I prviously had it all in two files, but split it when the helper got too big
+#include <config.h> + +#include "internal.h" + +#include <stdio.h> +#include <dlfcn.h> +#include <stdlib.h> +#include <unistd.h> +#include <fcntl.h> +#include <sys/stat.h> + +static int (*realopen)(const char *path, int flags, ...);
Don't you need <stdarg.h> to make it possible to implement your realopen()? [1]
Yes, though I guess something must have pulled it in indirectly. Will add the explicit include though.
+{ + int fd = -1; + int ret = -1; + char *filepath = NULL; + + if (asprintf(&filepath, "%s/%s", path, name) < 0) + return -1; + + if ((fd = open(filepath, O_CREAT|O_WRONLY, 0600)) < 0)
Is it okay if this calls our open() instead of realopen()?
We're lucky enough to be safe, but I switched it to use realopen()
+ if (STREQ(controller, "cpu")) { + if (symlink("cpu,cpuacct", path) < 0) + return -1; + return -0;
-0 is cute. Couldn't this just be:
return symlink("cpu,cpuacct", path);
Yes, I've made that change
+ +int open(const char *path, int flags, ...) +{ + int ret; + char *newpath = NULL; + + init_syms(); + + if (STRPREFIX(path, SYSFS_PREFIX)) { + init_sysfs(); + if (asprintf(&newpath, "%s/%s", + fakesysfsdir, + path + strlen(SYSFS_PREFIX)) < 0) { + errno = ENOMEM; + return -1; + } + } + if (flags & O_CREAT) { + va_list ap;
[1] Yep, you DO need to fix the includes to use <stdarg.h>.
+ mode_t mode; + va_start(ap, flags); + mode = va_arg(ap, mode_t); + va_end(ap); + ret = realopen(newpath ? newpath : path, flags, mode); + } else { + ret = realopen(newpath ? newpath : path, flags); + }
You know, wouldn't it just work to write the simpler:
int open(const char *path, int flags, mode_t mode)
The problem is that the decl on open() has to match the decl used in <fcntl.h>. Since that uses '...' we must too.
diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c new file mode 100644 index 0000000..a68aa88 --- /dev/null +++ b/tests/vircgrouptest.c @@ -0,0 +1,249 @@
...
+ +static int validateCgroup(virCgroupPtr cgroup, + const char *expectPath, + const char **expectMountPoint, + const char **expectPlacement) +{ + int i; + + if (STRNEQ(cgroup->path, expectPath)) { + fprintf(stderr, "Wrong path '%s', expected '%s'\n", + cgroup->path, expectPath); + return -1; + }
+ +const char *mountsSmall[VIR_CGROUP_CONTROLLER_LAST] = { + [VIR_CGROUP_CONTROLLER_CPU] = "/not/really/sys/fs/cgroup/cpu,cpuacct",
Is it worth spelling this: SYSFS_PREFIX "/cgroup/cpu,cpuacct"
Well SYSFS_PREFIX only exists in the other source file, not this one :-)
+ + return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/libvircgroupmock.so")
I know that you have access to a mingw cross-compilation environment - ACK if you fix the issues I mentioned above and check that things still compile on mingw.
Wow, we have neglected mingw recently. I've found a tonne of pre-existing problems :-( 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 :|