On 09/25/2013 11:23 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> The previous OOM testing support would re-run the entire "main"
> method each iteration, failing a different malloc each time.
> When a test suite has 'n' allocations, the number of repeats
> requires is (n * (n + 1) ) / 2. This gets very large, very
> quickly.
>
> This new OOM testing support instead integrates at the
> virtTestRun level, so each individual test case gets repeated,
> instead of the entire test suite. This means the values of
> 'n' are orders of magnitude smaller.
Still O(n^2) - but yes, a smaller n makes for a NOTICEABLE speedup [that
is, 30 alloc's per test on a main() that runs 30 tests is much nicer
than 900 alloc's per main()] :)
>
> The simple usage is
>
> $ VIR_TEST_OOM=1 ./qemuxml2argvtest
> ...
> 29) QEMU XML-2-ARGV clock-utc ... OK
> Test OOM for nalloc=36 .................................... OK
> 30) QEMU XML-2-ARGV clock-localtime ... OK
> Test OOM for nalloc=36 .................................... OK
> 31) QEMU XML-2-ARGV clock-france ... OK
> Test OOM for nalloc=38 ...................................... OK
> ...
>
> the second lines reports how many mallocs have to be failed, and thus
> how many repeats of the test wil be run.
s/wil/will/
>
> If it crashes, then running under valgrind will often show the problem
>
> $ VIR_TEST_OOM=1 ../run valgrind ./qemuxml2argvtest
>
> When debugging problems it is also helpful to select an individual
> test case
>
> $ VIR_TEST_RANGE=30 VIR_TEST_OOM=1 ../run valgrind ./qemuxml2argvtest
>
> When things get really tricky, it is possible to request that just
> specific allocs are failed. eg to fail allocs 5 -> 12, use
>
> $ VIR_TEST_RANGE=30 VIR_TEST_OOM=1:5-12 ../run valgrind ./qemuxml2argvtest
Please document VIR_TEST_OOM in docs/hacking.html.in (and thus HACKING).
Does it require building with './configure --enable-test-oom'?
>
> In the worse case, you might want to know the stack trace of the
> alloc which was failed then VIR_TEST_OOM_TRACE can be set. If it
> is set to 1 then it will only print if it things a mistake happened.
s/things/thinks/
> This is often not reliable, so setting it to 2 will make it print
> the stack trace for every alloc that is failed.
>
> $ VIR_TEST_OOM_TRACE=2 VIR_TEST_RANGE=30 VIR_TEST_OOM=1:5-5 ../run valgrind ./qemuxml2argvtest
> 30) QEMU XML-2-ARGV clock-localtime ... OK
> Test OOM for nalloc=36 !virAllocN
> /home/berrange/src/virt/libvirt/src/util/viralloc.c:180
> virHashCreateFull
> /home/berrange/src/virt/libvirt/src/util/virhash.c:144
> virDomainDefParseXML
> /home/berrange/src/virt/libvirt/src/conf/domain_conf.c:11745
> virDomainDefParseNode
> /home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12646
> virDomainDefParse
> /home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12590
> testCompareXMLToArgvFiles
> /home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:106
> virtTestRun
> /home/berrange/src/virt/libvirt/tests/testutils.c:250
> mymain
> /home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:418 (discriminator 2)
> virtTestMain
> /home/berrange/src/virt/libvirt/tests/testutils.c:750
> ??
> ??:0
> _start
> ??:?
> FAILED
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> tests/cputest.c | 3 +-
> tests/qemuargv2xmltest.c | 14 ++--
> tests/qemuxml2argvtest.c | 12 ++-
> tests/qemuxmlnstest.c | 18 +++--
> tests/testutils.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++-
> tests/testutils.h | 2 +
> 6 files changed, 216 insertions(+), 22 deletions(-)
>
> diff --git a/tests/cputest.c b/tests/cputest.c
> index 8e3640b..17cb6af 100644
> --- a/tests/cputest.c
> +++ b/tests/cputest.c
> @@ -476,7 +476,8 @@ cpuTestRun(const char *name, const struct data *data)
> if (virTestGetDebug()) {
> char *log;
> if ((log = virtTestLogContentAndReset()) &&
> - strlen(log) > 0)
> + log != NULL &&
> + strlen(log) > 0)
Spurious change adding dead code. You already checked that log was
non-null in the left half of the &&.
> +++ b/tests/testutils.c
> @@ -47,6 +47,9 @@
> #include "virprocess.h"
> #include "virstring.h"
>
> +#include <dlfcn.h>
> +#include <execinfo.h>
Needs to be conditional (these are not universal headers; gnulib won't
help you).
> +#ifdef TEST_OOM
> +static unsigned int testOOM = 0;
> +static unsigned int testOOMStart = -1;
> +static unsigned int testOOMEnd = -1;
> +static unsigned int testOOMTrace = 0;
> +# ifdef TEST_OOM_TRACE
Why the two conditions? Or is TEST_OOM_TRACE based on whether
<execinfo.h> is present?
> +virTestShowTrace(void)
> +{
> + size_t j;
> + for (j = 2; j < ntestAllocStack; j++) {
> + Dl_info info;
> + char *cmd;
> +
> + dladdr(testAllocStack[j], &info);
> + if (info.dli_fname &&
> + strstr(info.dli_fname, ".so")) {
> + if (virAsprintf(&cmd, "addr2line -f -e %s %p",
> + info.dli_fname,
> + ((void*)((unsigned long long)testAllocStack[j]
> + - (unsigned long long)info.dli_fbase))) < 0)
> + continue;
> + } else {
> + if (virAsprintf(&cmd, "addr2line -f -e %s %p",
> + (char*)(info.dli_fname ? info.dli_fname : "<unknown>"),
> + testAllocStack[j]) < 0)
> + continue;
> + }
> + ignore_value(system(cmd));
configure.ac defined TEST_OOM_TRACE solely based on whether
'backtrace()' exists; but as you are also using addr2line and looking
for .so files, which starts to feel specific to Linux, is it more
conservative to disable this option on non-Linux?
> @@ -168,7 +226,7 @@ virtTestRun(const char *title,
> !((testCounter-1) % 40)) {
> fprintf(stderr, " %-3zu\n", (testCounter-1));
> fprintf(stderr, " ");
> - }
> + }
Missed in patch 3/4?
> + testOOMActive = true;
> + for (i = start; i < end; i++) {
> + bool missingFail = false;
> +# ifdef TEST_OOM_TRACE
> + memset(testAllocStack, 0, ARRAY_CARDINALITY(testAllocStack));
> + ntestAllocStack = 0;
> +# endif
> + virAllocTestOOM(i+1, 1);
spaces around +
> + oomret = body(data);
> +
> + /* fprintf() disabled because XML parsing APIs don't allow
> + * distinguish between element / attribute not present
> + * in the XML (which is non-fatal), vs OOM / malformed
> + * which should be fatal. Thus error reporting for
> + * optionally present XML is mostly broken.
> + */
> + if (oomret == 0) {
> + missingFail = true;
> +# if 0
> + fprintf(stderr, " alloc %zu failed but no err status\n", i + 1);
> +# endif
> + } else {
> + virErrorPtr lerr = virGetLastError();
> + if (!lerr) {
> +# if 0
> + fprintf(stderr, " alloc %zu failed but no error report\n", i + 1);
> +# endif
> + missingFail = true;
> + }
> + }
> + if ((missingFail && testOOMTrace) || (testOOMTrace > 1)) {
Safe to drop () around 'testOOMTrace > 1'
> + fprintf(stderr, "%s", "!");
> +# ifdef TEST_OOM_TRACE
> + virTestShowTrace();
> +# endif
> + ret = -1;
> + } else {
> + fprintf(stderr, "%s", ".");
> + }
> + }
> + testOOMActive = false;
> + if (ret == 0)
> + fprintf(stderr, " OK\n");
> + else
> + fprintf(stderr, " FAILED\n");
> + virAllocTestInit();
> + }
> +#endif /* TEST_OOM */
> +
> return ret;
> }
>
> @@ -205,7 +334,7 @@ virtTestLoadFile(const char *file, char **buf)
> tmplen = buflen = st.st_size + 1;
>
> if (VIR_ALLOC_N(*buf, buflen) < 0) {
> - fprintf(stderr, "%s: larger than available memory (> %d)\n", file, buflen);
> + //fprintf(stderr, "%s: larger than available memory (> %d)\n", file, buflen);
Is this hunk intentional?
> VIR_FORCE_FCLOSE(fp);
> return -1;
> }
> @@ -481,7 +610,8 @@ virtTestLogOutput(virLogSource source ATTRIBUTE_UNUSED,
> {
> struct virtTestLogData *log = data;
> virCheckFlags(VIR_LOG_STACK_TRACE,);
> - virBufferAsprintf(&log->buf, "%s: %s", timestamp, str);
> + if (!testOOMActive)
> + virBufferAsprintf(&log->buf, "%s: %s", timestamp, str);
> }
>
> static void
> @@ -550,6 +680,9 @@ int virtTestMain(int argc,
> int ret;
> bool abs_srcdir_cleanup = false;
> char *testRange = NULL;
> +#ifdef TEST_OOM
> + char *oomstr;
> +#endif
>
> abs_srcdir = getenv("abs_srcdir");
> if (!abs_srcdir) {
> @@ -610,6 +743,56 @@ int virtTestMain(int argc,
> }
> }
>
> +#ifdef TEST_OOM
> + if ((oomstr = getenv("VIR_TEST_OOM")) != NULL) {
> + char *next;
> + if (testDebug == -1)
> + testDebug = 1;
> + testOOM = 1;
> + if (oomstr[0] != '\0' &&
> + oomstr[1] == ':') {
> + if (virStrToLong_ui(oomstr + 2, &next, 10, &testOOMStart) < 0) {
> + fprintf(stderr, "Cannot parse range %s\n", oomstr);
> + return EXIT_FAILURE;
> + }
> + if (*next != '-') {
> + fprintf(stderr, "Cannot parse range %s\n", oomstr);
> + return EXIT_FAILURE;
> + }
Worth accepting VIR_TEST_OOM=1:1 as shorthand for 1:1-1?
I like where it's headed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list