Re: [PATCH 2/2] qemuxmlconftest: Add tests for the ACPI stripping hack on s390

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

 



On Wed, Jul 31, 2024 at 01:02:27PM GMT, Peter Krempa wrote:
> +++ b/tests/qemuxmlconfdata/aarch64-nousb-acpi.xml
> @@ -0,0 +1,18 @@
> +<domain type='kvm'>
> +  <name>aarch64test</name>
> +  <uuid>6ba410c5-1e5c-4d57-bee7-2228e7ffa32f</uuid>
> +  <memory unit='KiB'>1048576</memory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <!-- machine type doesn't matter as long as it has no implicit USB -->
> +    <type arch='aarch64' machine='borzoi'>hvm</type>
> +  </os>

The relationship between having implicit USB and being able to use
ACPI is not explained. I could probably figure it out by looking at
the code, but I think it would be better if the comment was expanded
to include this information.

> +++ b/tests/qemuxmlconfdata/riscv64-virt-acpi.xml
> @@ -0,0 +1,15 @@
> +<domain type='qemu'>
> +  <name>guest</name>
> +  <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid>
> +  <memory>4194304</memory>
> +  <vcpu>4</vcpu>
> +  <features>
> +    <acpi/>
> +  </features>
> +  <os>
> +    <type arch='riscv64' machine='virt'>hvm</type>
> +  </os>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-riscv64</emulator>
> +  </devices>
> +</domain>

Here and in a few other input files you've put the <features> element
before the <os> element, which of course our parser is perfectly
capable of handling but it just looks... Off to the human eye :)

For this input file in particular, you could add

  <memballoon model='none'/>

which would make for slighly smaller output files. I'd personally
just include that (as well as the USB equivalent) in every file, just
to ensure minimal hardware while making it easier to compare them.

> +++ b/tests/qemuxmlconftest.c
> @@ -1732,7 +1732,18 @@ mymain(void)
>
>      DO_TEST_CAPS_LATEST("input-usbmouse");
>      DO_TEST_CAPS_LATEST("input-usbtablet");
> -    DO_TEST_CAPS_LATEST("misc-acpi");
> +
> +    /* tests for ACPI support handling:
> +     *  - x86(_64) and aarch attempt

Incomplete sentence? Also it's aarch64.

> +     *  - other architectures base the decision based on how qemu reports
> +     *    the support for ACPI
> +     *  - s390x has hack to strip ACPI to preserve migration of old configs */
> +    DO_TEST_CAPS_LATEST("x86_64-q35-acpi");
> +    DO_TEST_CAPS_ARCH_LATEST_PARSE_ERROR("aarch64-nousb-acpi", "aarch64");

We should have a positive test for aarch64 which uses the virt
machine type and has ACPI enabled.

-- 
Andrea Bolognani / Red Hat / Virtualization



[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]

  Powered by Linux