diff --git a/cfg.mk b/cfg.mk index 56d7a5b60c..fa2638f899 100644 --- a/cfg.mk +++ b/cfg.mk @@ -949,7 +949,7 @@ exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics = \ exclude_file_name_regexp--sc_po_check = ^(docs/|src/rpc/gendispatch\.pl$$) exclude_file_name_regexp--sc_prohibit_VIR_ERR_NO_MEMORY = \ - ^(include/libvirt/virterror\.h|daemon/dispatch\.c|src/util/virerror\.c)$$ + ^(include/libvirt/virterror\.h|daemon/dispatch\.c|src/util/virerror\.c|docs/internals/oomtesting\.html\.in)$$ exclude_file_name_regexp--sc_prohibit_access_xok = ^src/util/virutil\.c$$ diff --git a/configure.ac b/configure.ac index 47bc427c3b..9e76353848 100644 --- a/configure.ac +++ b/configure.ac @@ -422,6 +422,8 @@ AC_PATH_PROG([OVSVSCTL], [ovs-vsctl], [ovs-vsctl], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([SCRUB], [scrub], [scrub], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) +AC_PATH_PROG([ADDR2LINE], [addr2line], [addr2line], + [/sbin:/usr/bin:/usr/sbin:/usr/local/sbin:$PATH]) AC_DEFINE_UNQUOTED([DMIDECODE],["$DMIDECODE"], [Location or name of the dmidecode program]) @@ -452,6 +454,8 @@ if test -n "$RMMOD"; then fi AC_DEFINE_UNQUOTED([SCRUB],["$SCRUB"], [Location or name of the scrub program (for wiping algorithms)]) +AC_DEFINE_UNQUOTED([ADDR2LINE],["$ADDR2LINE"], + [Location of addr2line program]) dnl Specific dir for HTML output ? AC_ARG_WITH([html-dir], [AS_HELP_STRING([--with-html-dir=path], diff --git a/docs/internals/oomtesting.html.in b/docs/internals/oomtesting.html.in new file mode 100644 index 0000000000..c5edacff6a --- /dev/null +++ b/docs/internals/oomtesting.html.in @@ -0,0 +1,213 @@ + + + + +

Out of memory testing

+ + + + +

+ This page describes how to use the test suite todo out of memory + testing. +

+ +

Building with OOM testing

+ +

+ Since OOM testing requires hooking into the malloc APIs, it is + not enabled by default. The flag --enable-test-oom + must be given to configure. When this is done the + libvirt allocation APIs will have some hooks enabled. +

+ +
+$ ./configure --enable-test-oom
+
+ + +

Basic OOM testing support

+ +

+ The first step in validating OOM usage is to run a test suite + with full OOM testing enabled. This is done by setting the + VIR_TEST_OOM=1 environment variable. The way this + works is that it runs the test once normally to "prime" any + static memory allocations. Then it runs it once more counting + the total number of memory allocations. Then it runs it in a + loop failing a different memory allocation each time. For every + memory allocation failure triggered, it expects the test case + to return an error. OOM testing is quite slow requiring each + test case to be executed O(n) times, where 'n' is the total + number of memory allocations. This results in a total number + of memory allocations of '(n * (n + 1) ) / 2' +

+ +
+$ VIR_TEST_OOM=1 ./qemuxml2argvtest
+ 1) QEMU XML-2-ARGV minimal                                           ... OK
+    Test OOM for nalloc=42 .......................................... OK
+ 2) QEMU XML-2-ARGV minimal-s390                                      ... OK
+    Test OOM for nalloc=28 ............................ OK
+ 3) QEMU XML-2-ARGV machine-aliases1                                  ... OK
+    Test OOM for nalloc=38 ...................................... OK
+ 4) QEMU XML-2-ARGV machine-aliases2                                  ... OK
+    Test OOM for nalloc=38 ...................................... OK
+ 5) QEMU XML-2-ARGV machine-core-on                                   ... OK
+    Test OOM for nalloc=37 ..................................... OK
+...snip...
+
+ +

+ In this output, the first line shows the normal execution and + the test number, and the second line shows the total number + of memory allocations from that test case. +

+ +

Tracking failures with valgrind

+ +

+ The test suite should obviously *not* crash during OOM testing. + If it does crash, then to assist in tracking down the problem + it is worth using valgrind and only running a single test case. + For example, supposing test case 5 crashed. Then re-run the + test with +

+ +
+$ VIR_TEST_OOM=1 VIR_TEST_RANGE=5 ../run valgrind ./qemuxml2argvtest
+...snip...
+ 5) QEMU XML-2-ARGV machine-core-on                                   ... OK
+    Test OOM for nalloc=37 ..................................... OK
+...snip...
+    
+ +

+ Valgrind should report the cause of the crash - for example a + double free or use of uninitialized memory or NULL pointer + access. +

+ +

Tracking failures with stack traces

+ +

+ With some really difficult bugs valgrind is not sufficient to + identify the cause. In this case, it is useful to identify the + precise allocation which was failed, to allow the code path + to the error to be traced. The VIR_TEST_OOM + env variable can be given a range of memory allocations to + test. So if a test case has 150 allocations, it can be told + to only test allocation numbers 7-10. The VIR_TEST_OOM_TRACE + variable can be used to print out stack traces. +

+ +
+$ VIR_TEST_OOM_TRACE=2 VIR_TEST_OOM=1:7-10 VIR_TEST_RANGE=5 \
+    ../run valgrind ./qemuxml2argvtest
+ 5) QEMU XML-2-ARGV machine-core-on                                   ... OK
+    Test OOM for nalloc=37 !virAllocN
+/home/berrange/src/virt/libvirt/src/util/viralloc.c:180
+virDomainDefParseXML
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:11786 (discriminator 1)
+virDomainDefParseNode
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12677
+virDomainDefParse
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12621
+testCompareXMLToArgvFiles
+/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:107
+virtTestRun
+/home/berrange/src/virt/libvirt/tests/testutils.c:266
+mymain
+/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:388 (discriminator 2)
+virtTestMain
+/home/berrange/src/virt/libvirt/tests/testutils.c:791
+__libc_start_main
+??:?
+_start
+??:?
+!virAlloc
+/home/berrange/src/virt/libvirt/src/util/viralloc.c:133
+virDomainDiskDefParseXML
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:4790
+virDomainDefParseXML
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:11797
+virDomainDefParseNode
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12677
+virDomainDefParse
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12621
+testCompareXMLToArgvFiles
+/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:107
+virtTestRun
+/home/berrange/src/virt/libvirt/tests/testutils.c:266
+mymain
+/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:388 (discriminator 2)
+virtTestMain
+/home/berrange/src/virt/libvirt/tests/testutils.c:791
+__libc_start_main
+??:?
+_start
+??:?
+!virAllocN
+/home/berrange/src/virt/libvirt/src/util/viralloc.c:180
+virXPathNodeSet
+/home/berrange/src/virt/libvirt/src/util/virxml.c:609
+virDomainDefParseXML
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:11805
+virDomainDefParseNode
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12677
+virDomainDefParse
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12621
+testCompareXMLToArgvFiles
+/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:107
+virtTestRun
+/home/berrange/src/virt/libvirt/tests/testutils.c:266
+mymain
+/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:388 (discriminator 2)
+virtTestMain
+/home/berrange/src/virt/libvirt/tests/testutils.c:791
+__libc_start_main
+??:?
+_start
+??:?
+!virAllocN
+/home/berrange/src/virt/libvirt/src/util/viralloc.c:180
+virDomainDefParseXML
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:11808 (discriminator 1)
+virDomainDefParseNode
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12677
+virDomainDefParse
+/home/berrange/src/virt/libvirt/src/conf/domain_conf.c:12621
+testCompareXMLToArgvFiles
+/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:107
+virtTestRun
+/home/berrange/src/virt/libvirt/tests/testutils.c:266
+mymain
+/home/berrange/src/virt/libvirt/tests/qemuxml2argvtest.c:388 (discriminator 2)
+virtTestMain
+/home/berrange/src/virt/libvirt/tests/testutils.c:791
+__libc_start_main
+??:?
+_start
+??:?
+    
+ +

Non-crash related problems

+ +

+ Not all memory allocation bugs result in code crashing. Sometimes + the code will be silently ignoring the allocation failure, resulting + in incorrect data being produced. For example the XML parser may + mistakenly treat an allocation failure as indicating that an XML + attribute was not set in the input document. It is hard to identify + these problems from the test suite automatically. For this, the + test suites should be run with VIR_TEST_DEBUG=1 set + and then stderr analysed for any unexpected data. For example, + the XML conversion may show an embedded "(null)" literal, or the + test suite might complain about missing elements / attributes + in the actual vs expected data. These are all signs of bugs in + OOM handling. In the future the OOM tests will be enhanced to + validate that an error VIR_ERR_NO_MEMORY is returned for each + allocation failed, rather than some other error. +

+ + diff --git a/docs/sitemap.html.in b/docs/sitemap.html.in index 08c2fbddc9..7d0610b4bb 100644 --- a/docs/sitemap.html.in +++ b/docs/sitemap.html.in @@ -342,6 +342,10 @@ Lock managers Use lock managers to protect disk content +
  • + Out of memory testing + Simulating OOM conditions in the test suite +
  • diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 1b13fcc8db..e75c85db2f 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -42,7 +42,6 @@ static int testCompareXMLToArgvFiles(const char *xml, char *cmd = NULL; int ret = -1; virDomainDefPtr vmdef = NULL; - char *log; if (virtTestLoadFile(cmdfile, &cmd) < 0) goto fail; @@ -53,13 +52,16 @@ static int testCompareXMLToArgvFiles(const char *xml, cmd, NULL, NULL, NULL))) goto fail; - if ((log = virtTestLogContentAndReset()) == NULL) - goto fail; - if ((*log != '\0') != expect_warning) { + if (!virtTestOOMActive()) { + char *log; + if ((log = virtTestLogContentAndReset()) == NULL) + goto fail; + if ((*log != '\0') != expect_warning) { + VIR_FREE(log); + goto fail; + } VIR_FREE(log); - goto fail; } - VIR_FREE(log); if (!virDomainDefCheckABIStability(vmdef, vmdef)) { fprintf(stderr, "ABI stability check failed on %s", xml); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 55548513d5..af7194f69c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -284,7 +284,8 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(vmdef = virDomainDefParseFile(xml, driver.caps, driver.xmlopt, QEMU_EXPECTED_VIRT_TYPES, VIR_DOMAIN_XML_INACTIVE))) { - if (flags & FLAG_EXPECT_PARSE_ERROR) + if (!virtTestOOMActive() && + (flags & FLAG_EXPECT_PARSE_ERROR)) goto ok; goto out; } @@ -357,7 +358,8 @@ static int testCompareXMLToArgvFiles(const char *xml, migrateFrom, migrateFd, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, &testCallbacks))) { - if (flags & FLAG_EXPECT_FAILURE) { + if (!virtTestOOMActive() && + (flags & FLAG_EXPECT_FAILURE)) { ret = 0; if (virTestGetDebug() > 1) fprintf(stderr, "Got expected error: %s\n", @@ -371,7 +373,8 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; } - if (!!virGetLastError() != !!(flags & FLAG_EXPECT_ERROR)) { + if (!virtTestOOMActive() && + (!!virGetLastError() != !!(flags & FLAG_EXPECT_ERROR))) { if (virTestGetDebug() && (log = virtTestLogContentAndReset())) fprintf(stderr, "\n%s", log); goto out; @@ -392,7 +395,8 @@ static int testCompareXMLToArgvFiles(const char *xml, } ok: - if (flags & FLAG_EXPECT_ERROR) { + if (!virtTestOOMActive() && + (flags & FLAG_EXPECT_ERROR)) { /* need to suppress the errors */ virResetLastError(); } diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index 9426bf7571..20a5ccd040 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -122,15 +122,17 @@ static int testCompareXMLToArgvFiles(const char *xml, &testCallbacks))) goto fail; - if (!!virGetLastError() != expectError) { - if (virTestGetDebug() && (log = virtTestLogContentAndReset())) - fprintf(stderr, "\n%s", log); - goto fail; - } + if (!virtTestOOMActive()) { + if (!!virGetLastError() != expectError) { + if (virTestGetDebug() && (log = virtTestLogContentAndReset())) + fprintf(stderr, "\n%s", log); + goto fail; + } - if (expectError) { - /* need to suppress the errors */ - virResetLastError(); + if (expectError) { + /* need to suppress the errors */ + virResetLastError(); + } } if (!(actualargv = virCommandToString(cmd))) diff --git a/tests/testutils.c b/tests/testutils.c index 32fe3745c3..3f42b13270 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -47,6 +47,13 @@ #include "virprocess.h" #include "virstring.h" +#ifdef TEST_OOM +# ifdef TEST_OOM_TRACE +# include +# include +# endif +#endif + #ifdef HAVE_PATHS_H # include #endif @@ -64,12 +71,37 @@ static unsigned int testDebug = -1; static unsigned int testVerbose = -1; static unsigned int testExpensive = -1; +#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 +void *testAllocStack[30]; +int ntestAllocStack; +# endif +#endif +static bool testOOMActive = false; + static size_t testCounter = 0; static size_t testStart = 0; static size_t testEnd = 0; char *progname; +bool virtTestOOMActive(void) +{ + return testOOMActive; +} + +#ifdef TEST_OOM_TRACE +static void virTestAllocHook(int nalloc ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + ntestAllocStack = backtrace(testAllocStack, ARRAY_CARDINALITY(testAllocStack)); +} +#endif + void virtTestResult(const char *name, int ret, const char *msg, ...) { va_list vargs; @@ -108,6 +140,35 @@ void virtTestResult(const char *name, int ret, const char *msg, ...) va_end(vargs); } +#ifdef TEST_OOM_TRACE +static void +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 : ""), + testAllocStack[j]) < 0) + continue; + } + ignore_value(system(cmd)); + VIR_FREE(cmd); + } +} +#endif + /* * Runs test * @@ -154,7 +215,7 @@ virtTestRun(const char *title, !((testCounter-1) % 40)) { fprintf(stderr, " %-3zu\n", (testCounter-1)); fprintf(stderr, " "); - } + } if (ret == 0) fprintf(stderr, "."); else if (ret == EXIT_AM_SKIP) @@ -163,6 +224,77 @@ virtTestRun(const char *title, fprintf(stderr, "!"); } +#ifdef TEST_OOM + if (testOOM && ret != EXIT_AM_SKIP) { + int nalloc; + int oomret; + int start, end; + size_t i; + virResetLastError(); + virAllocTestInit(); +# ifdef TEST_OOM_TRACE + virAllocTestHook(virTestAllocHook, NULL); +# endif + oomret = body(data); + nalloc = virAllocTestCount(); + fprintf(stderr, " Test OOM for nalloc=%d ", nalloc); + if (testOOMStart == -1 || + testOOMEnd == -1) { + start = 0; + end = nalloc; + } else { + start = testOOMStart; + end = testOOMEnd + 1; + } + 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); + 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)) { + 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; } @@ -191,7 +323,6 @@ 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); VIR_FORCE_FCLOSE(fp); return -1; } @@ -467,7 +598,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 @@ -535,6 +667,9 @@ int virtTestMain(int argc, { int ret; char *testRange = NULL; +#ifdef TEST_OOM + char *oomstr; +#endif if (!virFileExists(abs_srcdir)) return EXIT_AM_HARDFAIL; @@ -590,6 +725,60 @@ 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 == '\0') { + testOOMEnd = testOOMStart; + } else { + if (*next != '-') { + fprintf(stderr, "Cannot parse range %s\n", oomstr); + return EXIT_FAILURE; + } + if (virStrToLong_ui(next+1, NULL, 10, &testOOMEnd) < 0) { + fprintf(stderr, "Cannot parse range %s\n", oomstr); + return EXIT_FAILURE; + } + } + } else { + testOOMStart = -1; + testOOMEnd = -1; + } + } + +# ifdef TEST_OOM_TRACE + if ((oomstr = getenv("VIR_TEST_OOM_TRACE")) != NULL) { + if (virStrToLong_ui(oomstr, NULL, 10, &testOOMTrace) < 0) { + fprintf(stderr, "Cannot parse oom trace %s\n", oomstr); + return EXIT_FAILURE; + } + } +# else + if (getenv("VIR_TEST_OOM_TRACE")) { + fprintf(stderr, "%s", "OOM test tracing not enabled in this build\n"); + return EXIT_FAILURE; + } +# endif +#else /* TEST_OOM */ + if (getenv("VIR_TEST_OOM")) { + fprintf(stderr, "%s", "OOM testing not enabled in this build\n"); + return EXIT_FAILURE; + } + if (getenv("VIR_TEST_OOM_TRACE")) { + fprintf(stderr, "%s", "OOM test tracing not enabled in this build\n"); + return EXIT_FAILURE; + } +#endif /* TEST_OOM */ + ret = (func)(); virResetLastError(); diff --git a/tests/testutils.h b/tests/testutils.h index 674d3df98d..29eb9d91d1 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -44,6 +44,8 @@ extern char *progname; # error Fix Makefile.am # endif +bool virtTestOOMActive(void); + void virtTestResult(const char *name, int ret, const char *msg, ...) ATTRIBUTE_FMT_PRINTF(3,4); int virtTestRun(const char *title,