[libvirt PATCH] test: remove redundant cpuTestGuestCPUID test

Jonathon Jongsma posted 1 patch 5 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20231106204134.3664715-1-jjongsma@redhat.com
tests/cputest.c | 4 ----
1 file changed, 4 deletions(-)
[libvirt PATCH] test: remove redundant cpuTestGuestCPUID test
Posted by Jonathon Jongsma 5 months, 3 weeks ago
DO_TEST_CPUID(arch, host, json) is a multipart test. It consists of the
following tests:
 - cpuTestHostCPUID()
 - cpuTestGuestCPUID(with JSON_* flag)
 - cpuTestCPUIDSignature()
 - DO_TEST_JSON():
   - if json==JSON_MODELS:
     - cpuTestGuestCPUID(without JSON_* flag)
   - cpuTestJSONCPUID()
   - cputestJSONSignature()

Notice that for tests with json==JSON_MODELS, cpuTestGuestCPUID() is
actually called twice but with different arguments. The first one passes
JSON_MODELS to the test function, while the second one passes 0.

The main difference in behavior when calling cpuTestGuestCPUID() with or
without the flag is that in the first case, it parses the captured qemu
output from $ARCH-cpuid-$CPU.json. It extracts the cpu model list from
that JSON, and uses that to filter out possible cpu models to match.
In other words, it tries to match the cpu to a model that was supported
by the qemu version that was used to generate this JSON file. When it
finds a match, it generates a cpu definition and compares the xml form
of that definition with the file $ARCH-cpuid-$CPU-guest.xml.

When called without the JSON_MODELS flag, it simply attempts to match it
against the full libvirt cpu map and doesn't attempt to filter out any
matches based on the JSON qemu cpu model list. After it finds a match,
it generates an xml definition for the cpu and compares it to the same
file listed above. So if these two invocations disagree on the cpu match
(e.g. because libvirt has added a cpu model to its cpu map that matches
better than one that was supported by the version of qemu that generated
the JSON file) the test will fail.

This duplicate call to cpuTestGuestCPUID() was originally added in
commit 49c945a6f5c885394507f88086cc2f9461df7c27. The original
justification for that commit was to fix test failures when the Qemu
driver was disabled. But since DO_TEST_JSON() is #defined empty when
qemu is disabled, this particular invocation would not even be executed
in this scenario, so it doesn't seem relevant.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 tests/cputest.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tests/cputest.c b/tests/cputest.c
index b3253e3116..93cd0e12a7 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -993,10 +993,6 @@ mymain(void)
 #if WITH_QEMU
 # define DO_TEST_JSON(arch, host, json) \
     do { \
-        if (json == JSON_MODELS) { \
-            DO_TEST(arch, cpuTestGuestCPUID, host, host, \
-                    NULL, NULL, 0, NULL, 0, 0); \
-        } \
         if (json != JSON_NONE) { \
             DO_TEST(arch, cpuTestJSONCPUID, host, host, \
                     NULL, NULL, 0, NULL, json, 0); \
-- 
2.41.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [libvirt PATCH] test: remove redundant cpuTestGuestCPUID test
Posted by Michal Prívozník 4 months, 3 weeks ago
On 11/6/23 21:41, Jonathon Jongsma wrote:
> DO_TEST_CPUID(arch, host, json) is a multipart test. It consists of the
> following tests:
>  - cpuTestHostCPUID()
>  - cpuTestGuestCPUID(with JSON_* flag)
>  - cpuTestCPUIDSignature()
>  - DO_TEST_JSON():
>    - if json==JSON_MODELS:
>      - cpuTestGuestCPUID(without JSON_* flag)
>    - cpuTestJSONCPUID()
>    - cputestJSONSignature()
> 
> Notice that for tests with json==JSON_MODELS, cpuTestGuestCPUID() is
> actually called twice but with different arguments. The first one passes
> JSON_MODELS to the test function, while the second one passes 0.
> 
> The main difference in behavior when calling cpuTestGuestCPUID() with or
> without the flag is that in the first case, it parses the captured qemu
> output from $ARCH-cpuid-$CPU.json. It extracts the cpu model list from
> that JSON, and uses that to filter out possible cpu models to match.
> In other words, it tries to match the cpu to a model that was supported
> by the qemu version that was used to generate this JSON file. When it
> finds a match, it generates a cpu definition and compares the xml form
> of that definition with the file $ARCH-cpuid-$CPU-guest.xml.
> 
> When called without the JSON_MODELS flag, it simply attempts to match it
> against the full libvirt cpu map and doesn't attempt to filter out any
> matches based on the JSON qemu cpu model list. After it finds a match,
> it generates an xml definition for the cpu and compares it to the same
> file listed above. So if these two invocations disagree on the cpu match
> (e.g. because libvirt has added a cpu model to its cpu map that matches
> better than one that was supported by the version of qemu that generated
> the JSON file) the test will fail.
> 
> This duplicate call to cpuTestGuestCPUID() was originally added in
> commit 49c945a6f5c885394507f88086cc2f9461df7c27. The original
> justification for that commit was to fix test failures when the Qemu
> driver was disabled. But since DO_TEST_JSON() is #defined empty when
> qemu is disabled, this particular invocation would not even be executed
> in this scenario, so it doesn't seem relevant.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
>  tests/cputest.c | 4 ----
>  1 file changed, 4 deletions(-)
>

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org