[libvirt] [PATCH 2/2] Add tests to xmconfigtest
Jim Fehlig
jfehlig at suse.com
Tue Dec 23 03:13:16 UTC 2014
Chun Yan Liu wrote:
>
>>>> On 12/23/2014 at 09:42 AM, in message <5498C888.6000903 at suse.com>, Jim Fehlig
>>>>
> <jfehlig at suse.com> 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 at suse.com>
>>> ---
>>> .../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>
>
Ah, ok. Agreed that this test is missing.
> 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.
>
This is actually tested in all of the other test-fullvirt-* tests. I
don't think we need an explicit test for it.
>
>>
>>
>>> @@ -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.
>
Yep, understood. Unlike the existing tests, in this case we only want
to test xm -> xml conversion. I think a better solution would be to
introduce DO_TEST_PARSE and DO_TEST_FORMAT macros, calling those in
DO_TEST and individually when only needing to test one conversion.
Regards,
Jim
More information about the libvir-list
mailing list