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

[Steven Sistare]

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