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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>>> On 12/23/2014 at 11:13 AM, in message <5498DDCC.2020002@xxxxxxxx>, Jim Fehlig
<jfehlig@xxxxxxxx> wrote: 
> Chun Yan Liu wrote: 
> >    
> >>>> 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> 
> >    
>  
> 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 
>  
>  


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]