tests/functional: Set 'pc' machine type instead of relying on the default

[PATCH] tests/functional: Set 'pc' machine type instead of relying on the default

Posted by Thomas Huth 5 months, 4 weeks ago

From: Thomas Huth <thuth@redhat.com>

In case the default machine has not been compiled into the QEMU
binary, the vnc and the cpu_hotplug_props test are currently
failing. Add a set_machine('pc') here to make sure that the tests
are correctly skipped in case the machine is not available.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/functional/test_pc_cpu_hotplug_props.py | 1 +
 tests/functional/test_vnc.py                  | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/tests/functional/test_pc_cpu_hotplug_props.py b/tests/functional/test_pc_cpu_hotplug_props.py
index 9d5a37cb170..2bed8ada023 100755
--- a/tests/functional/test_pc_cpu_hotplug_props.py
+++ b/tests/functional/test_pc_cpu_hotplug_props.py
@@ -26,6 +26,7 @@
 class OmittedCPUProps(QemuSystemTest):
 
     def test_no_die_id(self):
+        self.set_machine('pc')
         self.vm.add_args('-nodefaults', '-S')
         self.vm.add_args('-smp', '1,sockets=2,cores=2,threads=2,maxcpus=8')
         self.vm.add_args('-device', 'qemu64-x86_64-cpu,socket-id=1,core-id=0,thread-id=0')
diff --git a/tests/functional/test_vnc.py b/tests/functional/test_vnc.py
index 5c0ee5f9272..cbb01f00c7b 100755
--- a/tests/functional/test_vnc.py
+++ b/tests/functional/test_vnc.py
@@ -31,6 +31,7 @@ def check_connect(port: int) -> bool:
 class Vnc(QemuSystemTest):
 
     def test_no_vnc_change_password(self):
+        self.set_machine('pc')
         self.vm.add_args('-nodefaults', '-S')
         self.vm.launch()
 
@@ -62,6 +63,7 @@ def launch_guarded(self):
                 raise excp
 
     def test_change_password_requires_a_password(self):
+        self.set_machine('pc')
         self.vm.add_args('-nodefaults', '-S', '-vnc', ':1,to=999')
         self.launch_guarded()
         self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
@@ -74,6 +76,7 @@ def test_change_password_requires_a_password(self):
                          'Could not set password')
 
     def test_change_password(self):
+        self.set_machine('pc')
         self.vm.add_args('-nodefaults', '-S', '-vnc', ':1,to=999,password=on')
         self.launch_guarded()
         self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
@@ -103,6 +106,7 @@ def do_test_change_listen(self, a, b, c):
         self.assertTrue(check_connect(c))
 
     def test_change_listen(self):
+        self.set_machine('pc')
         with Ports() as ports:
             a, b, c = ports.find_free_ports(3)
             self.do_test_change_listen(a, b, c)
-- 
2.49.0

Re: [PATCH] tests/functional: Set 'pc' machine type instead of relying on the default

Posted by Daniel P. Berrangé 5 months, 4 weeks ago

On Wed, May 21, 2025 at 05:13:23PM +0200, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> In case the default machine has not been compiled into the QEMU
> binary, the vnc and the cpu_hotplug_props test are currently
> failing. Add a set_machine('pc') here to make sure that the tests
> are correctly skipped in case the machine is not available.

Hmmm, this is making this test be tied exclusively to x86.
If we can't ever omit the machine, then effectively all
our tests have to be tied to a specific target arch ?

What scenario did you find that 'pc' wasn't compiled
into QEMU  x86 targets ?  Shouldn't a new defualt
machine have been picked, or should our test harness
be picking a default for each targets if no 'set_machine'
call is present, to preserve target portability ?


> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/functional/test_pc_cpu_hotplug_props.py | 1 +
>  tests/functional/test_vnc.py                  | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/tests/functional/test_pc_cpu_hotplug_props.py b/tests/functional/test_pc_cpu_hotplug_props.py
> index 9d5a37cb170..2bed8ada023 100755
> --- a/tests/functional/test_pc_cpu_hotplug_props.py
> +++ b/tests/functional/test_pc_cpu_hotplug_props.py
> @@ -26,6 +26,7 @@
>  class OmittedCPUProps(QemuSystemTest):
>  
>      def test_no_die_id(self):
> +        self.set_machine('pc')
>          self.vm.add_args('-nodefaults', '-S')
>          self.vm.add_args('-smp', '1,sockets=2,cores=2,threads=2,maxcpus=8')
>          self.vm.add_args('-device', 'qemu64-x86_64-cpu,socket-id=1,core-id=0,thread-id=0')
> diff --git a/tests/functional/test_vnc.py b/tests/functional/test_vnc.py
> index 5c0ee5f9272..cbb01f00c7b 100755
> --- a/tests/functional/test_vnc.py
> +++ b/tests/functional/test_vnc.py
> @@ -31,6 +31,7 @@ def check_connect(port: int) -> bool:
>  class Vnc(QemuSystemTest):
>  
>      def test_no_vnc_change_password(self):
> +        self.set_machine('pc')
>          self.vm.add_args('-nodefaults', '-S')
>          self.vm.launch()
>  
> @@ -62,6 +63,7 @@ def launch_guarded(self):
>                  raise excp
>  
>      def test_change_password_requires_a_password(self):
> +        self.set_machine('pc')
>          self.vm.add_args('-nodefaults', '-S', '-vnc', ':1,to=999')
>          self.launch_guarded()
>          self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
> @@ -74,6 +76,7 @@ def test_change_password_requires_a_password(self):
>                           'Could not set password')
>  
>      def test_change_password(self):
> +        self.set_machine('pc')
>          self.vm.add_args('-nodefaults', '-S', '-vnc', ':1,to=999,password=on')
>          self.launch_guarded()
>          self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
> @@ -103,6 +106,7 @@ def do_test_change_listen(self, a, b, c):
>          self.assertTrue(check_connect(c))
>  
>      def test_change_listen(self):
> +        self.set_machine('pc')
>          with Ports() as ports:
>              a, b, c = ports.find_free_ports(3)
>              self.do_test_change_listen(a, b, c)
> -- 
> 2.49.0
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] tests/functional: Set 'pc' machine type instead of relying on the default

Posted by Thomas Huth 5 months, 3 weeks ago

On 21/05/2025 17.26, Daniel P. Berrangé wrote:
> On Wed, May 21, 2025 at 05:13:23PM +0200, Thomas Huth wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> In case the default machine has not been compiled into the QEMU
>> binary, the vnc and the cpu_hotplug_props test are currently
>> failing. Add a set_machine('pc') here to make sure that the tests
>> are correctly skipped in case the machine is not available.
> 
> Hmmm, this is making this test be tied exclusively to x86.
> If we can't ever omit the machine, then effectively all
> our tests have to be tied to a specific target arch ?

Since you never can rely on the availability of a default machine (there are 
also some targets like arm that don't have a default machine), you either 
have to set a certain machine explicitly with set_machine(), or use the 
"none" machine (if possible). Speaking of the latter - that's maybe a better 
option for this test, so I'll give that a try.

> What scenario did you find that 'pc' wasn't compiled
> into QEMU  x86 targets ?

Run "configure" with "--without-default-devices".

> Shouldn't a new default
> machine have been picked, or should our test harness
> be picking a default for each targets if no 'set_machine'
> call is present, to preserve target portability ?

We're using something like that in select_machine() in 
tests/functional/test_migration.py already ... we could maybe give it a try 
to turn that into a generic function of the qemu_test module, but I guess 
it's hard to find a default for each and every target that satisfies all the 
needs...

  Thomas