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

[Fabiano Rosas <farosas@suse.de>]

Steven Sistare <steven.sistare@oracle.com> writes:

> 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);

That's better, but still needs one unref for the listen_uri != defer path.

>
> - Steve