[v1] tests: add further XML namespace test

[libvirt] [PATCH] tests: add further XML namespace test

Posted by Daniel P. Berrange 7 years, 10 months ago

Validate that we can pass QEMU command line options using a default
namespace, instead of a prefixed namespace

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 .../qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.args | 25 ++++++++++++++++++
 .../qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml  | 30 ++++++++++++++++++++++
 tests/qemuxml2argvtest.c                           |  1 +
 3 files changed, 56 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.args b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.args
new file mode 100644
index 0000000..562b562
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.args
@@ -0,0 +1,25 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+NS=ns \
+BAR='' \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
+-unknown parameter
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml
new file mode 100644
index 0000000..491fc2d
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml
@@ -0,0 +1,30 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='ide' index='0'/>
+  </devices>
+  <commandline xmlns='http://libvirt.org/schemas/domain/qemu/1.0'>
+    <arg value='-unknown'/>
+    <arg value='parameter'/>
+    <env name='NS' value='ns'/>
+    <env name='BAR'/>
+  </commandline>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 27eea70..be2a9bc 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1505,6 +1505,7 @@ mymain(void)
 
     DO_TEST("qemu-ns", NONE);
     DO_TEST("qemu-ns-no-env", NONE);
+    DO_TEST("qemu-ns-alt", NONE);
 
     DO_TEST("smp", NONE);
 
-- 
2.9.4

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

Re: [libvirt] [PATCH] tests: add further XML namespace test

Posted by Andrea Bolognani 7 years, 10 months ago

On Mon, 2017-07-10 at 11:14 +0100, Daniel P. Berrange wrote:
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml
> @@ -0,0 +1,30 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219136</memory>
> +  <currentMemory unit='KiB'>219136</currentMemory>

<currentMemory> can be left out.

> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>

<boot> is unnecessary.

> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>

<clock> and <on_*> can be removed as well.

> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i686</emulator>
> +    <disk type='block' device='disk'>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> +    </disk>
> +    <controller type='ide' index='0'/>

You can get rid of <disk> and <controller> too, then add
<controller type='usb' model='none'/> and
<memballoon model='none'/> to avoid unnecessary devices
popping up in the output file.

> +++ b/tests/qemuxml2argvtest.c
> @@ -1505,6 +1505,7 @@ mymain(void)
>  
>      DO_TEST("qemu-ns", NONE);
>      DO_TEST("qemu-ns-no-env", NONE);
> +    DO_TEST("qemu-ns-alt", NONE);

You'll want to add this to qemuxml2xmltest too.


Consider the patch

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

if you address at least the last point.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH] tests: add further XML namespace test

Posted by Daniel P. Berrange 7 years, 10 months ago

On Mon, Jul 10, 2017 at 12:52:32PM +0200, Andrea Bolognani wrote:
> On Mon, 2017-07-10 at 11:14 +0100, Daniel P. Berrange wrote:
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-alt.xml
> > @@ -0,0 +1,30 @@
> > +<domain type='qemu'>
> > +  <name>QEMUGuest1</name>
> > +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> > +  <memory unit='KiB'>219136</memory>
> > +  <currentMemory unit='KiB'>219136</currentMemory>
> 
> <currentMemory> can be left out.
> 
> > +  <vcpu placement='static'>1</vcpu>
> > +  <os>
> > +    <type arch='i686' machine='pc'>hvm</type>
> > +    <boot dev='hd'/>
> 
> <boot> is unnecessary.
> 
> > +  </os>
> > +  <clock offset='utc'/>
> > +  <on_poweroff>destroy</on_poweroff>
> > +  <on_reboot>restart</on_reboot>
> > +  <on_crash>destroy</on_crash>
> 
> <clock> and <on_*> can be removed as well.
> 
> > +  <devices>
> > +    <emulator>/usr/bin/qemu-system-i686</emulator>
> > +    <disk type='block' device='disk'>
> > +      <source dev='/dev/HostVG/QEMUGuest1'/>
> > +      <target dev='hda' bus='ide'/>
> > +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> > +    </disk>
> > +    <controller type='ide' index='0'/>
> 
> You can get rid of <disk> and <controller> too, then add
> <controller type='usb' model='none'/> and
> <memballoon model='none'/> to avoid unnecessary devices
> popping up in the output file.

FYI, this XML file is identical to the pre-existing qemuxml2argv-qemu-ns.xml
file, so I'm inclined to leave it as is

> 
> > +++ b/tests/qemuxml2argvtest.c
> > @@ -1505,6 +1505,7 @@ mymain(void)
> >  
> >      DO_TEST("qemu-ns", NONE);
> >      DO_TEST("qemu-ns-no-env", NONE);
> > +    DO_TEST("qemu-ns-alt", NONE);
> 
> You'll want to add this to qemuxml2xmltest too.

This wont round trip - it'll end up outputing the canonical xmlns syntax
instead, for which we already have output tests.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] tests: add further XML namespace test

Posted by Andrea Bolognani 7 years, 10 months ago

On Tue, 2017-07-11 at 14:00 +0100, Daniel P. Berrange wrote:
> > You can get rid of <disk> and <controller> too, then add
> > <controller type='usb' model='none'/> and
> > <memballoon model='none'/> to avoid unnecessary devices
> > popping up in the output file.
> 
> FYI, this XML file is identical to the pre-existing qemuxml2argv-qemu-ns.xml
> file, so I'm inclined to leave it as is

I'm aware of that, that's why I said that my R-b would stand
whether or not you decide to address these points.

That said, I'd really prefer it if we started being more
to the point with our test data and really only include the
relevant information rather than copy the same pointless
bits over and over again.

Here's a counter-offer: if you agree to follow my advice
above and use a minimal input file, I'll post a patch that
strips the other qemu-ns*.xml from unnecessary cruft. Does
that sound fair? :)

> > > +++ b/tests/qemuxml2argvtest.c
> > > @@ -1505,6 +1505,7 @@ mymain(void)
> > >   
> > >       DO_TEST("qemu-ns", NONE);
> > >       DO_TEST("qemu-ns-no-env", NONE);
> > > +    DO_TEST("qemu-ns-alt", NONE);
> > 
> > You'll want to add this to qemuxml2xmltest too.
> 
> This wont round trip - it'll end up outputing the canonical xmlns syntax
> instead, for which we already have output tests.

Hence proving that the alternative syntax and the canonical
one result in the same output file. Why wouldn't we want to
test that?

-- 
Andrea Bolognani / Red Hat / Virtualization

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