Re: [PATCH v2 1/2] tests/functional/ppc64: extend pseries migration tests

[Aniket Sahu <asahu1x@linux.ibm.com>]

On 21/09/26 12:11 PM, Thomas Huth wrote:
> On 21/09/2026 07.56, Aniket Sahu wrote:
>>
>> On 19/09/26 12:15 AM, Amit Machhiwal wrote:
>>> Hi Aniket,
>>>
>>> Thanks for the patches.  Please find my comments inline below.
>>>
>>> On 2026/09/16 04:06 PM, Aniket Sahu wrote:
>>>> Made a few changes to test_pseries.py to extend the migration
>>>> tests to validate exec (socat) and UNIX paths.
>>>>
>>>> - Refactored test_ppc64_linux_migration to a private method
>>>> _do_linux_migration(dst_uri, src_uri=None).
>>>> - It is then called by three separate methods which exercise the
>>>> TCP, UNIX and exec paths via the migrate_vms method from
>>>> MigrationTest.
>>>> ---
>>>>   tests/functional/ppc64/test_pseries.py | 41 
>>>> ++++++++++++++++++++++----
>>>>   1 file changed, 36 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/tests/functional/ppc64/test_pseries.py 
>>>> b/tests/functional/ ppc64/test_pseries.py
>>>> index cb6d25dc81..19f31141b1 100755
>>>> --- a/tests/functional/ppc64/test_pseries.py
>>>> +++ b/tests/functional/ppc64/test_pseries.py
>>>> @@ -8,8 +8,9 @@
>>>>   # later.  See the COPYING file in the top-level directory.
>>>>   from qemu_test import Asset
>>>> -from qemu_test import wait_for_console_pattern
>>>> +from qemu_test import wait_for_console_pattern, which
>>>>   from migration import MigrationTest
>>>> +from qemu_test.ports import Ports
>>>>   class PseriesMachine(MigrationTest):
>>>> @@ -88,12 +89,20 @@ def test_ppc64_linux_big_boot(self):
>>>>           wait_for_console_pattern(self, console_pattern, 
>>>> self.panic_message)
>>>>           wait_for_console_pattern(self, self.good_message, 
>>>> self.panic_message)
>>>> -    def test_ppc64_linux_migration(self):
>>>> +    def _do_linux_migration(self, dst_uri, src_uri=None):
>>>> +        """
>>>> +        Boot a pseries kernel on a source VM, wait until Linux is 
>>>> up, then
>>>> +        migrate to a destination VM over the given URI(s) and 
>>>> verify the boot
>>>> +        continues on the destination.
>>>> +        """
>>>>           self.set_machine('pseries')
>>>>           kernel_path = self.ASSET_KERNEL.fetch()
>>>>           kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
>>>> +        if src_uri is None:
>>>> +            src_uri = dst_uri
>>> The default behavior of src_uri=None is non-obvious from the 
>>> docstring.  Could
>>> you mention that it defaults to dst_uri, and that callers using 
>>> transports with
>>> different source and destination URIs (such as the exec: case below) 
>>> need to
>>> provide src_uri explicitly?
>>>
>>>> +
>>>>           dest_vm = self.get_vm(name="dest-qemu")
>>>>           dest_vm.add_args('-incoming', 'defer')
>>>>           dest_vm.add_args('-smp', '4')
>>>> @@ -111,16 +120,38 @@ def test_ppc64_linux_migration(self):
>>>>           source_vm.set_console()
>>>>           source_vm.launch()
>>>> -        # ensure the boot has reached Linux
>>>> +        # ensure the boot has reached Linux before migrating
>>>>           console_pattern = 'smp: Brought up 1 node, 4 CPUs'
>>>>           wait_for_console_pattern(self, console_pattern, 
>>>> self.panic_message,
>>>>                                    vm=source_vm)
>>>> -        self.migration_with_tcp_localhost_vms(dest_vm, source_vm)
>>>> +        self.migrate_vms(dst_uri, src_uri, dest_vm, source_vm)
>>>> -        # ensure the boot proceeds after migration
>>>> +        # ensure the boot continues on the destination after 
>>>> migration
>>>>           wait_for_console_pattern(self, self.good_message, 
>>>> self.panic_message,
>>>>                                    vm=dest_vm)
>>>> +    def test_ppc64_linux_migration(self):
>>>> +        with Ports() as ports:
>>>> +            port = ports.find_free_port()
>>>> +            if port is None:
>>>> +                self.skipTest('Failed to find a free port')
>>>> +            self._do_linux_migration('tcp:localhost:%u' % port)
>>>> +
>>>> +    def test_ppc64_linux_migration_unix(self):
>>>> +        self._do_linux_migration(
>>>> +            'unix:%s/migration.sock' % self.socket_dir().name)
>>>> +
>>>> +    def test_ppc64_linux_migration_exec(self):
>>>> +        if not which('socat'):
>>>> +            self.skipTest('socat is not available')
>>>> +        with Ports() as ports:
>>>> +            port = ports.find_free_port()
>>>> +            if port is None:
>>>> +                self.skipTest('Failed to find a free port')
>>>> +            self._do_linux_migration(
>>>> +                'exec:socat TCP-LISTEN:%u -' % port,
>>>> +                'exec:socat - TCP:localhost:%u,forever' % port)
>>> I share Thomas's concern about CI cost.  With this patch, Linux is 
>>> fully booted
>>> three times — once per transport variant — which significantly 
>>> increases the
>>> wall-clock time of the pseries migration test slot.
>>>
>>> The UNIX and exec transport paths are already covered on ppc64 in
>>> tests/functional/ppc64/test_migration.py, though against mac99 
>>> rather than
>>> pseries.  If the intent is specifically to validate these transports 
>>> on the
>>> pseries machine, that is a reasonable goal — but does it require a 
>>> full Linux
>>> boot?  The transport layer is independent of what the guest is 
>>> running.  A
>>> firmware-only pseries migration (as was done in v1) would cover the
>>> pseries-specific migration path with substantially lower CI cost.
>>>
>>> Please clarify in the commit message why a full Linux boot is needed 
>>> here, or
>>> consider using a firmware-only guest for the two new tests.\
>>
>> Hi Amit, Thomas,
>>
>> I do agree that full boot test cases might be unnecessary, however it 
>> was suggested by Harsh during offline discussions that if a full boot 
>> test already exists for TCP, so perhaps the best course of action 
>> here is to revert back to the firmware-only test cases from v1. I do 
>> need suggestions on if we should extend test_migration.py or make a 
>> new file for the ppc machines.
> I think I'd just turn "mac99" in test_migration_with_unix() and 
> test_migration_with_exec() in test_migration.py into "pseries", and 
> keep the "mac99" in the tcp test there. That should give enough test 
> coverage for both machines in quick test mode.
>
Ack. Writing a follow-up patch with only the following change for 
covering the alternate path tests for pseries.

--- a/tests/functional/ppc64/test_migration.py
+++ b/tests/functional/ppc64/test_migration.py
@@ -15,11 +15,11 @@ def test_migration_with_tcp_localhost(self):
          self.migration_with_tcp_localhost()

      def test_migration_with_unix(self):
-        self.set_machine('mac99')
+        self.set_machine('pseries')
          self.migration_with_unix()

      def test_migration_with_exec(self):
-        self.set_machine('mac99')
+        self.set_machine('pseries')
          self.migration_with_exec()

Regards,
Aniket.