[RFC PATCH] tests/qemuxml2*/graphics-spice-timeout: skip CPU model check

Daniel Henrique Barboza posted 1 patch 2 years, 5 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20211117001150.845465-1-danielhb413@gmail.com
tests/qemuxml2argvdata/graphics-spice-timeout.xml               | 2 +-
.../qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
[RFC PATCH] tests/qemuxml2*/graphics-spice-timeout: skip CPU model check
Posted by Daniel Henrique Barboza 2 years, 5 months ago
Commit 65b0b746b516 changed spice tests to use latest caps. Before this
change, "FLAG_REAL_CAPS" wasn't being set in testQemuInfoInitArgs(). The
absence of this flag triggered the code path inside
testCompareXMLToArgv() that executed testUpdateQEMUCaps(). This function
will update the host CPU via virQEMUCapsUpdateHostCPUModel() into
virQEMUCapsInitHostCPUModel().  In this function,
virQEMUCapsInitCPUModel() would end up updating the hostCPU inside the
qemuCaps (via virQEMUCapsProbeHostCPU()). Before the forementioned
commit, the host CPU was being defaulted to x86_64, vendor Intel, for
the 'graphics-spice-timeout' test that is using the 'pc' machine type
and 'accel=kvm'.

Today, "FLAG_REAL_CAPS" is being set because we're using the latest caps
from x86_64. This means that the whole code path mentioned above is
skipped. qemuCaps are now being loaded via virQEMUCapsLoadCache()
directly.  Without the handling being done by testUpdateQEMUCaps(), the
host CPU is being retrieved later on, down below
qemuProcessCreatePretendCmdPrepare() into qemuProcessUpdateGuestCPU().
The latter will attempt to update the domain cpu and executing a
virCPUCompare with the hostCPU and def->cpu.

All this logic ended up causing a failure of the
'graphics-spice-timeout' test in ppc64 and s390x hosts. This test is
being run with KVM acceleration, and the KVM driver for ppc64 and s390x
will return a default x86_64 CPU with vendor "AMD", making
virCPUCompare() fail with the following message:

"QEMU XML-2-ARGV graphics-spice-timeout.x86_64-latest   ... libvirt: CPU
Driver error : the CPU is incompatible with host CPU: host CPU vendor does
not match required CPU vendor Intel"

Fix this test by setting cpu check='none' and avoid the virCPUCompare()
that causes the problem for ppc64 and s390x hosts.

Reported-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---

Sending as a RFC because I'm not sure if this patch fixes the
problem for s390x. Boris, can you please test and see if this
fix works for you?


 tests/qemuxml2argvdata/graphics-spice-timeout.xml               | 2 +-
 .../qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemuxml2argvdata/graphics-spice-timeout.xml b/tests/qemuxml2argvdata/graphics-spice-timeout.xml
index 065318651f..33dae5c4bf 100644
--- a/tests/qemuxml2argvdata/graphics-spice-timeout.xml
+++ b/tests/qemuxml2argvdata/graphics-spice-timeout.xml
@@ -15,7 +15,7 @@
     <apic/>
     <pae/>
   </features>
-  <cpu match='exact'>
+  <cpu match='exact' check='none'>
     <model>core2duo</model>
     <vendor>Intel</vendor>
     <topology sockets='1' dies='1' cores='2' threads='1'/>
diff --git a/tests/qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml b/tests/qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml
index fd34a6caf5..9e6782d00b 100644
--- a/tests/qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml
@@ -15,7 +15,7 @@
     <apic/>
     <pae/>
   </features>
-  <cpu mode='custom' match='exact' check='partial'>
+  <cpu mode='custom' match='exact' check='none'>
     <model fallback='allow'>core2duo</model>
     <vendor>Intel</vendor>
     <topology sockets='1' dies='1' cores='2' threads='1'/>
-- 
2.31.1

Re: [RFC PATCH] tests/qemuxml2*/graphics-spice-timeout: skip CPU model check
Posted by Boris Fiuczynski 2 years, 5 months ago
On 11/17/21 1:11 AM, Daniel Henrique Barboza wrote:
> Commit 65b0b746b516 changed spice tests to use latest caps. Before this
> change, "FLAG_REAL_CAPS" wasn't being set in testQemuInfoInitArgs(). The
> absence of this flag triggered the code path inside
> testCompareXMLToArgv() that executed testUpdateQEMUCaps(). This function
> will update the host CPU via virQEMUCapsUpdateHostCPUModel() into
> virQEMUCapsInitHostCPUModel().  In this function,
> virQEMUCapsInitCPUModel() would end up updating the hostCPU inside the
> qemuCaps (via virQEMUCapsProbeHostCPU()). Before the forementioned
> commit, the host CPU was being defaulted to x86_64, vendor Intel, for
> the 'graphics-spice-timeout' test that is using the 'pc' machine type
> and 'accel=kvm'.
> 
> Today, "FLAG_REAL_CAPS" is being set because we're using the latest caps
> from x86_64. This means that the whole code path mentioned above is
> skipped. qemuCaps are now being loaded via virQEMUCapsLoadCache()
> directly.  Without the handling being done by testUpdateQEMUCaps(), the
> host CPU is being retrieved later on, down below
> qemuProcessCreatePretendCmdPrepare() into qemuProcessUpdateGuestCPU().
> The latter will attempt to update the domain cpu and executing a
> virCPUCompare with the hostCPU and def->cpu.
> 
> All this logic ended up causing a failure of the
> 'graphics-spice-timeout' test in ppc64 and s390x hosts. This test is
> being run with KVM acceleration, and the KVM driver for ppc64 and s390x
> will return a default x86_64 CPU with vendor "AMD", making
> virCPUCompare() fail with the following message:
> 
> "QEMU XML-2-ARGV graphics-spice-timeout.x86_64-latest   ... libvirt: CPU
> Driver error : the CPU is incompatible with host CPU: host CPU vendor does
> not match required CPU vendor Intel"
> 
> Fix this test by setting cpu check='none' and avoid the virCPUCompare()
> that causes the problem for ppc64 and s390x hosts.
> 
> Reported-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
> 
> Sending as a RFC because I'm not sure if this patch fixes the
> problem for s390x. Boris, can you please test and see if this
> fix works for you?

The proposed patch works on s390x as well.

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>

> 
> 
>   tests/qemuxml2argvdata/graphics-spice-timeout.xml               | 2 +-
>   .../qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemuxml2argvdata/graphics-spice-timeout.xml b/tests/qemuxml2argvdata/graphics-spice-timeout.xml
> index 065318651f..33dae5c4bf 100644
> --- a/tests/qemuxml2argvdata/graphics-spice-timeout.xml
> +++ b/tests/qemuxml2argvdata/graphics-spice-timeout.xml
> @@ -15,7 +15,7 @@
>       <apic/>
>       <pae/>
>     </features>
> -  <cpu match='exact'>
> +  <cpu match='exact' check='none'>
>       <model>core2duo</model>
>       <vendor>Intel</vendor>
>       <topology sockets='1' dies='1' cores='2' threads='1'/>
> diff --git a/tests/qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml b/tests/qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml
> index fd34a6caf5..9e6782d00b 100644
> --- a/tests/qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml
> +++ b/tests/qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml
> @@ -15,7 +15,7 @@
>       <apic/>
>       <pae/>
>     </features>
> -  <cpu mode='custom' match='exact' check='partial'>
> +  <cpu mode='custom' match='exact' check='none'>
>       <model fallback='allow'>core2duo</model>
>       <vendor>Intel</vendor>
>       <topology sockets='1' dies='1' cores='2' threads='1'/>
> 


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


Re: [RFC PATCH] tests/qemuxml2*/graphics-spice-timeout: skip CPU model check
Posted by Daniel Henrique Barboza 2 years, 5 months ago

On 11/17/21 08:02, Boris Fiuczynski wrote:
> On 11/17/21 1:11 AM, Daniel Henrique Barboza wrote:
>> Commit 65b0b746b516 changed spice tests to use latest caps. Before this
>> change, "FLAG_REAL_CAPS" wasn't being set in testQemuInfoInitArgs(). The
>> absence of this flag triggered the code path inside
>> testCompareXMLToArgv() that executed testUpdateQEMUCaps(). This function
>> will update the host CPU via virQEMUCapsUpdateHostCPUModel() into
>> virQEMUCapsInitHostCPUModel().  In this function,
>> virQEMUCapsInitCPUModel() would end up updating the hostCPU inside the
>> qemuCaps (via virQEMUCapsProbeHostCPU()). Before the forementioned
>> commit, the host CPU was being defaulted to x86_64, vendor Intel, for
>> the 'graphics-spice-timeout' test that is using the 'pc' machine type
>> and 'accel=kvm'.
>>
>> Today, "FLAG_REAL_CAPS" is being set because we're using the latest caps
>> from x86_64. This means that the whole code path mentioned above is
>> skipped. qemuCaps are now being loaded via virQEMUCapsLoadCache()
>> directly.  Without the handling being done by testUpdateQEMUCaps(), the
>> host CPU is being retrieved later on, down below
>> qemuProcessCreatePretendCmdPrepare() into qemuProcessUpdateGuestCPU().
>> The latter will attempt to update the domain cpu and executing a
>> virCPUCompare with the hostCPU and def->cpu.
>>
>> All this logic ended up causing a failure of the
>> 'graphics-spice-timeout' test in ppc64 and s390x hosts. This test is
>> being run with KVM acceleration, and the KVM driver for ppc64 and s390x
>> will return a default x86_64 CPU with vendor "AMD", making
>> virCPUCompare() fail with the following message:
>>
>> "QEMU XML-2-ARGV graphics-spice-timeout.x86_64-latest   ... libvirt: CPU
>> Driver error : the CPU is incompatible with host CPU: host CPU vendor does
>> not match required CPU vendor Intel"
>>
>> Fix this test by setting cpu check='none' and avoid the virCPUCompare()
>> that causes the problem for ppc64 and s390x hosts.
>>
>> Reported-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>
>> Sending as a RFC because I'm not sure if this patch fixes the
>> problem for s390x. Boris, can you please test and see if this
>> fix works for you?
> 
> The proposed patch works on s390x as well.
> 
> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>


Thanks for checking. I'll hold this patch a little* to see if we can fix the
real cause of the bug, which can be something to do with how we are defaulting
x86_64 cpus for ppc64 and s390x.



* just a couple of days. No need to let the build being broken more than that.



Daniel


> 
>>
>>
>>   tests/qemuxml2argvdata/graphics-spice-timeout.xml               | 2 +-
>>   .../qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/qemuxml2argvdata/graphics-spice-timeout.xml b/tests/qemuxml2argvdata/graphics-spice-timeout.xml
>> index 065318651f..33dae5c4bf 100644
>> --- a/tests/qemuxml2argvdata/graphics-spice-timeout.xml
>> +++ b/tests/qemuxml2argvdata/graphics-spice-timeout.xml
>> @@ -15,7 +15,7 @@
>>       <apic/>
>>       <pae/>
>>     </features>
>> -  <cpu match='exact'>
>> +  <cpu match='exact' check='none'>
>>       <model>core2duo</model>
>>       <vendor>Intel</vendor>
>>       <topology sockets='1' dies='1' cores='2' threads='1'/>
>> diff --git a/tests/qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml b/tests/qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml
>> index fd34a6caf5..9e6782d00b 100644
>> --- a/tests/qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml
>> +++ b/tests/qemuxml2xmloutdata/graphics-spice-timeout.x86_64-latest.xml
>> @@ -15,7 +15,7 @@
>>       <apic/>
>>       <pae/>
>>     </features>
>> -  <cpu mode='custom' match='exact' check='partial'>
>> +  <cpu mode='custom' match='exact' check='none'>
>>       <model fallback='allow'>core2duo</model>
>>       <vendor>Intel</vendor>
>>       <topology sockets='1' dies='1' cores='2' threads='1'/>
>>
> 
> 

Re: [RFC PATCH] tests/qemuxml2*/graphics-spice-timeout: skip CPU model check
Posted by Peter Krempa 2 years, 5 months ago
On Wed, Nov 17, 2021 at 08:16:05 -0300, Daniel Henrique Barboza wrote:
> On 11/17/21 08:02, Boris Fiuczynski wrote:
> > On 11/17/21 1:11 AM, Daniel Henrique Barboza wrote:

[...]

> > > fix works for you?
> > 
> > The proposed patch works on s390x as well.
> > 
> > Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> 
> 
> Thanks for checking. I'll hold this patch a little* to see if we can fix the
> real cause of the bug, which can be something to do with how we are defaulting
> x86_64 cpus for ppc64 and s390x.
> 
> 
> 
> * just a couple of days. No need to let the build being broken more than that.

I don't think that's necessary, the CPU checking is totally irrelevant
to the test you are fixing. If you manage to fix the root cause it can
definitely be done separately.

Re: [RFC PATCH] tests/qemuxml2*/graphics-spice-timeout: skip CPU model check
Posted by Daniel Henrique Barboza 2 years, 5 months ago

On 11/17/21 08:26, Peter Krempa wrote:
> On Wed, Nov 17, 2021 at 08:16:05 -0300, Daniel Henrique Barboza wrote:
>> On 11/17/21 08:02, Boris Fiuczynski wrote:
>>> On 11/17/21 1:11 AM, Daniel Henrique Barboza wrote:
> 
> [...]
> 
>>>> fix works for you?
>>>
>>> The proposed patch works on s390x as well.
>>>
>>> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
>>
>>
>> Thanks for checking. I'll hold this patch a little* to see if we can fix the
>> real cause of the bug, which can be something to do with how we are defaulting
>> x86_64 cpus for ppc64 and s390x.
>>
>>
>>
>> * just a couple of days. No need to let the build being broken more than that.
> 
> I don't think that's necessary, the CPU checking is totally irrelevant
> to the test you are fixing. If you manage to fix the root cause it can
> definitely be done separately.

Boris also mentioned that this is broken since Nov 4th. I'll make the commit msg
adjustments you mentioned and push it.




>