Re: [PATCH] qtest: migration: Add failure test for 'uri' and 'channels' combination in 'migrate' QAPI

[Het Gala]

I wanted to share an update regarding the patch I've been working on. It 
seems that the patch is not yet fully ready as it encountered some 
issues during the check-qtest builds.

This is my first attempt at writing a test case related to migration, 
and I'm aware that there may be areas where I could use some guidance. 
If there are any gaps in my understanding of how to properly mock a 
migration or if there are any other issues with the test case, I would 
greatly appreciate your assistance. I'm also struggling to understand 
why the test is failing. If anyone could provide some insight or 
assistance with troubleshooting, it would be greatly appreciated.

I've cc'd Fabino, Daniel, as I believe they may have expertise in 
migration testing and could offer some valuable insights.

Thank you for your help with this, and I look forward to any feedback or 
assistance you can provide.

On 09/02/24 1:21 pm, Het Gala wrote:
Ensure failure occurs while adding validation test for 'uri' and 'channels' 
arguments
used simultaneously in the 'migrate' QAPI command.

Signed-off-by: Het Gala <het.g...@nutanix.com>
---
  tests/qtest/migration-helpers.c | 14 ++++++--
  tests/qtest/migration-helpers.h |  5 +--
  tests/qtest/migration-test.c    | 60 +++++++++++++++++++++++++++++++--
  3 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index e451dbdbed..2dbb01e413 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -43,7 +43,8 @@ bool migrate_watch_for_events(QTestState *who, const char 
*name,
      return false;
  }
  
-void migrate_qmp_fail(QTestState *who, const char *uri, const char *fmt, ...)
+void migrate_qmp_fail(QTestState *who, const char *uri,
+                      const char *channels, const char *fmt, ...)
  {
      va_list ap;
      QDict *args, *err;
@@ -52,8 +53,15 @@ void migrate_qmp_fail(QTestState *who, const char *uri, 
const char *fmt, ...)
      args = qdict_from_vjsonf_nofail(fmt, ap);
      va_end(ap);
  
-    g_assert(!qdict_haskey(args, "uri"));
-    qdict_put_str(args, "uri", uri);
+    if (uri) {
+        g_assert(!qdict_haskey(args, "uri"));
+        qdict_put_str(args, "uri", uri);
+    }
+
+    if (channels) {
+        g_assert(!qdict_haskey(args, "channels"));
+        qdict_put_str(args, "channels", channels);
+    }
  
      err = qtest_qmp_assert_failure_ref(
          who, "{ 'execute': 'migrate', 'arguments': %p}", args);
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index 3bf7ded1b9..d49e289c51 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -32,8 +32,9 @@ G_GNUC_PRINTF(3, 4)
  void migrate_incoming_qmp(QTestState *who, const char *uri,
                            const char *fmt, ...);
  
-G_GNUC_PRINTF(3, 4)
-void migrate_qmp_fail(QTestState *who, const char *uri, const char *fmt, ...);
+G_GNUC_PRINTF(4, 5)
+void migrate_qmp_fail(QTestState *who, const char *uri,
+                      const char *channels, const char *fmt, ...);
  
  void migrate_set_capability(QTestState *who, const char *capability,
                              bool value);
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d3066e119f..3aaffc2667 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -18,6 +18,7 @@
  #include "qemu/module.h"
  #include "qemu/option.h"
  #include "qemu/range.h"
+#include "migration/migration.h"
  #include "qemu/sockets.h"
  #include "chardev/char.h"
  #include "qapi/qapi-visit-sockets.h"
@@ -1773,7 +1774,7 @@ static void test_precopy_common(MigrateCommon *args)
      }
  
      if (args->result == MIG_TEST_QMP_ERROR) {
-        migrate_qmp_fail(from, connect_uri, "{}");
+        migrate_qmp_fail(from, connect_uri, NULL, "{}");
          goto finish;
      }
  
@@ -1869,7 +1870,7 @@ static void test_file_common(MigrateCommon *args, bool stop_src)
      }
  
      if (args->result == MIG_TEST_QMP_ERROR) {
-        migrate_qmp_fail(from, connect_uri, "{}");
+        migrate_qmp_fail(from, connect_uri, NULL, "{}");
          goto finish;
      }
  
@@ -2508,6 +2509,59 @@ static void test_validate_uuid_dst_not_set(void)
      do_test_validate_uuid(&args, false);
  }
  
+static void do_test_validate_uri_channel(MigrateCommon *args, bool should_fail)
Not sure if should_fail is of any value here. The test ideally should 
not enter migration also. Should just fail even before making the 
connection, at the QMP level itself. I added it here, by taking the 
reference of validate_uuid tests.
+{
+    g_autofree const char *uri = "127.0.0.1:0";
+    g_autofree const char *channels = "{"
+               "   'channels': [ { 'channel-type': 'main',"
+               "                   'addr': { 'transport': 'socket',"
+               "                             'type': 'inet',"
+               "                             'host': '127.0.0.1',"
+               "                             'port': '0' } } ] }";
+    QTestState *from, *to;
+
+    if (test_migrate_start(&from, &to, uri, &args->start)) {
+        return;
+    }
+
+    /* Wait for the first serial output from the source */
+    wait_for_serial("src_serial");
+
+    /*
+     * 'uri' and 'channels' validation is checked even before the migration
+     * starts.
+     */
+    if (args->result == MIG_TEST_QMP_ERROR) {
+        migrate_qmp_fail(from, uri, channels, "{}");
+        goto finish;
+    }
+
+    migrate_qmp(from, uri, "{}");
+
+    if (should_fail) {
+        qtest_set_expected_status(to, EXIT_FAILURE);
+        wait_for_migration_fail(from, false);
+    } else {
+        wait_for_migration_complete(from);
+    }
+
+finish:
+    test_migrate_end(from, to, args->result == MIG_TEST_QMP_ERROR);
+}
+
+static void
+test_validate_uri_channel_both_set(void)
+{
+    MigrateCommon args = {
+        .start = {
+            .hide_stderr = true,
+        },
+        .result = MIG_TEST_QMP_ERROR,
+    };
+
+    do_test_validate_uri_channel(&args, true);
+}
+
  /*
   * The way auto_converge works, we need to do too many passes to
   * run this test.  Auto_converge logic is only run once every
@@ -3536,6 +3590,8 @@ int main(int argc, char **argv)
                         test_validate_uuid_src_not_set);
      migration_test_add("/migration/validate_uuid_dst_not_set",
                         test_validate_uuid_dst_not_set);
+    migration_test_add("/migration/validate_uri_channel_both_set",
+                       test_validate_uri_channel_both_set);
      /*
       * See explanation why this test is slow on function definition
       */