Re: [PATCH 3/4] tests/qtest/migration: Add channels parameter in migrate_incoming_qmp

Thu, 11 Apr 2024 05:40:38 -0700 


On 10/04/24 6:44 pm, Fabiano Rosas wrote:

!-------------------------------------------------------------------|
   CAUTION: External Email

|-------------------------------------------------------------------!

Het Gala<het.g...@nutanix.com>  writes:


Alter migrate_incoming_qmp() to allow both uri and channels
independently. For channels, convert string to a QDict.

Signed-off-by: Het Gala<het.g...@nutanix.com>
---
  tests/qtest/migration-helpers.c   | 13 +++++++++++--
  tests/qtest/migration-helpers.h   |  4 ++--
  tests/qtest/migration-test.c      | 12 ++++++------
  tests/qtest/virtio-net-failover.c |  8 ++++----
  4 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 3b72cad6c1..cbd91719ae 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -245,7 +245,8 @@ void migrate_set_capability(QTestState *who, const char 
*capability,
                               capability, value);
  }
-void migrate_incoming_qmp(QTestState *to, const char *uri, const char *fmt, ...) 
+void migrate_incoming_qmp(QTestState *to, const char *uri,
+                          const char *channels, const char *fmt, ...)
  {
      va_list ap;
      QDict *args, *rsp, *data;
@@ -255,7 +256,15 @@ void migrate_incoming_qmp(QTestState *to, const char *uri, 
const char *fmt, ...)
      va_end(ap);
g_assert(!qdict_haskey(args, "uri")); 
-    qdict_put_str(args, "uri", uri);
+    if (uri) {
+        qdict_put_str(args, "uri", uri);
+    }
+
+    g_assert(!qdict_haskey(args, "channels"));
+    if (channels) {
+        QObject *channels_obj = qobject_from_json(channels, &error_abort);
+        qdict_put_obj(args, "channels", channels_obj);
+    }

Do you think it makes sense to have channels take precedence here? It
would make the next patch cleaner without having to check for channels
presence. I don't think we need a 'both' test for incoming.


Yes, this hack would silently solve, avoiding the above check.

IMO, it is good to have like to like QAPIs while running a qtest.
If the qtest uses the new syntax then, both 'migrate' and 'migrate-incoming'
QAPIs should use the new syntax. Even though implementation wise, it makes
no difference (qtests run successfully) but it would be good to ensure, we have 
grammatical correctness.
migrate_set_capability(to, "events", true); diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h 
index 1339835698..9f74281aea 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -29,9 +29,9 @@ G_GNUC_PRINTF(5, 6)
  void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
                   const char *channels, const char *fmt, ...);
-G_GNUC_PRINTF(3, 4) 
+G_GNUC_PRINTF(4, 5)
  void migrate_incoming_qmp(QTestState *who, const char *uri,
-                          const char *fmt, ...);
+                          const char *channels, const char *fmt, ...);
G_GNUC_PRINTF(4, 5) 
  void migrate_qmp_fail(QTestState *who, const char *uri,
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index f2c27d611c..fa8a860811 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1296,7 +1296,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
      migrate_ensure_non_converge(from);
migrate_prepare_for_dirty_mem(from); 
-    migrate_incoming_qmp(to, "tcp:127.0.0.1:0", "{}");
+    migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}");
/* Wait for the first serial output from the source */ 
      wait_for_serial("src_serial");
@@ -1824,7 +1824,7 @@ static void test_file_common(MigrateCommon *args, bool 
stop_src)
       * We need to wait for the source to finish before starting the
       * destination.
       */
-    migrate_incoming_qmp(to, args->connect_uri, "{}");
+    migrate_incoming_qmp(to, args->connect_uri, NULL, "{}");
      wait_for_migration_complete(to);
if (stop_src) { 
@@ -2405,7 +2405,7 @@ static void *test_migrate_fd_start_hook(QTestState *from,
      close(pair[0]);
/* Start incoming migration from the 1st socket */ 
-    migrate_incoming_qmp(to, "fd:fd-mig", "{}");
+    migrate_incoming_qmp(to, "fd:fd-mig", NULL, "{}");
/* Send the 2nd socket to the target */ 
      qtest_qmp_fds_assert_success(from, &pair[1], 1,
@@ -2715,7 +2715,7 @@ test_migrate_precopy_tcp_multifd_start_common(QTestState 
*from,
      migrate_set_capability(to, "multifd", true);
/* Start incoming migration from the 1st socket */ 
-    migrate_incoming_qmp(to, "tcp:127.0.0.1:0", "{}");
+    migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}");
return NULL; 
  }
@@ -3040,7 +3040,7 @@ static void test_multifd_tcp_cancel(void)
      migrate_set_capability(to, "multifd", true);
/* Start incoming migration from the 1st socket */ 
-    migrate_incoming_qmp(to, "tcp:127.0.0.1:0", "{}");
+    migrate_incoming_qmp(to, "tcp:127.0.0.1:0", NULL, "{}");
/* Wait for the first serial output from the source */ 
      wait_for_serial("src_serial");
@@ -3068,7 +3068,7 @@ static void test_multifd_tcp_cancel(void)
      migrate_set_capability(to2, "multifd", true);
/* Start incoming migration from the 1st socket */ 
-    migrate_incoming_qmp(to2, "tcp:127.0.0.1:0", "{}");
+    migrate_incoming_qmp(to2, "tcp:127.0.0.1:0", NULL, "{}");
wait_for_migration_status(from, "cancelled", NULL); diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c 
index 73dfabc272..e263ecd80e 100644
--- a/tests/qtest/virtio-net-failover.c
+++ b/tests/qtest/virtio-net-failover.c
@@ -772,7 +772,7 @@ static void test_migrate_in(gconstpointer opaque)
      check_one_card(qts, true, "standby0", MAC_STANDBY0);
      check_one_card(qts, false, "primary0", MAC_PRIMARY0);
- migrate_incoming_qmp(qts, uri, "{}"); 
+    migrate_incoming_qmp(qts, uri, NULL, "{}");
resp = get_failover_negociated_event(qts); 
      g_assert_cmpstr(qdict_get_str(resp, "device-id"), ==, "standby0");
@@ -894,7 +894,7 @@ static void test_off_migrate_in(gconstpointer opaque)
      check_one_card(qts, true, "standby0", MAC_STANDBY0);
      check_one_card(qts, true, "primary0", MAC_PRIMARY0);
- migrate_incoming_qmp(qts, uri, "{}"); 
+    migrate_incoming_qmp(qts, uri, NULL, "{}");
check_one_card(qts, true, "standby0", MAC_STANDBY0); 
      check_one_card(qts, true, "primary0", MAC_PRIMARY0);
@@ -1021,7 +1021,7 @@ static void test_guest_off_migrate_in(gconstpointer 
opaque)
      check_one_card(qts, true, "standby0", MAC_STANDBY0);
      check_one_card(qts, false, "primary0", MAC_PRIMARY0);
- migrate_incoming_qmp(qts, uri, "{}"); 
+    migrate_incoming_qmp(qts, uri, NULL, "{}");
check_one_card(qts, true, "standby0", MAC_STANDBY0); 
      check_one_card(qts, false, "primary0", MAC_PRIMARY0);
@@ -1746,7 +1746,7 @@ static void test_multi_in(gconstpointer opaque)
      check_one_card(qts, true, "standby1", MAC_STANDBY1);
      check_one_card(qts, false, "primary1", MAC_PRIMARY1);
- migrate_incoming_qmp(qts, uri, "{}"); 
+    migrate_incoming_qmp(qts, uri, NULL, "{}");
resp = get_failover_negociated_event(qts); 
      g_assert_cmpstr(qdict_get_str(resp, "device-id"), ==, "standby0");

Regards,
Het Gala

Reply via email to