>>> On 12/23/2014 at 09:42 AM, in message <5498C888.6000903@xxxxxxxx>, Jim Fehlig
<jfehlig@xxxxxxxx> 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@xxxxxxxx>
> > ---
> > .../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.
- Chunyan
> Was
> there any reason for test-fullvirt-default-feature.cfg vs
> test-fullvirt-default-feature.cfg?
>
>
> Regards,
> Jim
>
> >
> > return result;
> > }
> > @@ -254,6 +259,8 @@ mymain(void)
> > DO_TEST("fullvirt-net-ioemu", 2);
> > DO_TEST("fullvirt-net-netfront", 2);
> >
> > + DO_TEST("fullvirt-default-feature", 2);
> > +
> > DO_TEST("escape-paths", 2);
> > DO_TEST("no-source-cdrom", 2);
> > DO_TEST("pci-devs", 2);
> >
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list