[libvirt] [RFC] [PATCH] Fix flawed testcases in qemuhotplugtest.c
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). --- 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(-) diff --git a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml index 25fc120..ec1c6e8 100644 --- a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml +++ b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml @@ -1,4 +1,4 @@ -<domain type='kvm' id='2'> +<domain type='kvm'> <name>f17</name> <uuid>a1cd52eb-d37f-4717-fc6e-972f0774f4c9</uuid> <memory unit='KiB'>1048576</memory> @@ -30,7 +30,6 @@ <driver name='qemu' type='qcow2' cache='none'/> <source file='/var/lib/libvirt/images/f17.qcow2'/> <target dev='vda' bus='virtio'/> - <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </disk> <disk type='file' device='cdrom'> @@ -38,22 +37,16 @@ <source file='/home/user/tmp/Fedora-17-x86_64-Live-KDE.iso'/> <target dev='hdc' bus='ide'/> <readonly/> - <alias name='ide0-1-0'/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <controller type='ide' index='0'> - <alias name='ide0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <controller type='usb' index='0'> - <alias name='usb0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='pci' index='0' model='pci-root'> - <alias name='pci.0'/> - </controller> + <controller type='pci' index='0' model='pci-root'/> <controller type='virtio-serial' index='0'> - <alias name='virtio-serial0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </controller> <interface type='network'> @@ -63,62 +56,45 @@ <inbound average='4000' peak='8000' floor='200' burst='1024'/> <outbound average='4000' peak='8000' burst='1024'/> </bandwidth> - <target dev='vnet0'/> <model type='virtio'/> - <alias name='net0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> - <source path='/dev/pts/22'/> <target type='isa-serial' port='0'/> - <alias name='serial0'/> </serial> <serial type='pty'> - <source path='/dev/pts/25'/> <target port='0'/> - <alias name='serial1'/> </serial> <serial type='tcp'> <source mode='bind' host='0.0.0.0' service='2445'/> <protocol type='raw'/> <target port='1'/> - <alias name='serial2'/> </serial> - <console type='pty' tty='/dev/pts/22'> - <source path='/dev/pts/22'/> + <console type='pty'> <target type='serial' port='0'/> - <alias name='serial0'/> </console> <console type='pty'> - <source path='/dev/pts/26'/> <target type='virtio' port='1'/> - <alias name='console1'/> </console> <channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f17x86_64.agent'/> <target type='virtio' name='org.qemu.guest_agent.0'/> - <alias name='channel0'/> <address type='virtio-serial' controller='0' bus='0' port='1'/> </channel> - <input type='tablet' bus='usb'> - <alias name='input0'/> - </input> + <input type='tablet' bus='usb'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <graphics type='vnc' port='5900' autoport='yes' listen='0.0.0.0'> + <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'> <listen type='address' address='0.0.0.0'/> </graphics> <sound model='ich6'> - <alias name='sound0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </sound> <video> <model type='cirrus' vram='9216' heads='1'/> - <alias name='video0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> - <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </memballoon> </devices> diff --git a/tests/qemuhotplugtestdata/qemuhotplug-console-virtio.xml b/tests/qemuhotplugtestdata/qemuhotplug-console-virtio.xml index bfab6ff..3eb2be9 100644 --- a/tests/qemuhotplugtestdata/qemuhotplug-console-virtio.xml +++ b/tests/qemuhotplugtestdata/qemuhotplug-console-virtio.xml @@ -1,5 +1,4 @@ <console type='pty'> <source path='/dev/pts/26'/> <target type='virtio' port='1'/> - <alias name='console1'/> </console> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml index e5c45a3..4d4ac47 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml @@ -1,4 +1,4 @@ -<domain type='kvm' id='2'> +<domain type='kvm'> <name>f17</name> <uuid>a1cd52eb-d37f-4717-fc6e-972f0774f4c9</uuid> <memory unit='KiB'>1048576</memory> @@ -30,7 +30,6 @@ <driver name='qemu' type='qcow2' cache='none'/> <source file='/var/lib/libvirt/images/f17.qcow2'/> <target dev='vda' bus='virtio'/> - <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </disk> <disk type='file' device='cdrom'> @@ -38,22 +37,16 @@ <source file='/home/user/tmp/Fedora-17-x86_64-Live-KDE.iso'/> <target dev='hdc' bus='ide'/> <readonly/> - <alias name='ide0-1-0'/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <controller type='ide' index='0'> - <alias name='ide0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> </controller> <controller type='usb' index='0'> - <alias name='usb0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> - <controller type='pci' index='0' model='pci-root'> - <alias name='pci.0'/> - </controller> + <controller type='pci' index='0' model='pci-root'/> <controller type='virtio-serial' index='0'> - <alias name='virtio-serial0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </controller> <interface type='network'> @@ -63,57 +56,42 @@ <inbound average='4000' peak='8000' floor='200' burst='1024'/> <outbound average='4000' peak='8000' burst='1024'/> </bandwidth> - <target dev='vnet0'/> <model type='virtio'/> - <alias name='net0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> - <source path='/dev/pts/22'/> <target type='isa-serial' port='0'/> - <alias name='serial0'/> </serial> <serial type='pty'> - <source path='/dev/pts/25'/> <target port='0'/> - <alias name='serial1'/> </serial> <serial type='tcp'> <source mode='bind' host='0.0.0.0' service='2445'/> <protocol type='raw'/> <target port='1'/> - <alias name='serial2'/> </serial> - <console type='pty' tty='/dev/pts/22'> - <source path='/dev/pts/22'/> + <console type='pty'> <target type='serial' port='0'/> - <alias name='serial0'/> </console> <channel type='unix'> <source mode='bind' path='/var/lib/libvirt/qemu/f17x86_64.agent'/> <target type='virtio' name='org.qemu.guest_agent.0'/> - <alias name='channel0'/> <address type='virtio-serial' controller='0' bus='0' port='1'/> </channel> - <input type='tablet' bus='usb'> - <alias name='input0'/> - </input> + <input type='tablet' bus='usb'/> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <graphics type='vnc' port='5900' autoport='yes' listen='0.0.0.0'> + <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'> <listen type='address' address='0.0.0.0'/> </graphics> <sound model='ich6'> - <alias name='sound0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </sound> <video> <model type='cirrus' vram='9216' heads='1'/> - <alias name='video0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> - <alias name='balloon0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </memballoon> </devices> -- 1.7.1
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
participants (2)
-
Michal Privoznik -
Nehal J Wani