[libvirt] [PATCH 0/2] Couple of fixes after mockup patches
I've noticed a few tests failing strangely after [1] was merged. Here are the fixes. 1: https://www.redhat.com/archives/libvir-list/2015-September/msg00460.html Michal Privoznik (2): qemuTestDriverInit: init the driver lock too tests: Avoid use of virQEMUDriverCreateXMLConf(NULL) tests/qemucapabilitiestest.c | 9 +++++---- tests/qemumonitorjsontest.c | 19 ++++++++++--------- tests/qemumonitortest.c | 9 +++++---- tests/securityselinuxlabeltest.c | 8 +++++--- tests/testutilsqemu.c | 6 +++++- 5 files changed, 30 insertions(+), 21 deletions(-) -- 2.4.9
Even though usage of the lock is limited to a very few cases, it's still needed. Therefore we should initialize it too. Otherwise we may get some random test failures: ==1204== Conditional jump or move depends on uninitialised value(s) ==1204== at 0xEF7F7CF: pthread_mutex_lock (in /lib64/libpthread-2.20.so) ==1204== by 0x9CA89A5: virMutexLock (virthread.c:89) ==1204== by 0x450B2A: qemuDriverLock (qemu_conf.c:83) ==1204== by 0x45549C: virQEMUDriverGetConfig (qemu_conf.c:869) ==1204== by 0x448E29: qemuDomainDeviceDefPostParse (qemu_domain.c:1240) ==1204== by 0x9CC9B13: virDomainDeviceDefPostParse (domain_conf.c:4224) ==1204== by 0x9CC9B91: virDomainDefPostParseDeviceIterator (domain_conf.c:4251) ==1204== by 0x9CC7843: virDomainDeviceInfoIterateInternal (domain_conf.c:3440) ==1204== by 0x9CC9C25: virDomainDefPostParse (domain_conf.c:4276) ==1204== by 0x9CEEE03: virDomainDefParseXML (domain_conf.c:16400) ==1204== by 0x9CEF5B4: virDomainDefParseNode (domain_conf.c:16582) ==1204== by 0x9CEF423: virDomainDefParse (domain_conf.c:16529) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/testutilsqemu.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index cec2f6c..2f2be12 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -531,6 +531,7 @@ qemuTestParseCapabilities(const char *capsFile) void qemuTestDriverFree(virQEMUDriver *driver) { + virMutexDestroy(&driver->lock); virQEMUCapsCacheFree(driver->qemuCapsCache); virObjectUnref(driver->xmlopt); virObjectUnref(driver->caps); @@ -567,9 +568,12 @@ int qemuTestCapsCacheInsert(virQEMUCapsCachePtr cache, const char *binary, int qemuTestDriverInit(virQEMUDriver *driver) { + if (virMutexInit(&driver->lock) < 0) + return -1; + driver->config = virQEMUDriverConfigNew(false); if (!driver->config) - return -1; + goto error; driver->caps = testQemuCapsInit(); if (!driver->caps) -- 2.4.9
We use the function to create a virDomainXMLOption object that is required for some functions. However, we don't pass the driver pointer to the object anywhere - rather than pass NULL. This causes trouble later when parsing a domain XML and calling post parse callbacks: Program received signal SIGSEGV, Segmentation fault. 0x000000000043fa3e in qemuDomainDefPostParse (def=0x7d36c0, caps=0x7caf10, opaque=0x0) at qemu/qemu_domain.c:1043 1043 qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator); (gdb) bt #0 0x000000000043fa3e in qemuDomainDefPostParse (def=0x7d36c0, caps=0x7caf10, opaque=0x0) at qemu/qemu_domain.c:1043 #1 0x00007ffff2928bf9 in virDomainDefPostParse (def=0x7d36c0, caps=0x7caf10, xmlopt=0x7c82c0) at conf/domain_conf.c:4269 #2 0x00007ffff294de04 in virDomainDefParseXML (xml=0x7da8c0, root=0x7dab80, ctxt=0x7da980, caps=0x7caf10, xmlopt=0x7c82c0, flags=0) at conf/domain_conf.c:16400 #3 0x00007ffff294e5b5 in virDomainDefParseNode (xml=0x7da8c0, root=0x7dab80, caps=0x7caf10, xmlopt=0x7c82c0, flags=0) at conf/domain_conf.c:16582 #4 0x00007ffff294e424 in virDomainDefParse (xmlStr=0x0, filename=0x7c7ef0 "/home/zippy/work/libvirt/libvirt.git/tests/securityselinuxlabeldata/disks.xml", caps=0x7caf10, xmlopt=0x7c82c0, flags=0) at conf/domain_conf.c:16529 #5 0x00007ffff294e4b2 in virDomainDefParseFile (filename=0x7c7ef0 "/home/zippy/work/libvirt/libvirt.git/tests/securityselinuxlabeldata/disks.xml", caps=0x7caf10, xmlopt=0x7c82c0, flags=0) at conf/domain_conf.c:16553 #6 0x00000000004303ca in testSELinuxLoadDef (testname=0x53c929 "disks") at securityselinuxlabeltest.c:192 #7 0x00000000004309e8 in testSELinuxLabeling (opaque=0x53c929) at securityselinuxlabeltest.c:313 #8 0x0000000000431207 in virtTestRun (title=0x53c92f "Labelling \"disks\"", body=0x430964 <testSELinuxLabeling>, data=0x53c929) at testutils.c:211 #9 0x0000000000430c5d in mymain () at securityselinuxlabeltest.c:373 #10 0x00000000004325c2 in virtTestMain (argc=1, argv=0x7fffffffd7e8, func=0x430b4a <mymain>) at testutils.c:863 #11 0x0000000000430deb in main (argc=1, argv=0x7fffffffd7e8) at securityselinuxlabeltest.c:381 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemucapabilitiestest.c | 9 +++++---- tests/qemumonitorjsontest.c | 19 ++++++++++--------- tests/qemumonitortest.c | 9 +++++---- tests/securityselinuxlabeltest.c | 8 +++++--- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index 0fbc43c..99abd02 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -160,7 +160,7 @@ static int mymain(void) { int ret = 0; - virDomainXMLOptionPtr xmlopt; + virQEMUDriver driver; testQemuData data; #if !WITH_YAJL @@ -169,12 +169,12 @@ mymain(void) #endif if (virThreadInitialize() < 0 || - !(xmlopt = virQEMUDriverCreateXMLConf(NULL))) + qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; virEventRegisterDefaultImpl(); - data.xmlopt = xmlopt; + data.xmlopt = driver.xmlopt; #define DO_TEST(name) \ do { \ @@ -191,7 +191,8 @@ mymain(void) DO_TEST("caps_1.6.50-1"); DO_TEST("caps_2.1.1-1"); - virObjectUnref(xmlopt); + qemuTestDriverFree(&driver); + return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index ded0423..7d4741e 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2238,7 +2238,7 @@ static int mymain(void) { int ret = 0; - virDomainXMLOptionPtr xmlopt; + virQEMUDriver driver; testQemuMonitorJSONSimpleFuncData simpleFunc; #if !WITH_YAJL @@ -2247,29 +2247,30 @@ mymain(void) #endif if (virThreadInitialize() < 0 || - !(xmlopt = virQEMUDriverCreateXMLConf(NULL))) + qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; virEventRegisterDefaultImpl(); -#define DO_TEST(name) \ - if (virtTestRun(# name, testQemuMonitorJSON ## name, xmlopt) < 0) \ +#define DO_TEST(name) \ + if (virtTestRun(# name, testQemuMonitorJSON ## name, driver.xmlopt) < 0) \ ret = -1 #define DO_TEST_SIMPLE(CMD, FNC, ...) \ simpleFunc = (testQemuMonitorJSONSimpleFuncData) {.cmd = CMD, .func = FNC, \ - .xmlopt = xmlopt, __VA_ARGS__ }; \ + .xmlopt = driver.xmlopt, __VA_ARGS__ }; \ if (virtTestRun(# FNC, testQemuMonitorJSONSimpleFunc, &simpleFunc) < 0) \ ret = -1 #define DO_TEST_GEN(name, ...) \ - simpleFunc = (testQemuMonitorJSONSimpleFuncData) {.xmlopt = xmlopt, __VA_ARGS__ }; \ - if (virtTestRun(# name, testQemuMonitorJSON ## name, &simpleFunc) < 0) \ + simpleFunc = (testQemuMonitorJSONSimpleFuncData) {.xmlopt = driver.xmlopt, \ + __VA_ARGS__ }; \ + if (virtTestRun(# name, testQemuMonitorJSON ## name, &simpleFunc) < 0) \ ret = -1 #define DO_TEST_CPU_DATA(name) \ do { \ - struct testCPUData data = { name, xmlopt }; \ + struct testCPUData data = { name, driver.xmlopt }; \ const char *label = "GetCPUData(" name ")"; \ if (virtTestRun(label, testQemuMonitorJSONGetCPUData, &data) < 0) \ ret = -1; \ @@ -2347,7 +2348,7 @@ mymain(void) DO_TEST_CPU_DATA("host"); DO_TEST_CPU_DATA("full"); - virObjectUnref(xmlopt); + qemuTestDriverFree(&driver); return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemumonitortest.c b/tests/qemumonitortest.c index 12308f9..324021a 100644 --- a/tests/qemumonitortest.c +++ b/tests/qemumonitortest.c @@ -14,6 +14,7 @@ # include "qemu/qemu_monitor.h" # include "qemu/qemu_monitor_text.h" # include "qemumonitortestutils.h" +# include "testutilsqemu.h" # define VIR_FROM_THIS VIR_FROM_NONE @@ -164,11 +165,11 @@ testMonitorTextBlockInfo(const void *opaque) static int mymain(void) { - virDomainXMLOptionPtr xmlopt; + virQEMUDriver driver; int result = 0; if (virThreadInitialize() < 0 || - !(xmlopt = virQEMUDriverCreateXMLConf(NULL))) + qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; virEventRegisterDefaultImpl(); @@ -176,7 +177,7 @@ mymain(void) # define DO_TEST(_name) \ do { \ if (virtTestRun("qemu monitor "#_name, test##_name, \ - xmlopt) < 0) { \ + driver.xmlopt) < 0) { \ result = -1; \ } \ } while (0) @@ -185,7 +186,7 @@ mymain(void) DO_TEST(UnescapeArg); DO_TEST(MonitorTextBlockInfo); - virObjectUnref(xmlopt); + qemuTestDriverFree(&driver); return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index 4808eea..4b8fa6f 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -46,7 +46,7 @@ VIR_LOG_INIT("tests.securityselinuxlabeltest"); static virCapsPtr caps; -static virDomainXMLOptionPtr xmlopt; +static virQEMUDriver driver; static virSecurityManagerPtr mgr; @@ -189,7 +189,7 @@ testSELinuxLoadDef(const char *testname) abs_srcdir, testname) < 0) goto cleanup; - if (!(def = virDomainDefParseFile(xmlfile, caps, xmlopt, 0))) + if (!(def = virDomainDefParseFile(xmlfile, caps, driver.xmlopt, 0))) goto cleanup; for (i = 0; i < def->ndisks; i++) { @@ -361,7 +361,7 @@ mymain(void) if ((caps = testQemuCapsInit()) == NULL) return EXIT_FAILURE; - if (!(xmlopt = virQEMUDriverCreateXMLConf(NULL))) + if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; #define DO_TEST_LABELING(name) \ @@ -375,6 +375,8 @@ mymain(void) DO_TEST_LABELING("chardev"); DO_TEST_LABELING("nfs"); + qemuTestDriverFree(&driver); + return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.4.9
On Tue, Sep 22, 2015 at 04:39:03PM +0200, Michal Privoznik wrote:
I've noticed a few tests failing strangely after [1] was merged. Here are the fixes.
1: https://www.redhat.com/archives/libvir-list/2015-September/msg00460.html
Michal Privoznik (2): qemuTestDriverInit: init the driver lock too tests: Avoid use of virQEMUDriverCreateXMLConf(NULL)
tests/qemucapabilitiestest.c | 9 +++++---- tests/qemumonitorjsontest.c | 19 ++++++++++--------- tests/qemumonitortest.c | 9 +++++---- tests/securityselinuxlabeltest.c | 8 +++++--- tests/testutilsqemu.c | 6 +++++- 5 files changed, 30 insertions(+), 21 deletions(-)
ACK Jan
participants (2)
-
Ján Tomko -
Michal Privoznik