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

[Steven Sistare]

On 1/16/2025 3:02 PM, Fabiano Rosas wrote:

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

On 1/16/2025 2:06 PM, Fabiano Rosas wrote:

Steve Sistare <steven.sist...@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 <faro...@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.

OK, then:

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

Or keep your fix.  I have no preference.

- Steve