Re: [libvirt] [PATCH v3 06/16] Add a test suite for cgroups functionality

On 04/10/2013 04:08 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > Some aspects of the cgroups setup / detection code are quite subtle > and easy to break. It would greatly benefit from unit testing, but > this is difficult because the test suite won't have privileges to > play around with cgroups. The solution is to use monkey patching > via LD_PRELOAD to override the fopen, open, mkdir, access functions > to redirect access of cgroups files to some magic stubs in the > test suite. > > Using this we provide custom content for the /proc/cgroup and > /proc/self/mounts files which report a fixed cgroup setup. We > then override open/mkdir/access so that access to the cgroups > filesystem gets redirected into files in a temporary directory > tree in the test suite build dir. Do you also need to override openat/mkdirat/faccessat, in case we (or even libc on our behalf) ever uses the newer *at syscalls?

Wow, this looks complicated, so I'll have to defer my review to sometime earlier in my day when I'm thinking straight. But the premise is useful, and a passing 'make check' even on a system with no cgroups mounted is a pretty good indication of whether you got it right. > +/* > + * The plan: > + * > + * We fake out /proc/mounts, so make it look as is cgroups > + * are mounted on /not/really/sys/fs/cgroup. We don't > + * use /sys/fs/cgroup, because we want to make it easy to > + * detect places where we've not mocked enough syscalls. and so that the testsuite will run and pass even on systems without cgroups mounted.

> +mymain(void) > +{ > + int ret = 0; > + char *fakesysfsdir; > + > + if (!(fakesysfsdir = strdup(FAKESYSFSDIRTEMPLATE))) { > + fprintf(stderr, "Out of memory\n"); > + abort(); > + } > + > + if (!mkdtemp(fakesysfsdir)) { Does this compile on mingw, or do you need to modify bootstrap.conf to pull in the mkdtemp gnulib module? [Then again, it won't compile on mingw in the first place, since the Makefile.am limits it to platforms with LD_PRELOAD support]