1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/3] tests: Clean up qemuxml2xml a bit
  4. View patch

[libvirt] [PATCH 2/3] tests: Use WHEN_BOTH in qemuxml2xml whenever possible

Andrea Bolognani posted 3 patches 6 years, 11 months ago
Only 2 patches received!
[libvirt] [PATCH 2/3] tests: Use WHEN_BOTH in qemuxml2xml whenever possible
Posted by Andrea Bolognani 6 years, 11 months ago 
There are a few cases where we are using either WHEN_ACTIVE
or WHEN_INACTIVE even though WHEN_BOTH would work perfectly
fine: for those, start using the simpler DO_TEST() macro.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 ...ource-path.xml => channel-unix-source-path-active.xml} | 3 +++
 ...rce-path.xml => channel-unix-source-path-inactive.xml} | 0
 ...sk-active-commit.xml => disk-active-commit-active.xml} | 0
 ...-active-commit.xml => disk-active-commit-inactive.xml} | 8 ++------
 ...labelskip.xml => seclabel-static-labelskip-active.xml} | 0
 ...belskip.xml => seclabel-static-labelskip-inactive.xml} | 7 ++-----
 tests/qemuxml2xmltest.c                                   | 6 +++---
 7 files changed, 10 insertions(+), 14 deletions(-)
 copy tests/qemuxml2xmloutdata/{channel-unix-source-path.xml => channel-unix-source-path-active.xml} (86%)
 rename tests/qemuxml2xmloutdata/{channel-unix-source-path.xml => channel-unix-source-path-inactive.xml} (100%)
 copy tests/qemuxml2xmloutdata/{disk-active-commit.xml => disk-active-commit-active.xml} (100%)
 rename tests/qemuxml2xmloutdata/{disk-active-commit.xml => disk-active-commit-inactive.xml} (86%)
 copy tests/qemuxml2xmloutdata/{seclabel-static-labelskip.xml => seclabel-static-labelskip-active.xml} (100%)
 rename tests/qemuxml2xmloutdata/{seclabel-static-labelskip.xml => seclabel-static-labelskip-inactive.xml} (86%)

diff --git a/tests/qemuxml2xmloutdata/channel-unix-source-path.xml b/tests/qemuxml2xmloutdata/channel-unix-source-path-active.xml
similarity index 86%
copy from tests/qemuxml2xmloutdata/channel-unix-source-path.xml
copy to tests/qemuxml2xmloutdata/channel-unix-source-path-active.xml
index 2d8fa1e3ac..4c2a02b49a 100644
--- a/tests/qemuxml2xmloutdata/channel-unix-source-path.xml
+++ b/tests/qemuxml2xmloutdata/channel-unix-source-path-active.xml
@@ -22,14 +22,17 @@
       <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
     </controller>
     <channel type='unix'>
+      <source mode='bind' path='/tmp/channel/QEMUGuest1.org.qemu.guest_agent.0'/>
       <target type='virtio' name='org.qemu.guest_agent.0'/>
       <address type='virtio-serial' controller='0' bus='0' port='1'/>
     </channel>
     <channel type='unix'>
+      <source mode='bind' path='/tmp/channel/domain-QEMUGuest1/org.qemu.guest_agent.1'/>
       <target type='virtio' name='org.qemu.guest_agent.1'/>
       <address type='virtio-serial' controller='0' bus='0' port='2'/>
     </channel>
     <channel type='unix'>
+      <source mode='bind' path='/tmp/channel/domain-1-QEMUGuest1/org.qemu.guest_agent.2'/>
       <target type='virtio' name='org.qemu.guest_agent.2'/>
       <address type='virtio-serial' controller='0' bus='0' port='3'/>
     </channel>
diff --git a/tests/qemuxml2xmloutdata/channel-unix-source-path.xml b/tests/qemuxml2xmloutdata/channel-unix-source-path-inactive.xml
similarity index 100%
rename from tests/qemuxml2xmloutdata/channel-unix-source-path.xml
rename to tests/qemuxml2xmloutdata/channel-unix-source-path-inactive.xml
diff --git a/tests/qemuxml2xmloutdata/disk-active-commit.xml b/tests/qemuxml2xmloutdata/disk-active-commit-active.xml
similarity index 100%
copy from tests/qemuxml2xmloutdata/disk-active-commit.xml
copy to tests/qemuxml2xmloutdata/disk-active-commit-active.xml
diff --git a/tests/qemuxml2xmloutdata/disk-active-commit.xml b/tests/qemuxml2xmloutdata/disk-active-commit-inactive.xml
similarity index 86%
rename from tests/qemuxml2xmloutdata/disk-active-commit.xml
rename to tests/qemuxml2xmloutdata/disk-active-commit-inactive.xml
index cc26af1096..b14345049d 100644
--- a/tests/qemuxml2xmloutdata/disk-active-commit.xml
+++ b/tests/qemuxml2xmloutdata/disk-active-commit-inactive.xml
@@ -1,4 +1,4 @@
-<domain type='qemu' id='1'>
+<domain type='qemu'>
   <name>QEMUGuest1</name>
   <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
   <memory unit='KiB'>219136</memory>
@@ -17,15 +17,11 @@
     <disk type='file' device='disk'>
       <driver name='qemu' type='qcow2'/>
       <source file='/tmp/HostVG/QEMUGuest1-snap'/>
-      <backingStore type='block' index='1'>
+      <backingStore type='block'>
         <format type='raw'/>
         <source dev='/dev/HostVG/QEMUGuest1'/>
         <backingStore/>
       </backingStore>
-      <mirror type='block' job='active-commit'>
-        <format type='raw'/>
-        <source dev='/dev/HostVG/QEMUGuest1'/>
-      </mirror>
       <target dev='hda' bus='ide'/>
       <address type='drive' controller='0' bus='0' target='0' unit='0'/>
     </disk>
diff --git a/tests/qemuxml2xmloutdata/seclabel-static-labelskip.xml b/tests/qemuxml2xmloutdata/seclabel-static-labelskip-active.xml
similarity index 100%
copy from tests/qemuxml2xmloutdata/seclabel-static-labelskip.xml
copy to tests/qemuxml2xmloutdata/seclabel-static-labelskip-active.xml
diff --git a/tests/qemuxml2xmloutdata/seclabel-static-labelskip.xml b/tests/qemuxml2xmloutdata/seclabel-static-labelskip-inactive.xml
similarity index 86%
rename from tests/qemuxml2xmloutdata/seclabel-static-labelskip.xml
rename to tests/qemuxml2xmloutdata/seclabel-static-labelskip-inactive.xml
index 7c448675e2..e705b5452c 100644
--- a/tests/qemuxml2xmloutdata/seclabel-static-labelskip.xml
+++ b/tests/qemuxml2xmloutdata/seclabel-static-labelskip-inactive.xml
@@ -1,4 +1,4 @@
-<domain type='qemu' id='1'>
+<domain type='qemu'>
   <name>QEMUGuest1</name>
   <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
   <memory unit='KiB'>219100</memory>
@@ -16,9 +16,7 @@
     <emulator>/usr/bin/qemu-system-i686</emulator>
     <disk type='block' device='disk'>
       <driver name='qemu' type='raw'/>
-      <source dev='/dev/HostVG/QEMUGuest1'>
-        <seclabel model='selinux' labelskip='yes'/>
-      </source>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
       <backingStore/>
       <target dev='hda' bus='ide'/>
       <address type='drive' controller='0' bus='0' target='0' unit='0'/>
@@ -38,6 +36,5 @@
   </devices>
   <seclabel type='static' model='selinux' relabel='yes'>
     <label>system_u:system_r:svirt_custom_t:s0:c192,c392</label>
-    <imagelabel>system_u:system_r:svirt_custom_t:s0:c192,c392</imagelabel>
   </seclabel>
 </domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index b38cbd6994..59535e0f19 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -375,7 +375,7 @@ mymain(void)
             QEMU_CAPS_CCW);
     DO_TEST("disk-mirror-old", NONE);
     DO_TEST("disk-mirror", NONE);
-    DO_TEST_FULL("disk-active-commit", WHEN_ACTIVE, GIC_NONE, NONE);
+    DO_TEST("disk-active-commit", NONE);
     DO_TEST("graphics-listen-network", NONE);
     DO_TEST("graphics-vnc", NONE);
     DO_TEST("graphics-vnc-websocket", NONE);
@@ -451,7 +451,7 @@ mymain(void)
     DO_TEST("channel-virtio", NONE);
     DO_TEST("channel-virtio-state", NONE);
 
-    DO_TEST_FULL("channel-unix-source-path", WHEN_INACTIVE, GIC_NONE, NONE);
+    DO_TEST("channel-unix-source-path", NONE);
 
     DO_TEST("hostdev-usb-address", NONE);
     DO_TEST("hostdev-pci-address", NONE);
@@ -549,7 +549,7 @@ mymain(void)
     DO_TEST_FULL("seclabel-dynamic-labelskip", WHEN_INACTIVE, GIC_NONE, NONE);
     DO_TEST_FULL("seclabel-dynamic-relabel", WHEN_INACTIVE, GIC_NONE, NONE);
     DO_TEST("seclabel-static", NONE);
-    DO_TEST_FULL("seclabel-static-labelskip", WHEN_ACTIVE, GIC_NONE, NONE);
+    DO_TEST("seclabel-static-labelskip", NONE);
     DO_TEST("seclabel-none", NONE);
     DO_TEST("seclabel-dac-none", NONE);
     DO_TEST("seclabel-dynamic-none", NONE);
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] tests: Use WHEN_BOTH in qemuxml2xml whenever possible
Posted by Ján Tomko 6 years, 11 months ago 
On Thu, Feb 28, 2019 at 05:38:46PM +0100, Andrea Bolognani wrote:
>There are a few cases where we are using either WHEN_ACTIVE
>or WHEN_INACTIVE even though WHEN_BOTH would work perfectly
>fine: for those, start using the simpler DO_TEST() macro.

The goal of those test cases is to test some specific feature,
not to use the simplest macros.

>
>Signed-off-by: Andrea Bolognani <abologna@redhat.com>
>---
> ...ource-path.xml => channel-unix-source-path-active.xml} | 3 +++
> ...rce-path.xml => channel-unix-source-path-inactive.xml} | 0
> ...sk-active-commit.xml => disk-active-commit-active.xml} | 0
> ...-active-commit.xml => disk-active-commit-inactive.xml} | 8 ++------
> ...labelskip.xml => seclabel-static-labelskip-active.xml} | 0
> ...belskip.xml => seclabel-static-labelskip-inactive.xml} | 7 ++-----
> tests/qemuxml2xmltest.c                                   | 6 +++---
> 7 files changed, 10 insertions(+), 14 deletions(-)
> copy tests/qemuxml2xmloutdata/{channel-unix-source-path.xml => channel-unix-source-path-active.xml} (86%)
> rename tests/qemuxml2xmloutdata/{channel-unix-source-path.xml => channel-unix-source-path-inactive.xml} (100%)
> copy tests/qemuxml2xmloutdata/{disk-active-commit.xml => disk-active-commit-active.xml} (100%)
> rename tests/qemuxml2xmloutdata/{disk-active-commit.xml => disk-active-commit-inactive.xml} (86%)
> copy tests/qemuxml2xmloutdata/{seclabel-static-labelskip.xml => seclabel-static-labelskip-active.xml} (100%)
> rename tests/qemuxml2xmloutdata/{seclabel-static-labelskip.xml => seclabel-static-labelskip-inactive.xml} (86%)
>
>diff --git a/tests/qemuxml2xmloutdata/channel-unix-source-path.xml b/tests/qemuxml2xmloutdata/channel-unix-source-path-active.xml
>similarity index 86%
>copy from tests/qemuxml2xmloutdata/channel-unix-source-path.xml
>copy to tests/qemuxml2xmloutdata/channel-unix-source-path-active.xml
>index 2d8fa1e3ac..4c2a02b49a 100644
>--- a/tests/qemuxml2xmloutdata/channel-unix-source-path.xml
>+++ b/tests/qemuxml2xmloutdata/channel-unix-source-path-active.xml
>@@ -22,14 +22,17 @@
>       <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>     </controller>
>     <channel type='unix'>
>+      <source mode='bind' path='/tmp/channel/QEMUGuest1.org.qemu.guest_agent.0'/>

This checks that we correctly omit the auto-generated channel paths from
inactive XML.

>       <target type='virtio' name='org.qemu.guest_agent.0'/>
>       <address type='virtio-serial' controller='0' bus='0' port='1'/>
>     </channel>
>     <channel type='unix'>
>+      <source mode='bind' path='/tmp/channel/domain-QEMUGuest1/org.qemu.guest_agent.1'/>
>       <target type='virtio' name='org.qemu.guest_agent.1'/>
>       <address type='virtio-serial' controller='0' bus='0' port='2'/>
>     </channel>
>     <channel type='unix'>
>+      <source mode='bind' path='/tmp/channel/domain-1-QEMUGuest1/org.qemu.guest_agent.2'/>
>       <target type='virtio' name='org.qemu.guest_agent.2'/>
>       <address type='virtio-serial' controller='0' bus='0' port='3'/>
>     </channel>
>diff --git a/tests/qemuxml2xmloutdata/channel-unix-source-path.xml b/tests/qemuxml2xmloutdata/channel-unix-source-path-inactive.xml
>similarity index 100%
>rename from tests/qemuxml2xmloutdata/channel-unix-source-path.xml
>rename to tests/qemuxml2xmloutdata/channel-unix-source-path-inactive.xml
>diff --git a/tests/qemuxml2xmloutdata/disk-active-commit.xml b/tests/qemuxml2xmloutdata/disk-active-commit-active.xml
>similarity index 100%
>copy from tests/qemuxml2xmloutdata/disk-active-commit.xml
>copy to tests/qemuxml2xmloutdata/disk-active-commit-active.xml
>diff --git a/tests/qemuxml2xmloutdata/disk-active-commit.xml b/tests/qemuxml2xmloutdata/disk-active-commit-inactive.xml
>similarity index 86%
>rename from tests/qemuxml2xmloutdata/disk-active-commit.xml
>rename to tests/qemuxml2xmloutdata/disk-active-commit-inactive.xml
>index cc26af1096..b14345049d 100644
>--- a/tests/qemuxml2xmloutdata/disk-active-commit.xml
>+++ b/tests/qemuxml2xmloutdata/disk-active-commit-inactive.xml
>@@ -1,4 +1,4 @@
>-<domain type='qemu' id='1'>
>+<domain type='qemu'>
>   <name>QEMUGuest1</name>
>   <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
>   <memory unit='KiB'>219136</memory>
>@@ -17,15 +17,11 @@
>     <disk type='file' device='disk'>
>       <driver name='qemu' type='qcow2'/>
>       <source file='/tmp/HostVG/QEMUGuest1-snap'/>
>-      <backingStore type='block' index='1'>
>+      <backingStore type='block'>
>         <format type='raw'/>
>         <source dev='/dev/HostVG/QEMUGuest1'/>
>         <backingStore/>
>       </backingStore>
>-      <mirror type='block' job='active-commit'>
>-        <format type='raw'/>
>-        <source dev='/dev/HostVG/QEMUGuest1'/>
>-      </mirror>

Checking active-commit in inactive XML does not seem useful either.

>       <target dev='hda' bus='ide'/>
>       <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>     </disk>
>diff --git a/tests/qemuxml2xmloutdata/seclabel-static-labelskip.xml b/tests/qemuxml2xmloutdata/seclabel-static-labelskip-active.xml
>similarity index 100%
>copy from tests/qemuxml2xmloutdata/seclabel-static-labelskip.xml
>copy to tests/qemuxml2xmloutdata/seclabel-static-labelskip-active.xml
>diff --git a/tests/qemuxml2xmloutdata/seclabel-static-labelskip.xml b/tests/qemuxml2xmloutdata/seclabel-static-labelskip-inactive.xml
>similarity index 86%
>rename from tests/qemuxml2xmloutdata/seclabel-static-labelskip.xml
>rename to tests/qemuxml2xmloutdata/seclabel-static-labelskip-inactive.xml
>index 7c448675e2..e705b5452c 100644
>--- a/tests/qemuxml2xmloutdata/seclabel-static-labelskip.xml
>+++ b/tests/qemuxml2xmloutdata/seclabel-static-labelskip-inactive.xml
>@@ -1,4 +1,4 @@
>-<domain type='qemu' id='1'>
>+<domain type='qemu'>
>   <name>QEMUGuest1</name>
>   <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
>   <memory unit='KiB'>219100</memory>
>@@ -16,9 +16,7 @@
>     <emulator>/usr/bin/qemu-system-i686</emulator>
>     <disk type='block' device='disk'>
>       <driver name='qemu' type='raw'/>
>-      <source dev='/dev/HostVG/QEMUGuest1'>
>-        <seclabel model='selinux' labelskip='yes'/>
>-      </source>
>+      <source dev='/dev/HostVG/QEMUGuest1'/>

And labelskip is an active-XML only feature.

>       <backingStore/>
>       <target dev='hda' bus='ide'/>
>       <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>@@ -38,6 +36,5 @@
>   </devices>
>   <seclabel type='static' model='selinux' relabel='yes'>
>     <label>system_u:system_r:svirt_custom_t:s0:c192,c392</label>
>-    <imagelabel>system_u:system_r:svirt_custom_t:s0:c192,c392</imagelabel>
>   </seclabel>
> </domain>

But for the renames which reflect whether the test case is active or
inactive, you have my:
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] tests: Use WHEN_BOTH in qemuxml2xml whenever possible
Posted by Andrea Bolognani 6 years, 11 months ago 
On Fri, 2019-03-01 at 14:59 +0100, Ján Tomko wrote:
> On Thu, Feb 28, 2019 at 05:38:46PM +0100, Andrea Bolognani wrote:
[...]
> > +++ b/tests/qemuxml2xmloutdata/channel-unix-source-path-active.xml
> > @@ -22,14 +22,17 @@
> >       <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
> >     </controller>
> >     <channel type='unix'>
> > +      <source mode='bind' path='/tmp/channel/QEMUGuest1.org.qemu.guest_agent.0'/>
> 
> This checks that we correctly omit the auto-generated channel paths from
> inactive XML.

And having the active version side-by-side makes their absence more
obvious, no? So this still seems valuable.

[...]
> > +++ b/tests/qemuxml2xmloutdata/disk-active-commit-inactive.xml
> > @@ -1,4 +1,4 @@
> > -<domain type='qemu' id='1'>
> > +<domain type='qemu'>
> >   <name>QEMUGuest1</name>
> >   <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> >   <memory unit='KiB'>219136</memory>
> > @@ -17,15 +17,11 @@
> >     <disk type='file' device='disk'>
> >       <driver name='qemu' type='qcow2'/>
> >       <source file='/tmp/HostVG/QEMUGuest1-snap'/>
> > -      <backingStore type='block' index='1'>
> > +      <backingStore type='block'>
> >         <format type='raw'/>
> >         <source dev='/dev/HostVG/QEMUGuest1'/>
> >         <backingStore/>
> >       </backingStore>
> > -      <mirror type='block' job='active-commit'>
> > -        <format type='raw'/>
> > -        <source dev='/dev/HostVG/QEMUGuest1'/>
> > -      </mirror>
> 
> Checking active-commit in inactive XML does not seem useful either.

Again, it highlights the fact that the active commit feature is, as
the name suggests, only useful for the active XML :)

[...]
> > +++ b/tests/qemuxml2xmloutdata/seclabel-static-labelskip-inactive.xml
> > @@ -1,4 +1,4 @@
> > -<domain type='qemu' id='1'>
> > +<domain type='qemu'>
> >   <name>QEMUGuest1</name>
> >   <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> >   <memory unit='KiB'>219100</memory>
> > @@ -16,9 +16,7 @@
> >     <emulator>/usr/bin/qemu-system-i686</emulator>
> >     <disk type='block' device='disk'>
> >       <driver name='qemu' type='raw'/>
> > -      <source dev='/dev/HostVG/QEMUGuest1'>
> > -        <seclabel model='selinux' labelskip='yes'/>
> > -      </source>
> > +      <source dev='/dev/HostVG/QEMUGuest1'/>
> 
> And labelskip is an active-XML only feature.

Same as above.

Basically I see no reason not to cover both active and inactive
when possible, because even when the feature is only going to
manifest itself only in one of the two output files, the fact that
it does *not* show up in the other one represent some amount of
useful (negative) coverage.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] tests: Use WHEN_BOTH in qemuxml2xml whenever possible
Posted by Ján Tomko 6 years, 11 months ago 
On Fri, Mar 01, 2019 at 04:57:32PM +0100, Andrea Bolognani wrote:
>Basically I see no reason not to cover both active and inactive
>when possible, because even when the feature is only going to
>manifest itself only in one of the two output files, the fact that
>it does *not* show up in the other one represent some amount of
>useful (negative) coverage.
>

k

Reviewed-by: Ján Tomko <jtomko@redhat.com>

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.