[12/12] tests: qemuxml2xml: Add DO_TEST_CAPS*

Message ID 6f9a1738d7d6d824c00c08191e9ff50a2784cdb6.1554137098.git.crobinso@redhat.com
State New
Headers show
Series
  • tests: qemuxml2xml: add DO_TEST_CAPS*
Related show

Commit Message

Cole Robinson April 1, 2019, 4:47 p.m.
Add DO_TEST_CAPS* macros, lifted from qemuxml2argvtest. Use it on
a few recently added xml2xml tests that use DO_TEST_CAPS in the argv
test case. The firmware examples require breaking the symlink and
creating our own test file. Also add a test for os-firmware-efi which
seems to have been missed.

One subtle difference compared to qemuxml2argv is output file naming.
qemuxml2xml uses a system where if specially named files
${basename}-active.xml or ${basename}-inactive.xml exist, those are
used as output, otherwise just ${basename}.xml is used. I'm not quite
sure how to make this fit with the caps suffix naming scheme used
in qemuxml2argv, where for example DO_CAPS_LATEST will always add a
-latest suffix to basename.

This code by default will store the output in ${basename}.xml with no
-latest suffix. This makes it easier to convert DO_TEST calls to CAPS
variants, because it won't require any test file rename/removal, but if
we ever want to add more than one qemuxml2xml output for a single input,
it will require special file naming to not collide. IMO that's not a
big deal as it follows the existing -active pattern. But it's a
divergence from qemuxml2argv behavior

Signed-off-by: Cole Robinson <crobinso@redhat.com>

---
 tests/qemuxml2argvdata/vhost-vsock.xml        |  2 +-
 .../aarch64-os-firmware-efi.xml               | 32 +++++++-
 tests/qemuxml2xmloutdata/os-firmware-bios.xml | 69 +++++++++++++++-
 .../os-firmware-efi-secboot.xml               | 69 +++++++++++++++-
 tests/qemuxml2xmloutdata/os-firmware-efi.xml  | 69 +++++++++++++++-
 tests/qemuxml2xmltest.c                       | 81 +++++++------------
 6 files changed, 267 insertions(+), 55 deletions(-)
 mode change 120000 => 100644 tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.xml
 mode change 120000 => 100644 tests/qemuxml2xmloutdata/os-firmware-bios.xml
 mode change 120000 => 100644 tests/qemuxml2xmloutdata/os-firmware-efi-secboot.xml
 mode change 120000 => 100644 tests/qemuxml2xmloutdata/os-firmware-efi.xml

-- 
2.21.0

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

Comments

Andrea Bolognani April 11, 2019, 10:44 a.m. | #1
On Mon, 2019-04-01 at 12:47 -0400, Cole Robinson wrote:
> Add DO_TEST_CAPS* macros, lifted from qemuxml2argvtest. Use it on

> a few recently added xml2xml tests that use DO_TEST_CAPS in the argv

> test case. The firmware examples require breaking the symlink and

> creating our own test file. Also add a test for os-firmware-efi which

> seems to have been missed.


Please have each of these changes as separate preparatory commits.

> One subtle difference compared to qemuxml2argv is output file naming.

> qemuxml2xml uses a system where if specially named files

> ${basename}-active.xml or ${basename}-inactive.xml exist, those are

> used as output, otherwise just ${basename}.xml is used. I'm not quite

> sure how to make this fit with the caps suffix naming scheme used

> in qemuxml2argv, where for example DO_CAPS_LATEST will always add a

> -latest suffix to basename.

> 

> This code by default will store the output in ${basename}.xml with no

> -latest suffix. This makes it easier to convert DO_TEST calls to CAPS

> variants, because it won't require any test file rename/removal, but if

> we ever want to add more than one qemuxml2xml output for a single input,

> it will require special file naming to not collide. IMO that's not a

> big deal as it follows the existing -active pattern. But it's a

> divergence from qemuxml2argv behavior


More on this later.

[...]
>  static int

>  testInfoSetPaths(struct testQemuInfo *info,

>                   const char *name,

> -                 int when)

> +                 int when,

> +                 const char *suffix)


'suffix' should come before 'when', to match the corresponding
function in xml2argv.

Additionally, we're passing 'name' to the function here but we're
storing it inside 'info' in xml2argv - we should be doing the same
here. Please make that change as a separate preparatory commit.

[...]
>      if (virAsprintf(&info->outfile,

> -                    "%s/qemuxml2xmloutdata/%s-%s.xml",

> +                    "%s/qemuxml2xmloutdata/%s-%s%s.xml",

>                      abs_srcdir, name,

> -                    when == WHEN_ACTIVE ? "active" : "inactive") < 0)

> +                    when == WHEN_ACTIVE ? "active" : "inactive",

> +                    suffix) < 0)

>          goto error;

>  

>      if (!virFileExists(info->outfile)) {


Missing from the context, but immediately after this line we have:

  if (virAsprintf(&info->outfile,
                  "%s/qemuxml2xmloutdata/%s.xml",
                  abs_srcdir, name) < 0)
      ...

We should format 'suffix' here too.

---

Following up from the commit message: I was wondering about the way
this test works, and discussing it with Jano (CC'd since he had his
own opinion on the matter).

There are several situations we can run into with these tests:

  1) we test WHEN_BOTH and the output for the WHEN_ACTIVE and
     WHEN_INACTIVE cases match;

  2) same as the above, but the outputs don't match;

  3) we test only one of WHEN_ACTIVE and WHEN_INACTIVE.

Case 1) covers the vast majority of existing test cases.

As for output files, I would expect respectively

  1) a single output file, with no suffix;

  2) two output files with different suffixes;

  3) a single output file with the corresponding -inactive or
     -active suffix.

The problem with the current algorithm is that, when
VIR_TEST_REGENERATE_OUTPUT is used, and you're introducing a new
test case so no output files exist yet, you'll end up with

  1) the expected output, yay!;

  2) failure, because both WHEN variants will write the (different)
     output to the unsuffixed file and step on each other's toes
     every time. This is annoying but ultimately okay, because the
     developer can break the loop simply by touching the suffixed
     files before running the test program;

  3) the unsuffixed file being created instead of the suffixed one.

The third scenario is suboptimal but not necessarily a very big deal
either.

One way to get rid of the above would be to pass an extra flags that
controls whether falling back to the unsuffixed output files should
be considered at all: the default would be to pass it for WHEN_BOTH,
so that scenario 1) would be covered, and not to pass it for
WHEN_ACTIVE and WHEN_INACTIVE to take care of scenario 3); the few
test cases that fall into scenario 2) would have to go for a more
verbose macro and *not* pass the flag manually. I feel like that
would be acceptable given that most tests cases fall into 1) anyway.

---

None of the above is really connected to whether or not we should
use 'suffix' as I suggested earlier: we should definitely format it,
even if it causes test suite churn. Not only that: you should also
make sure...

[...]
> +# define DO_TEST_CAPS_INTERNAL(name, arch, ver, ...) \

> +    DO_TEST_INTERNAL(name, "." arch "-" ver, WHEN_BOTH, \

> +                     ARG_CAPS_ARCH, arch, \

> +                     ARG_CAPS_VER, ver, \

> +                     __VA_ARGS__)


... you copy DO_TEST_CAPS_ARCH_VER() and DO_TEST_CAPS_VER() from
xml2argv too, so that you can later introduce...

[...]
> +    DO_TEST_CAPS_LATEST("virtio-transitional");

> +    DO_TEST_CAPS_LATEST("virtio-non-transitional");


  DO_TEST_CAPS_VER("virtio-transitional", "3.1.0");
  DO_TEST_CAPS_VER("virtio-non-transitional", "3.1.0");

and friends like in xml2argv, too.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Ján Tomko April 11, 2019, 12:15 p.m. | #2
On Thu, Apr 11, 2019 at 12:44:16PM +0200, Andrea Bolognani wrote:
>On Mon, 2019-04-01 at 12:47 -0400, Cole Robinson wrote:

>> Add DO_TEST_CAPS* macros, lifted from qemuxml2argvtest. Use it on

>> a few recently added xml2xml tests that use DO_TEST_CAPS in the argv

>> test case. The firmware examples require breaking the symlink and

>> creating our own test file. Also add a test for os-firmware-efi which

>> seems to have been missed.

>

>Please have each of these changes as separate preparatory commits.

>

>> One subtle difference compared to qemuxml2argv is output file naming.

>> qemuxml2xml uses a system where if specially named files

>> ${basename}-active.xml or ${basename}-inactive.xml exist, those are

>> used as output, otherwise just ${basename}.xml is used. I'm not quite

>> sure how to make this fit with the caps suffix naming scheme used

>> in qemuxml2argv, where for example DO_CAPS_LATEST will always add a

>> -latest suffix to basename.

>>

>> This code by default will store the output in ${basename}.xml with no

>> -latest suffix. This makes it easier to convert DO_TEST calls to CAPS

>> variants, because it won't require any test file rename/removal, but if

>> we ever want to add more than one qemuxml2xml output for a single input,

>> it will require special file naming to not collide. IMO that's not a

>> big deal as it follows the existing -active pattern. But it's a

>> divergence from qemuxml2argv behavior

>

>More on this later.

>

>[...]

>>  static int

>>  testInfoSetPaths(struct testQemuInfo *info,

>>                   const char *name,

>> -                 int when)

>> +                 int when,

>> +                 const char *suffix)

>

>'suffix' should come before 'when', to match the corresponding

>function in xml2argv.

>

>Additionally, we're passing 'name' to the function here but we're

>storing it inside 'info' in xml2argv - we should be doing the same

>here. Please make that change as a separate preparatory commit.

>

>[...]

>>      if (virAsprintf(&info->outfile,

>> -                    "%s/qemuxml2xmloutdata/%s-%s.xml",

>> +                    "%s/qemuxml2xmloutdata/%s-%s%s.xml",


I'd definitely put another minus between these suffixes (also, I'd like
to see them in use).

>>                      abs_srcdir, name,

>> -                    when == WHEN_ACTIVE ? "active" : "inactive") < 0)

>> +                    when == WHEN_ACTIVE ? "active" : "inactive",

>> +                    suffix) < 0)

>>          goto error;

>>

>>      if (!virFileExists(info->outfile)) {


As for this virFileExists - I really dislike it. It is on my TODO
list(TM) to change this to either:
A) specify exactly which output files the test needs by using the
appropriate DO_TEST macro
or
B) add a lot of symlinks for the expected output to the output
directory.

>

>Missing from the context, but immediately after this line we have:

>

>  if (virAsprintf(&info->outfile,

>                  "%s/qemuxml2xmloutdata/%s.xml",

>                  abs_srcdir, name) < 0)

>      ...

>

>We should format 'suffix' here too.

>

>---

>

>Following up from the commit message: I was wondering about the way

>this test works, and discussing it with Jano (CC'd since he had his

>own opinion on the matter).

>

>There are several situations we can run into with these tests:

>

>  1) we test WHEN_BOTH and the output for the WHEN_ACTIVE and

>     WHEN_INACTIVE cases match;

>

>  2) same as the above, but the outputs don't match;

>

>  3) we test only one of WHEN_ACTIVE and WHEN_INACTIVE.

>

>Case 1) covers the vast majority of existing test cases.

>

>As for output files, I would expect respectively

>

>  1) a single output file, with no suffix;

>

>  2) two output files with different suffixes;

>

>  3) a single output file with the corresponding -inactive or

>     -active suffix.

>

>The problem with the current algorithm is that, when

>VIR_TEST_REGENERATE_OUTPUT is used, and you're introducing a new

>test case so no output files exist yet, you'll end up with

>

>  1) the expected output, yay!;

>

>  2) failure, because both WHEN variants will write the (different)

>     output to the unsuffixed file and step on each other's toes

>     every time. This is annoying but ultimately okay, because the

>     developer can break the loop simply by touching the suffixed

>     files before running the test program;

>

>  3) the unsuffixed file being created instead of the suffixed one.

>

>The third scenario is suboptimal but not necessarily a very big deal

>either.

>

>One way to get rid of the above would be to pass an extra flags that

>controls whether falling back to the unsuffixed output files should

>be considered at all: the default would be to pass it for WHEN_BOTH,

>so that scenario 1) would be covered, and not to pass it for

>WHEN_ACTIVE and WHEN_INACTIVE to take care of scenario 3); the few

>test cases that fall into scenario 2) would have to go for a more

>verbose macro and *not* pass the flag manually. I feel like that

>would be acceptable given that most tests cases fall into 1) anyway.

>

>---

>

>None of the above is really connected to whether or not we should

>use 'suffix' as I suggested earlier: we should definitely format it,

>even if it causes test suite churn. Not only that: you should also

>make sure...

>


The reason for the suffix in xml2argv is to allow the CAPS_LATEST tests
to coexist with the ones with enumerated capabilities.

But it also contains the architecture, so even if -latest would be the
prevailing case, I'd rather format it anyway.

Jano

>[...]

>> +# define DO_TEST_CAPS_INTERNAL(name, arch, ver, ...) \

>> +    DO_TEST_INTERNAL(name, "." arch "-" ver, WHEN_BOTH, \

>> +                     ARG_CAPS_ARCH, arch, \

>> +                     ARG_CAPS_VER, ver, \

>> +                     __VA_ARGS__)

>

>... you copy DO_TEST_CAPS_ARCH_VER() and DO_TEST_CAPS_VER() from

>xml2argv too, so that you can later introduce...

>

>[...]

>> +    DO_TEST_CAPS_LATEST("virtio-transitional");

>> +    DO_TEST_CAPS_LATEST("virtio-non-transitional");

>

>  DO_TEST_CAPS_VER("virtio-transitional", "3.1.0");

>  DO_TEST_CAPS_VER("virtio-non-transitional", "3.1.0");

>

>and friends like in xml2argv, too.

>

>-- 

>Andrea Bolognani / Red Hat / Virtualization

>

>--

>libvir-list mailing list

>libvir-list@redhat.com

>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Andrea Bolognani April 11, 2019, 1:18 p.m. | #3
On Thu, 2019-04-11 at 14:15 +0200, Ján Tomko wrote:
> On Thu, Apr 11, 2019 at 12:44:16PM +0200, Andrea Bolognani wrote:
> > On Mon, 2019-04-01 at 12:47 -0400, Cole Robinson wrote:
> > >      if (virAsprintf(&info->outfile,
> > > -                    "%s/qemuxml2xmloutdata/%s-%s.xml",
> > > +                    "%s/qemuxml2xmloutdata/%s-%s%s.xml",
> 
> I'd definitely put another minus between these suffixes (also, I'd like
> to see them in use).

Looking at xml2argv, and as I suggested the implementation should be
the same in xml2xml, we have

  # define DO_TEST_CAPS_INTERNAL(name, arch, ver, ...) \
      DO_TEST_INTERNAL(name, "." arch "-" ver, \
                       ARG_CAPS_ARCH, arch, \
                       ARG_CAPS_VER, ver, \
                       __VA_ARGS__)

  # define DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, ...) \
      DO_TEST_CAPS_INTERNAL(name, arch, "latest", __VA_ARGS__)

  # define DO_TEST_CAPS_ARCH_LATEST(name, arch) \
      DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, ARG_END)

  # define DO_TEST_CAPS_LATEST(name) \
      DO_TEST_CAPS_ARCH_LATEST(name, "x86_64")

so when you use
  
    DO_TEST_CAPS_LATEST("virtio-transitional");

'suffix' will be ".x86_64-latest" and the resulting filename
will be

  qemuxml2xmloutdata/virtio-transitional.x86_64-latest.xml

If you had a similar test for WHEN_ACTIVE only, then you'd get

  qemuxml2xmloutdata/virtio-transitional-active.x86_64-latest.xml

It doesn't look to me like any extra separators are necessary.

[...]
> > None of the above is really connected to whether or not we should
> > use 'suffix' as I suggested earlier: we should definitely format it,
> > even if it causes test suite churn. Not only that: you should also
> > make sure...
> 
> The reason for the suffix in xml2argv is to allow the CAPS_LATEST tests
> to coexist with the ones with enumerated capabilities.
> 
> But it also contains the architecture, so even if -latest would be the
> prevailing case, I'd rather format it anyway.

Looks like we agree then :)
Cole Robinson April 15, 2019, 10:35 p.m. | #4
On 4/11/19 8:15 AM, Ján Tomko wrote:
> On Thu, Apr 11, 2019 at 12:44:16PM +0200, Andrea Bolognani wrote:

>> On Mon, 2019-04-01 at 12:47 -0400, Cole Robinson wrote:

>>> Add DO_TEST_CAPS* macros, lifted from qemuxml2argvtest. Use it on

>>> a few recently added xml2xml tests that use DO_TEST_CAPS in the argv

>>> test case. The firmware examples require breaking the symlink and

>>> creating our own test file. Also add a test for os-firmware-efi which

>>> seems to have been missed.

>>

>> Please have each of these changes as separate preparatory commits.

>>

>>> One subtle difference compared to qemuxml2argv is output file naming.

>>> qemuxml2xml uses a system where if specially named files

>>> ${basename}-active.xml or ${basename}-inactive.xml exist, those are

>>> used as output, otherwise just ${basename}.xml is used. I'm not quite

>>> sure how to make this fit with the caps suffix naming scheme used

>>> in qemuxml2argv, where for example DO_CAPS_LATEST will always add a

>>> -latest suffix to basename.

>>>

>>> This code by default will store the output in ${basename}.xml with no

>>> -latest suffix. This makes it easier to convert DO_TEST calls to CAPS

>>> variants, because it won't require any test file rename/removal, but if

>>> we ever want to add more than one qemuxml2xml output for a single input,

>>> it will require special file naming to not collide. IMO that's not a

>>> big deal as it follows the existing -active pattern. But it's a

>>> divergence from qemuxml2argv behavior

>>

>> More on this later.

>>

>> [...]

>>>  static int

>>>  testInfoSetPaths(struct testQemuInfo *info,

>>>                   const char *name,

>>> -                 int when)

>>> +                 int when,

>>> +                 const char *suffix)

>>

>> 'suffix' should come before 'when', to match the corresponding

>> function in xml2argv.

>>

>> Additionally, we're passing 'name' to the function here but we're

>> storing it inside 'info' in xml2argv - we should be doing the same

>> here. Please make that change as a separate preparatory commit.

>>

>> [...]

>>>      if (virAsprintf(&info->outfile,

>>> -                    "%s/qemuxml2xmloutdata/%s-%s.xml",

>>> +                    "%s/qemuxml2xmloutdata/%s-%s%s.xml",

> 

> I'd definitely put another minus between these suffixes (also, I'd like

> to see them in use).

> 


Sure I'll add example usage for this in the next version

>>>                      abs_srcdir, name,

>>> -                    when == WHEN_ACTIVE ? "active" : "inactive") < 0)

>>> +                    when == WHEN_ACTIVE ? "active" : "inactive",

>>> +                    suffix) < 0)

>>>          goto error;

>>>

>>>      if (!virFileExists(info->outfile)) {

> 

> As for this virFileExists - I really dislike it. It is on my TODO

> list(TM) to change this to either:

> A) specify exactly which output files the test needs by using the

> appropriate DO_TEST macro

> or

> B) add a lot of symlinks for the expected output to the output

> directory.

> 


Agreed that this should go away. I think the whole WHEN_X concept should
go away: always test active and inactive paths, but the only DO_TEST
distinction is active vs inactive output is expected to be the same, vs
expected to be different. The latter case should mandate that -active
and -inactive filesnames exist, the former case doesn't look for any
state suffix. I think only the few WEHN_INACTIVE cases will need some
extra output but otherwise it's pretty compatible with the current state
of things.

>>

>> Missing from the context, but immediately after this line we have:

>>

>>  if (virAsprintf(&info->outfile,

>>                  "%s/qemuxml2xmloutdata/%s.xml",

>>                  abs_srcdir, name) < 0)

>>      ...

>>

>> We should format 'suffix' here too.

>>

>> ---

>>

>> Following up from the commit message: I was wondering about the way

>> this test works, and discussing it with Jano (CC'd since he had his

>> own opinion on the matter).

>>

>> There are several situations we can run into with these tests:

>>

>>  1) we test WHEN_BOTH and the output for the WHEN_ACTIVE and

>>     WHEN_INACTIVE cases match;

>>

>>  2) same as the above, but the outputs don't match;

>>

>>  3) we test only one of WHEN_ACTIVE and WHEN_INACTIVE.

>>

>> Case 1) covers the vast majority of existing test cases.

>>

>> As for output files, I would expect respectively

>>

>>  1) a single output file, with no suffix;

>>

>>  2) two output files with different suffixes;

>>

>>  3) a single output file with the corresponding -inactive or

>>     -active suffix.

>>

>> The problem with the current algorithm is that, when

>> VIR_TEST_REGENERATE_OUTPUT is used, and you're introducing a new

>> test case so no output files exist yet, you'll end up with

>>

>>  1) the expected output, yay!;

>>

>>  2) failure, because both WHEN variants will write the (different)

>>     output to the unsuffixed file and step on each other's toes

>>     every time. This is annoying but ultimately okay, because the

>>     developer can break the loop simply by touching the suffixed

>>     files before running the test program;

>>

>>  3) the unsuffixed file being created instead of the suffixed one.

>>

>> The third scenario is suboptimal but not necessarily a very big deal

>> either.

>>

>> One way to get rid of the above would be to pass an extra flags that

>> controls whether falling back to the unsuffixed output files should

>> be considered at all: the default would be to pass it for WHEN_BOTH,

>> so that scenario 1) would be covered, and not to pass it for

>> WHEN_ACTIVE and WHEN_INACTIVE to take care of scenario 3); the few

>> test cases that fall into scenario 2) would have to go for a more

>> verbose macro and *not* pass the flag manually. I feel like that

>> would be acceptable given that most tests cases fall into 1) anyway.

>>

>> ---

>>

>> None of the above is really connected to whether or not we should

>> use 'suffix' as I suggested earlier: we should definitely format it,

>> even if it causes test suite churn. Not only that: you should also

>> make sure...

>>

> 

> The reason for the suffix in xml2argv is to allow the CAPS_LATEST tests

> to coexist with the ones with enumerated capabilities.

> 

> But it also contains the architecture, so even if -latest would be the

> prevailing case, I'd rather format it anyway.

> 


Hopefully one day we get to a point that DO_TEST always uses latest
caps, and we have a DO_TEST_CAPS_LIST for the explicit caps list...

Thanks,
Cole

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

Patch

diff --git a/tests/qemuxml2argvdata/vhost-vsock.xml b/tests/qemuxml2argvdata/vhost-vsock.xml
index bc550ace4e..f594c53f48 100644
--- a/tests/qemuxml2argvdata/vhost-vsock.xml
+++ b/tests/qemuxml2argvdata/vhost-vsock.xml
@@ -15,7 +15,7 @@ 
   <on_crash>restart</on_crash>
   <devices>
     <emulator>/usr/bin/qemu-system-x86_64</emulator>
-    <controller type='usb' index='0'>
+    <controller type='usb' index='0' model='piix3-uhci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
     </controller>
     <controller type='virtio-serial' index='0'>
diff --git a/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.xml b/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.xml
deleted file mode 120000
index beea6b2955..0000000000
--- a/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.xml
+++ /dev/null
@@ -1 +0,0 @@ 
-../qemuxml2argvdata/aarch64-os-firmware-efi.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.xml b/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.xml
new file mode 100644
index 0000000000..529ce6f3c2
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/aarch64-os-firmware-efi.xml
@@ -0,0 +1,31 @@ 
+<domain type='qemu'>
+  <name>aarch64test</name>
+  <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os firmware='efi'>
+    <type arch='aarch64' machine='virt-4.0'>hvm</type>
+    <kernel>/aarch64.kernel</kernel>
+    <initrd>/aarch64.initrd</initrd>
+    <cmdline>earlyprintk console=ttyAMA0,115200n8 rw root=/dev/vda rootwait</cmdline>
+    <dtb>/aarch64.dtb</dtb>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <apic/>
+    <pae/>
+    <gic version='2'/>
+  </features>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='allow'>cortex-a53</model>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-aarch64</emulator>
+    <controller type='pci' index='0' model='pcie-root'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/os-firmware-bios.xml b/tests/qemuxml2xmloutdata/os-firmware-bios.xml
deleted file mode 120000
index 3d36d5df68..0000000000
--- a/tests/qemuxml2xmloutdata/os-firmware-bios.xml
+++ /dev/null
@@ -1 +0,0 @@ 
-../qemuxml2argvdata/os-firmware-bios.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmloutdata/os-firmware-bios.xml b/tests/qemuxml2xmloutdata/os-firmware-bios.xml
new file mode 100644
index 0000000000..63886666dd
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/os-firmware-bios.xml
@@ -0,0 +1,68 @@ 
+<domain type='kvm'>
+  <name>fedora</name>
+  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+  <memory unit='KiB'>8192</memory>
+  <currentMemory unit='KiB'>8192</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os firmware='bios'>
+    <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+    <loader secure='no'/>
+    <nvram>/var/lib/libvirt/qemu/nvram/fedora_VARS.fd</nvram>
+    <boot dev='hd'/>
+    <bootmenu enable='yes'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <pm>
+    <suspend-to-mem enabled='yes'/>
+    <suspend-to-disk enabled='no'/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0' model='ich9-ehci1'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x7'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci1'>
+      <master startport='0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x0' multifunction='on'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci2'>
+      <master startport='2'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x1'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci3'>
+      <master startport='4'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x2'/>
+    </controller>
+    <controller type='sata' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <controller type='pci' index='1' model='dmi-to-pci-bridge'>
+      <model name='i82801b11-bridge'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/>
+    </controller>
+    <controller type='pci' index='2' model='pci-bridge'>
+      <model name='pci-bridge'/>
+      <target chassisNr='2'/>
+      <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
+    </controller>
+    <controller type='pci' index='3' model='pcie-root-port'>
+      <model name='ioh3420'/>
+      <target chassis='3' port='0x8'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+    </controller>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.xml b/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.xml
deleted file mode 120000
index 93e184e2d2..0000000000
--- a/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.xml
+++ /dev/null
@@ -1 +0,0 @@ 
-../qemuxml2argvdata/os-firmware-efi-secboot.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.xml b/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.xml
new file mode 100644
index 0000000000..a285e06334
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/os-firmware-efi-secboot.xml
@@ -0,0 +1,68 @@ 
+<domain type='kvm'>
+  <name>fedora</name>
+  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+  <memory unit='KiB'>8192</memory>
+  <currentMemory unit='KiB'>8192</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os firmware='efi'>
+    <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+    <loader secure='yes'/>
+    <nvram>/var/lib/libvirt/qemu/nvram/fedora_VARS.fd</nvram>
+    <boot dev='hd'/>
+    <bootmenu enable='yes'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <pm>
+    <suspend-to-mem enabled='yes'/>
+    <suspend-to-disk enabled='no'/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0' model='ich9-ehci1'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x7'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci1'>
+      <master startport='0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x0' multifunction='on'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci2'>
+      <master startport='2'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x1'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci3'>
+      <master startport='4'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x2'/>
+    </controller>
+    <controller type='sata' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <controller type='pci' index='1' model='dmi-to-pci-bridge'>
+      <model name='i82801b11-bridge'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/>
+    </controller>
+    <controller type='pci' index='2' model='pci-bridge'>
+      <model name='pci-bridge'/>
+      <target chassisNr='2'/>
+      <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
+    </controller>
+    <controller type='pci' index='3' model='pcie-root-port'>
+      <model name='ioh3420'/>
+      <target chassis='3' port='0x8'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+    </controller>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/os-firmware-efi.xml b/tests/qemuxml2xmloutdata/os-firmware-efi.xml
deleted file mode 120000
index 15cfad1ea0..0000000000
--- a/tests/qemuxml2xmloutdata/os-firmware-efi.xml
+++ /dev/null
@@ -1 +0,0 @@ 
-../qemuxml2argvdata/os-firmware-efi.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmloutdata/os-firmware-efi.xml b/tests/qemuxml2xmloutdata/os-firmware-efi.xml
new file mode 100644
index 0000000000..46a7b1b780
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/os-firmware-efi.xml
@@ -0,0 +1,68 @@ 
+<domain type='kvm'>
+  <name>fedora</name>
+  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+  <memory unit='KiB'>8192</memory>
+  <currentMemory unit='KiB'>8192</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os firmware='efi'>
+    <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+    <loader secure='no'/>
+    <nvram>/var/lib/libvirt/qemu/nvram/fedora_VARS.fd</nvram>
+    <boot dev='hd'/>
+    <bootmenu enable='yes'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <pm>
+    <suspend-to-mem enabled='yes'/>
+    <suspend-to-disk enabled='no'/>
+  </pm>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0' model='ich9-ehci1'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x7'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci1'>
+      <master startport='0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x0' multifunction='on'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci2'>
+      <master startport='2'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x1'/>
+    </controller>
+    <controller type='usb' index='0' model='ich9-uhci3'>
+      <master startport='4'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1d' function='0x2'/>
+    </controller>
+    <controller type='sata' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <controller type='pci' index='1' model='dmi-to-pci-bridge'>
+      <model name='i82801b11-bridge'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1e' function='0x0'/>
+    </controller>
+    <controller type='pci' index='2' model='pci-bridge'>
+      <model name='pci-bridge'/>
+      <target chassisNr='2'/>
+      <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
+    </controller>
+    <controller type='pci' index='3' model='pcie-root-port'>
+      <model name='ioh3420'/>
+      <target chassis='3' port='0x8'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+    </controller>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 3b5314df78..6ae3cbb11f 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -91,7 +91,8 @@  testCompareStatusXMLToXMLFiles(const void *opaque)
 static int
 testInfoSetPaths(struct testQemuInfo *info,
                  const char *name,
-                 int when)
+                 int when,
+                 const char *suffix)
 {
     VIR_FREE(info->infile);
     VIR_FREE(info->outfile);
@@ -101,9 +102,10 @@  testInfoSetPaths(struct testQemuInfo *info,
         goto error;
 
     if (virAsprintf(&info->outfile,
-                    "%s/qemuxml2xmloutdata/%s-%s.xml",
+                    "%s/qemuxml2xmloutdata/%s-%s%s.xml",
                     abs_srcdir, name,
-                    when == WHEN_ACTIVE ? "active" : "inactive") < 0)
+                    when == WHEN_ACTIVE ? "active" : "inactive",
+                    suffix) < 0)
         goto error;
 
     if (!virFileExists(info->outfile)) {
@@ -175,7 +177,7 @@  mymain(void)
 
     cfg = virQEMUDriverGetConfig(&driver);
 
-# define DO_TEST_FULL(name, when, ...) \
+# define DO_TEST_INTERNAL(name, suffix, when, ...) \
     do { \
         if (testQemuInfoSetArgs(&info, capslatest, \
                                 __VA_ARGS__, \
@@ -186,7 +188,7 @@  mymain(void)
         } \
  \
         if (when & WHEN_INACTIVE) { \
-            if (testInfoSetPaths(&info, name, WHEN_INACTIVE) < 0) { \
+            if (testInfoSetPaths(&info, name, WHEN_INACTIVE, suffix) < 0) { \
                 VIR_TEST_DEBUG("Failed to generate inactive paths for '%s'", name); \
                 return -1; \
             } \
@@ -196,7 +198,7 @@  mymain(void)
         } \
  \
         if (when & WHEN_ACTIVE) { \
-            if (testInfoSetPaths(&info, name, WHEN_ACTIVE) < 0) { \
+            if (testInfoSetPaths(&info, name, WHEN_ACTIVE, suffix) < 0) { \
                 VIR_TEST_DEBUG("Failed to generate active paths for '%s'", name); \
                 return -1; \
             } \
@@ -209,9 +211,26 @@  mymain(void)
 
 # define NONE QEMU_CAPS_LAST
 
+# define DO_TEST_FULL(name, when, ...) \
+    DO_TEST_INTERNAL(name, "", when, __VA_ARGS__)
+
 # define DO_TEST(name, ...) \
     DO_TEST_FULL(name, WHEN_BOTH, ARG_QEMU_CAPS, __VA_ARGS__, QEMU_CAPS_LAST)
 
+# define DO_TEST_CAPS_INTERNAL(name, arch, ver, ...) \
+    DO_TEST_INTERNAL(name, "." arch "-" ver, WHEN_BOTH, \
+                     ARG_CAPS_ARCH, arch, \
+                     ARG_CAPS_VER, ver, \
+                     __VA_ARGS__)
+
+# define DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, ...) \
+    DO_TEST_CAPS_INTERNAL(name, arch, "latest", __VA_ARGS__)
+
+# define DO_TEST_CAPS_ARCH_LATEST(name, arch) \
+    DO_TEST_CAPS_ARCH_LATEST_FULL(name, arch, ARG_END)
+
+# define DO_TEST_CAPS_LATEST(name) \
+    DO_TEST_CAPS_ARCH_LATEST(name, "x86_64")
 
 
     /* Unset or set all envvars here that are copied in qemudBuildCommandLine
@@ -979,36 +998,14 @@  mymain(void)
     DO_TEST("smbios", NONE);
     DO_TEST("smbios-multiple-type2", NONE);
 
-    DO_TEST("os-firmware-bios",
-            QEMU_CAPS_DEVICE_PCI_BRIDGE,
-            QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
-            QEMU_CAPS_DEVICE_IOH3420,
-            QEMU_CAPS_ICH9_AHCI,
-            QEMU_CAPS_ICH9_USB_EHCI1,
-            QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
-            QEMU_CAPS_DEVICE_QXL);
-    DO_TEST("os-firmware-efi",
-            QEMU_CAPS_DEVICE_PCI_BRIDGE,
-            QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
-            QEMU_CAPS_DEVICE_IOH3420,
-            QEMU_CAPS_ICH9_AHCI,
-            QEMU_CAPS_ICH9_USB_EHCI1,
-            QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
-            QEMU_CAPS_DEVICE_QXL);
-    DO_TEST("os-firmware-efi-secboot",
-            QEMU_CAPS_DEVICE_PCI_BRIDGE,
-            QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
-            QEMU_CAPS_DEVICE_IOH3420,
-            QEMU_CAPS_ICH9_AHCI,
-            QEMU_CAPS_ICH9_USB_EHCI1,
-            QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
-            QEMU_CAPS_DEVICE_QXL);
+    DO_TEST_CAPS_LATEST("os-firmware-bios");
+    DO_TEST_CAPS_LATEST("os-firmware-efi");
+    DO_TEST_CAPS_LATEST("os-firmware-efi-secboot");
 
     DO_TEST("aarch64-aavmf-virtio-mmio",
             QEMU_CAPS_DEVICE_VIRTIO_MMIO,
             QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM);
-    DO_TEST("aarch64-os-firmware-efi",
-            QEMU_CAPS_DEVICE_VIRTIO_MMIO);
+    DO_TEST_CAPS_ARCH_LATEST("aarch64-os-firmware-efi", "aarch64");
     DO_TEST("aarch64-virtio-pci-default",
             QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY,
             QEMU_CAPS_DEVICE_VIRTIO_MMIO,
@@ -1256,24 +1253,8 @@  mymain(void)
     DO_TEST("riscv64-virt-pci",
             QEMU_CAPS_OBJECT_GPEX);
 
-    DO_TEST("virtio-transitional",
-            QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
-            QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE,
-            QEMU_CAPS_DEVICE_PCIE_ROOT_PORT,
-            QEMU_CAPS_DEVICE_VIRTIO_RNG,
-            QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY,
-            QEMU_CAPS_DEVICE_VHOST_VSOCK,
-            QEMU_CAPS_VIRTIO_INPUT_HOST,
-            QEMU_CAPS_VIRTIO_SCSI);
-    DO_TEST("virtio-non-transitional",
-            QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
-            QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE,
-            QEMU_CAPS_DEVICE_PCIE_ROOT_PORT,
-            QEMU_CAPS_DEVICE_VIRTIO_RNG,
-            QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY,
-            QEMU_CAPS_DEVICE_VHOST_VSOCK,
-            QEMU_CAPS_VIRTIO_INPUT_HOST,
-            QEMU_CAPS_VIRTIO_SCSI);
+    DO_TEST_CAPS_LATEST("virtio-transitional");
+    DO_TEST_CAPS_LATEST("virtio-non-transitional");
 
     if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
         virFileDeleteTree(fakerootdir);