Re: [PATCH 6/6] tests/qtest: migration-test: Add tests for file-based migration

[Fabiano Rosas <farosas@suse.de>]

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

> On Mon, Jun 26, 2023 at 03:22:10PM -0300, Fabiano Rosas wrote:
>> From: Nikolay Borisov <nborisov@suse.com>
>> 
>> Add basic tests for file-based migration.
>> 
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  tests/qtest/migration-test.c | 66 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 66 insertions(+)
>> 
>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>> index acb778a8cd..5a77257a53 100644
>> --- a/tests/qtest/migration-test.c
>> +++ b/tests/qtest/migration-test.c
>> @@ -763,6 +763,7 @@ static void test_migrate_end(QTestState *from, QTestState *to, bool test_dest)
>>      cleanup("migsocket");
>>      cleanup("src_serial");
>>      cleanup("dest_serial");
>> +    cleanup("migfile");
>>  }
>>  
>>  #ifdef CONFIG_GNUTLS
>> @@ -1460,11 +1461,28 @@ static void test_precopy_common(MigrateCommon *args)
>>               */
>>              wait_for_migration_complete(from);
>>  
>> +            /*
>> +             * For file based migration the target must begin its
>> +             * migration after the source has finished.
>> +             */
>> +            if (strstr(connect_uri, "file:")) {
>> +                migrate_incoming_qmp(to, connect_uri, "{}");
>> +            }
>> +
>>              if (!got_src_stop) {
>>                  qtest_qmp_eventwait(from, "STOP");
>>              }
>>          } else {
>>              wait_for_migration_complete(from);
>> +
>> +            /*
>> +             * For file based migration the target must begin its
>> +             * migration after the source has finished.
>> +             */
>> +            if (strstr(connect_uri, "file:")) {
>> +                migrate_incoming_qmp(to, connect_uri, "{}");
>> +            }
>> +
>>              /*
>>               * Must wait for dst to finish reading all incoming
>>               * data on the socket before issuing 'cont' otherwise
>> @@ -1682,6 +1700,46 @@ static void test_precopy_unix_compress_nowait(void)
>>      test_precopy_common(&args);
>>  }
>>  
>> +static void test_precopy_file(void)
>> +{
>> +    g_autofree char *uri = g_strdup_printf("file:%s/migfile", tmpfs);
>
> There's no unlink(), so I presume you're relying on the entire 'tmpfs'
> being recursively deleted ?
>

The first hunk adds a call to cleanup(), which has the unlink().

>> +    MigrateCommon args = {
>> +        .connect_uri = uri,
>> +        .listen_uri = "defer",
>> +    };
>> +
>> +    test_precopy_common(&args);
>> +}
>> +
>> +static void test_precopy_file_offset(void)
>> +{
>> +    g_autofree char *uri = g_strdup_printf("file:%s/migfile,offset=0x1000",
>> +                                           tmpfs);
>> +    MigrateCommon args = {
>> +        .connect_uri = uri,
>> +        .listen_uri = "defer",
>> +    };
>> +
>> +    test_precopy_common(&args);
>
> There ought to be something here that values 0->0x1000 bytes are
> all zeroes in the file.
>

Good idea, thanks.