Re: [libvirt] [PATCH 2/2] Add tests to xmconfigtest

>>> On 12/23/2014 at 09:42 AM, in message <5498C888.6000903(a)suse.com&gt;, Jim Fehlig >>> <jfehlig(a)suse.com&gt; wrote: > Chunyan Liu wrote: > > Hi Chunyan, > > Thanks for the fix, and the test! But I have a few questions regarding > the latter... > > >> Add tests to testing HVM default features (pae, acpi, apic) >> conversion from xm config to libvirt xml and from libvirt >> xml to xm config. >> >> Signed-off-by: Chunyan Liu <cyliu(a)suse.com&gt; >> --- >> .../xmconfigdata/test-fullvirt-default-feature.cfg | 23 +++++++++++ >> .../test-fullvirt-default-feature.cfg.out | 26 ++++++++++++ >> .../xmconfigdata/test-fullvirt-default-feature.xml | 48 >> > ++++++++++++++++++++++ > >> tests/xmconfigtest.c | 9 +++- >> 4 files changed, 105 insertions(+), 1 deletion(-) >> create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.cfg >> create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.cfg.out >> create mode 100644 tests/xmconfigdata/test-fullvirt-default-feature.xml >> >> diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.cfg >> > b/tests/xmconfigdata/test-fullvirt-default-feature.cfg > >> new file mode 100644 >> index 0000000..5ce234f >> --- /dev/null >> +++ b/tests/xmconfigdata/test-fullvirt-default-feature.cfg >> >> > > Why is this file needed? > " Here we are testing default value conversion. That is: if there is no apci/pae/apic specified in xm config, after conversion, libvirt xml should include: <features> <apic/> <acpi/> <pae/> </features>

So, from xm config -> xml, the cfg file should be like this. > > >> @@ -0,0 +1,23 @@ >> +name = "XenGuest2" >> +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809 >> +maxmem = 579 >> +memory = 394 >> +vcpus = 1 >> +builder = "hvm" >> +kernel = "/usr/lib/xen/boot/hvmloader" >> +boot = "d" >> +hpet = 1 >> +localtime = 0 >> +on_poweroff = "destroy" >> +on_reboot = "restart" >> +on_crash = "restart" >> +device_model = "/usr/lib/xen/bin/qemu-dm" >> +sdl = 0 >> +vnc = 1 >> +vncunused = 1 >> +vnclisten = "127.0.0.1" >> +vncpasswd = "123poi" >> +vif = [ >> > "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioem > u" ] > >> +parallel = "none" >> +serial = "none" >> +disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", >> > "file:/root/boot.iso,hdc:cdrom,r" ] > >> diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out >> > b/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out > >> new file mode 100644 >> index 0000000..97a9827 >> --- /dev/null >> +++ b/tests/xmconfigdata/test-fullvirt-default-feature.cfg.out >> >> > > IMO, this file should be renamed to 'test-fullvirt-default-feature.cfg'. > And from xml -> xm config, if in xml there is: <features> <apic/> <acpi/> <pae/> </features> Then after conversion, in xm config out file there will be explicitly: acpi=1 apic=1 pae=1 So, thlis is a little different from test-fullvirt-default-feature.cfg.

> > >> @@ -0,0 +1,26 @@ >> +name = "XenGuest2" >> +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" >> +maxmem = 579 >> +memory = 394 >> +vcpus = 1 >> +builder = "hvm" >> +kernel = "/usr/lib/xen/boot/hvmloader" >> +boot = "d" >> +pae = 1 >> +acpi = 1 >> +apic = 1 >> +hpet = 1 >> +localtime = 0 >> +on_poweroff = "destroy" >> +on_reboot = "restart" >> +on_crash = "restart" >> +device_model = "/usr/lib/xen/bin/qemu-dm" >> +sdl = 0 >> +vnc = 1 >> +vncunused = 1 >> +vnclisten = "127.0.0.1" >> +vncpasswd = "123poi" >> +vif = [ >> > "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioem > u" ] > >> +parallel = "none" >> +serial = "none" >> +disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", >> > "file:/root/boot.iso,hdc:cdrom,r" ] > >> diff --git a/tests/xmconfigdata/test-fullvirt-default-feature.xml >> > b/tests/xmconfigdata/test-fullvirt-default-feature.xml > >> new file mode 100644 >> index 0000000..57a6531 >> --- /dev/null >> +++ b/tests/xmconfigdata/test-fullvirt-default-feature.xml >> @@ -0,0 +1,48 @@ >> +<domain type='xen'> >> + <name>XenGuest2</name> >> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> >> + <memory unit='KiB'>592896</memory> >> + <currentMemory unit='KiB'>403456</currentMemory> >> + <vcpu placement='static'>1</vcpu> >> + <os> >> + <type arch='i686' machine='xenfv'>hvm</type> >> + <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader> >> + <boot dev='cdrom'/> >> + </os> >> + <features> >> + <acpi/> >> + <apic/> >> + <pae/> >> + </features> >> + <clock offset='utc' adjustment='reset'> >> + <timer name='hpet' present='yes'/> >> + </clock> >> + <on_poweroff>destroy</on_poweroff> >> + <on_reboot>restart</on_reboot> >> + <on_crash>restart</on_crash> >> + <devices> >> + <emulator>/usr/lib/xen/bin/qemu-dm</emulator> >> + <disk type='block' device='disk'> >> + <driver name='phy'/> >> + <source dev='/dev/HostVG/XenGuest2'/> >> + <target dev='hda' bus='ide'/> >> + </disk> >> + <disk type='file' device='cdrom'> >> + <driver name='file'/> >> + <source file='/root/boot.iso'/> >> + <target dev='hdc' bus='ide'/> >> + <readonly/> >> + </disk> >> + <interface type='bridge'> >> + <mac address='00:16:3e:66:92:9c'/> >> + <source bridge='xenbr1'/> >> + <script path='vif-bridge'/> >> + <model type='e1000'/> >> + </interface> >> + <input type='mouse' bus='ps2'/> >> + <input type='keyboard' bus='ps2'/> >> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1' >> > passwd='123poi'> > >> + <listen type='address' address='127.0.0.1'/> >> + </graphics> >> + </devices> >> +</domain> >> diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c >> index 0c6f803..b0b7b30 100644 >> --- a/tests/xmconfigtest.c >> +++ b/tests/xmconfigtest.c >> @@ -176,21 +176,26 @@ testCompareHelper(const void *data) >> const struct testInfo *info = data; >> char *xml = NULL; >> char *cfg = NULL; >> + char *cfgout = NULL; >> >> if (virAsprintf(&xml, "%s/xmconfigdata/test-%s.xml", >> abs_srcdir, info->name) < 0 || >> virAsprintf(&cfg, "%s/xmconfigdata/test-%s.cfg", >> + abs_srcdir, info->name) < 0 || >> + virAsprintf(&cfgout, "%s/xmconfigdata/test-%s.cfg.out", >> abs_srcdir, info->name) < 0) >> goto cleanup; >> >> if (info->mode == 0) >> - result = testCompareParseXML(cfg, xml, info->version); >> + result = testCompareParseXML(virFileExists(cfgout) ? cfgout : cfg, >> + xml, info->version); >> else >> result = testCompareFormatXML(cfg, xml, info->version); >> >> cleanup: >> VIR_FREE(xml); >> VIR_FREE(cfg); >> + VIR_FREE(cfgout); >> >> > > With the change suggested above, this hunk can be removed from > xmconfigtest.c. 'make check' passes for me with these changes. > 'make check' could pass, since it explicitly specify acpi|pae|apic=1 in xm config, and explicitly include those features in xml. But our interested case is not tested, what we are trying to test is: if user doesn't specify acpi|pae|apic, xml should by default include those features.