Re: [PATCH 11/22] tests/qtest/migration: Move common test code

[Fabiano Rosas <farosas@suse.de>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Nov 05, 2024 at 03:08:26PM -0300, Fabiano Rosas wrote:
>> Put the common test code in a separate file. Leave only individual
>> test functions and their static helpers in migration-test.c.
>> 
>> This moves the shared:
>> 
>>  test_postcopy_common
>>  test_postcopy_recovery_common
>>  test_precopy_common
>>  test_file_common
>>  migrate_precopy_tcp_multifd_start_common
>>  migrate_start
>>  migrate_end
>>  migration_get_env
>>  migration_env_clean
>> 
>> and some of their static helpers to migrate-common.c.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  tests/qtest/meson.build                  |    1 +
>>  tests/qtest/migration-test.c             | 1124 +---------------------
>>  tests/qtest/migration/bootfile.c         |    2 +-
>>  tests/qtest/migration/bootfile.h         |    2 +-
>>  tests/qtest/migration/migration-common.c |  963 ++++++++++++++++++
>>  tests/qtest/migration/migration-common.h |  214 ++++
>>  tests/qtest/migration/migration-util.h   |   13 -
>
> Looking at these I guess I'd ask the question of what is the rule for
> deciding whether to put something in -common vs in -util ?
>
> Both names sounds like being a general "bag of bits", so it isn't
> obvious to me what their distinct roles are.
>

The idea was to make -util the general bag of bits just like
migration-helpers is currently.

-common should be the more "important" migration-specific functions,
code that we spend more time reading and that are actually responsible
for the results of the tests. Consider:

static void test_multifd_file_mapped_ram_fdset_dio(void)
{
    ...
    if (!probe_o_direct_support(tmpfs)) {          <--- just a helper
        g_test_skip("Filesystem does not support O_DIRECT");
        return;
    }

    test_file_common(&args, true);  <--- actually doing the heavy-lifting
}

We could to some renaming and restructuring, but I think the distinction
is important. We already have it today, what is -common is all stuffed
into migration-test.c and the rest is in migration-helpers.c.

>>  7 files changed, 1193 insertions(+), 1126 deletions(-)
>>  create mode 100644 tests/qtest/migration/migration-common.c
>>  create mode 100644 tests/qtest/migration/migration-common.h
>> 
>
> With regards,
> Daniel