Re: [libvirt] [RFC] [PATCH] Fix flawed testcases in qemuhotplugtest.c
On 19.03.2014 00:08, Nehal J Wani wrote:
While running qemuhotplugtest, it was found that valgrind pointed
out
the following memory leak:
==7906== 5 bytes in 1 blocks are definitely lost in loss record 7 of 121
==7906== at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==7906== by 0x3E782A754D: xmlStrndup (in /usr/lib64/libxml2.so.2.7.6)
==7906== by 0x4CDAE03: virDomainDeviceInfoParseXML.isra.32 (domain_conf.c:3685)
==7906== by 0x4CE3BB9: virDomainNetDefParseXML (domain_conf.c:6707)
==7906== by 0x4CFBA08: virDomainDefParseXML (domain_conf.c:12235)
==7906== by 0x4CFBC1E: virDomainDefParseNode (domain_conf.c:13039)
==7906== by 0x4CFBD95: virDomainDefParse (domain_conf.c:12981)
==7906== by 0x41FEB4: testQemuHotplug (qemuhotplugtest.c:66)
==7906== by 0x420F41: virtTestRun (testutils.c:201)
==7906== by 0x41F287: mymain (qemuhotplugtest.c:422)
==7906== by 0x4216BD: virtTestMain (testutils.c:784)
==7906== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
...and 10 more.
The 'alias' attribute should *not* be parsed from the XML provided by
the user. It should only be parsed in the live state XML. In the latter
case no codepath should take us to qemuAssignDeviceAliases (which, in
this case, is causing the above leaks).
You're right, in case of domains, alias should not be parsed from user supplied XMLs.
But the XMLs used in the test are not user supplied ones. In fact, we want domain to
contain all information that a running domain does (e.g. all aliases) - so for the hotplug
code it looks like domain is running (coldplug code just don't care).
Having said that, I agree that there's no need to have aliases in the XMLs but
generate them on the fly. That's what is happening now, well since 20745748 which
introduced the generation at runtime.
So I guess your comment is not quite correct.
---
Caused by the test cases:
DO_TEST_ATTACH("console-compat-2", "console-virtio", false,
true,
"chardev-add", "{\"return\":
{\"pty\": \"/dev/pts/26\"}}",
"device_add", QMP_OK);
DO_TEST_DETACH("console-compat-2", "console-virtio", false,
false,
"device_del", QMP_OK,
"chardev-remove", QMP_OK);
Refer: http://www.redhat.com/archives/libvir-list/2013-November/msg01187.html
...qemuhotplug-console-compat-2+console-virtio.xml | 34 +++-----------------
.../qemuhotplug-console-virtio.xml | 1 -
.../qemuxml2argv-console-compat-2.xml | 32 +++---------------
3 files changed, 10 insertions(+), 57 deletions(-)
The patch is incomplete. We need this as well:
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 8036adc..4ef81e0 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -67,7 +67,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
driver.caps,
driver.xmlopt,
QEMU_EXPECTED_VIRT_TYPES,
- 0)))
+ VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
priv = (*vm)->privateData;
Anyway, I would not call the testcases flawed, just mem-leaking. There's still one
mem-leak that I'll investigate further.
ACK with the qemuhotplugtest change squashed in. I'm rewording both the subject and
commit message and pushing this in. Thanks for sorting this out.
Michal