Re: [PATCH v2 4/4] tests/functional/ppc64: Remove custom migration routine

[Fabiano Rosas <farosas@suse.de>]

Thomas Huth <thuth@redhat.com> writes:

> On 04/02/2026 20.51, Peter Xu wrote:
>> On Wed, Feb 04, 2026 at 02:23:32PM -0300, Fabiano Rosas wrote:
>>> Don't implement a custom migration routine at PpcMigrationTest and
>>> instead reuse the generic one from MigrationTest.
>>>
>>> This removes the dependency of PpcMigrationTest from
>>> PseriesMachine. Having one test import another causes unittest code to
>>> instantiate the imported test, resulting in the setup and teardown
>>> methods being invoked for the imported test class, even if no test
>>> from that class will be executed.
>>>
>>> If run in parallel, the extra setup/teardown methods that result from
>>> importing can race with the ones from the actual test being executed
>>> and cause the following error:
>>>
>>> File "<SRC_DIR>/tests/functional/qemu_test/testcase.py", line 238, in tearDown
>>> shutil.rmtree(self.workdir)
>>> ...
>>> FileNotFoundError: [Errno 2] No such file or directory:
>>> '<SRC_DIR>/build/tests/functional/ppc64/.../test_migration_with_exec/scratch'
>>>
>>> Fixes: f4e34d0fd5 ("tests/functional: Add a OS level migration test for pseries")
>>> Reported-by: Aditya Gupta <adityag@linux.ibm.com>
>>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>> ---
>>>   tests/functional/migration.py            |  5 +++++
>>>   tests/functional/ppc64/test_migration.py | 11 -----------
>>>   tests/functional/ppc64/test_pseries.py   |  6 ++++--
>>>   3 files changed, 9 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/tests/functional/migration.py b/tests/functional/migration.py
>>> index 49347a30bb..e995328e83 100644
>>> --- a/tests/functional/migration.py
>>> +++ b/tests/functional/migration.py
>>> @@ -65,6 +65,11 @@ def _get_free_port(self, ports):
>>>               self.skipTest('Failed to find a free port')
>>>           return port
>>>   
>>> +    def migration_with_tcp_localhost_vms(self, dst_vm, src_vm):
>>> +        with Ports() as ports:
>>> +            uri = 'tcp:localhost:%u' % self._get_free_port(ports)
>>> +            self.migrate_vms(uri, uri, dst_vm, src_vm)
>>> +
>>>       def migration_with_tcp_localhost(self):
>>>           with Ports() as ports:
>>>               dst_uri = 'tcp:localhost:%u' % self._get_free_port(ports)
>>> diff --git a/tests/functional/ppc64/test_migration.py b/tests/functional/ppc64/test_migration.py
>>> index a3b819680b..7d49ee175b 100755
>>> --- a/tests/functional/ppc64/test_migration.py
>>> +++ b/tests/functional/ppc64/test_migration.py
>>> @@ -22,17 +22,6 @@ def test_migration_with_exec(self):
>>>           self.set_machine('mac99')
>>>           self.migration_with_exec()
>>>   
>>> -    def do_migrate_ppc64_linux(self, source_vm, dest_vm):
>>> -        with Ports() as ports:
>>> -            port = ports.find_free_port()
>>> -            if port is None:
>>> -                self.skipTest('Failed to find a free port')
>>> -            uri = 'tcp:localhost:%u' % port
>>> -
>>> -            dest_vm.qmp('migrate-incoming', uri=uri)
>>> -            source_vm.qmp('migrate', uri=uri)
>>> -            self.assert_migration(source_vm, dest_vm)
>>> -
>>>   
>>>   if __name__ == '__main__':
>>>       MigrationTest.main()
>>> diff --git a/tests/functional/ppc64/test_pseries.py b/tests/functional/ppc64/test_pseries.py
>>> index b45763c305..3996a4a878 100755
>>> --- a/tests/functional/ppc64/test_pseries.py
>>> +++ b/tests/functional/ppc64/test_pseries.py
>>> @@ -9,7 +9,7 @@
>>>   
>>>   from qemu_test import QemuSystemTest, Asset
>>>   from qemu_test import wait_for_console_pattern
>>> -from test_migration import PpcMigrationTest
>>> +from migration import MigrationTest
>>>   
>>>   class PseriesMachine(QemuSystemTest):
>> 
>> I wonder if PseriesMachine would be better to inherit directly from
>> MigrationTest, to avoid the timeout setup and temp QemuSystemTest to be
>> created on the fly (I'm not familiar with the test framework on side
>> effects).  But I don't see a major issue with it either so far.
>
> I think I'd also prefer to inherit directly from MigrationTest.
> You could put the test_ppc64_linux_migration() method into a separate class 
> that inherits from MigrationTest, while the other test_* methods stay in the 
> old class, so that there is for sure no impact on the other tests.
> (Having multiple test classes in one file is fine, we have that in a couple 
> of other places already).
>

A new class will make it cumbersome to access the class attributes from
PseriesMachine:

    KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 console=hvc0 '
    panic_message = 'Kernel panic - not syncing'
    good_message = 'VFS: Cannot open root device'
    ASSET_KERNEL = Asset(...

The "best" way is to make the whole class inherit from MigrationTest. If
you're ok with that I can send a v3.

>   Thomas
>
>
>>>   
>>> @@ -116,7 +116,9 @@ def test_ppc64_linux_migration(self):
>>>           wait_for_console_pattern(self, console_pattern, self.panic_message,
>>>                                    vm=source_vm)
>>>   
>>> -        PpcMigrationTest().do_migrate_ppc64_linux(source_vm, dest_vm);
>>> +        mt = MigrationTest()
>>> +        mt.timeout = self.timeout
>>> +        mt.migration_with_tcp_localhost_vms(dest_vm, source_vm);
>>>   
>>>           # ensure the boot proceeds after migration
>>>           wait_for_console_pattern(self, self.good_message, self.panic_message,
>>> -- 
>>> 2.51.0
>>>
>>