Re: [PATCH V7 23/24] migration-test: cpr-transfer

[Steven Sistare <steven.sistare@oracle.com>]

On 1/16/2025 2:06 PM, Fabiano Rosas wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
> 
[...]
>> +    /*
>> +     * The cpr channel must be included in outgoing channels, but not in
>> +     * migrate-incoming channels.
>> +     */
>>       if (args->connect_channels) {
>> +        in_channels = qobject_from_json(args->connect_channels, &error_abort);
>>           out_channels = qobject_from_json(args->connect_channels, &error_abort);
>> +
>> +        if (args->cpr_channel) {
>> +            QList *channels_list = qobject_to(QList, out_channels);
>> +            QObject *obj = migrate_str_to_channel(args->cpr_channel);
>> +
>> +            qlist_append(channels_list, obj);
>> +        }
>>       }
>>   
>>       if (args->result == MIG_TEST_QMP_ERROR) {
>> @@ -735,6 +751,9 @@ void test_precopy_common(MigrateCommon *args)
>>       if (args->start.defer_target_connect) {
>>           qtest_connect(to);
>>           qtest_qmp_handshake(to);
>> +        if (!strcmp(args->listen_uri, "defer")) {
>> +            migrate_incoming_qmp(to, args->connect_uri, in_channels, "{}");
>> +        }
> 
> Paths that don't call migrate_incoming_qmp() never free
> in_channels. We'll need something like this, let me know if I can squash
> it in or you want to do it differently:
> 
> -- >8 --
>  From 62d60c39b3e5d38cac20241e63b9d023bd296d2f Mon Sep 17 00:00:00 2001
> From: Fabiano Rosas <farosas@suse.de>
> Date: Thu, 16 Jan 2025 15:40:22 -0300
> Subject: [PATCH] fixup! migration-test: cpr-transfer
> 
> ---
>   tests/qtest/migration/framework.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> index 699bedae69..1d5918d922 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -753,9 +753,14 @@ void test_precopy_common(MigrateCommon *args)
>           qtest_qmp_handshake(to);
>           if (!strcmp(args->listen_uri, "defer")) {
>               migrate_incoming_qmp(to, args->connect_uri, in_channels, "{}");
> +            in_channels = NULL;
>           }
>       }
>   
> +    if (in_channels) {
> +        qobject_unref(in_channels);
> +    }
> +
>       if (args->result != MIG_TEST_SUCCEED) {
>           bool allow_active = args->result == MIG_TEST_FAIL;
>           wait_for_migration_fail(from, allow_active);

Thank-you, though it would be more direct to avoid creating in_channels when
not needed:

     if (args->connect_channels) {
         if (args->start.defer_target_connect) {
             in_channels = qobject_from_json(args->connect_channels,
                                             &error_abort);
         }
         out_channels = qobject_from_json(args->connect_channels, &error_abort);

- Steve