[libvirt] [PATCH 2/2] Add tests to xmconfigtest
Chun Yan Liu
cyliu at suse.com
Tue Dec 23 03:19:03 UTC 2014
>>> On 12/23/2014 at 11:13 AM, in message <5498DDCC.2020002 at suse.com>, Jim Fehlig
<jfehlig at suse.com> wrote:
> 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.
That's a good idea. I'll update.
Thanks very much for reviewing the patch in your holiday!
- Chunyan
>
> Regards,
> Jim
>
>
More information about the libvir-list
mailing list