qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/3] UUID validation during migration
@ 2019-09-03 16:22 Yury Kotov
  2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 1/3] migration: Add validate-uuid capability Yury Kotov
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Yury Kotov @ 2019-09-03 16:22 UTC (permalink / raw)
  To: Dr. David Alan Gilbert, Eric Blake, Juan Quintela,
	Laurent Vivier, Markus Armbruster, Paolo Bonzini, Thomas Huth,
	Daniel P. Berrangé
  Cc: qemu-devel, yc-core

Hi,

V2:
* Remove x- prefix from capability name
* Fix expected status checking
* Fix description of capability

This series adds an UUID validation at the start of the migration
on the target side. The idea is to identify the source of migration.

Possible case of problem:
1. There are 3 servers: A, B and C
2. Server A has a VM 1, server B has a VM 2
3. VM 1 and VM 2 want to migrate to the server C
4. Target of VM 1 starts on the server C and dies too quickly for some reason
5. Target of VM 2 starts just after that and listen the same tcp port X, which
   the target of VM 1 wanted to use
6. Source of VM 1 connects to the tcp port X, and migrates to VM 2 source
7. It's possible that migration might be successful (e.g., devices are the same)
8. So, the target of VM 2 is in undefined state

The series adds a capability to prevent successful (by mistake) migration.

The new capability validate-uuid only affects the source so that it sends
its UUID to the target. The target will validate the received UUID and stop
the migration if UUIDs are not equal.

Regards,
Yury

Yury Kotov (3):
  migration: Add validate-uuid capability
  tests/libqtest: Allow setting expected exit status
  tests/migration: Add a test for validate-uuid capability

 migration/migration.c  |   9 +++
 migration/migration.h  |   1 +
 migration/savevm.c     |  45 +++++++++++++
 qapi/migration.json    |   5 +-
 tests/libqtest.c       |  36 ++++++-----
 tests/libqtest.h       |   9 +++
 tests/migration-test.c | 140 ++++++++++++++++++++++++++++++++---------
 7 files changed, 199 insertions(+), 46 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v2 1/3] migration: Add validate-uuid capability
  2019-09-03 16:22 [Qemu-devel] [PATCH v2 0/3] UUID validation during migration Yury Kotov
@ 2019-09-03 16:22 ` Yury Kotov
  2019-09-11  9:15   ` Dr. David Alan Gilbert
  2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 2/3] tests/libqtest: Allow setting expected exit status Yury Kotov
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Yury Kotov @ 2019-09-03 16:22 UTC (permalink / raw)
  To: Dr. David Alan Gilbert, Eric Blake, Juan Quintela,
	Laurent Vivier, Markus Armbruster, Paolo Bonzini, Thomas Huth,
	Daniel P. Berrangé
  Cc: qemu-devel, yc-core

This capability realizes simple source validation by UUID.
It's useful for live migration between hosts.

Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
---
 migration/migration.c |  9 +++++++++
 migration/migration.h |  1 +
 migration/savevm.c    | 45 +++++++++++++++++++++++++++++++++++++++++++
 qapi/migration.json   |  5 ++++-
 4 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 8b9f2fe30a..2391a8d418 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2140,6 +2140,15 @@ bool migrate_ignore_shared(void)
     return s->enabled_capabilities[MIGRATION_CAPABILITY_X_IGNORE_SHARED];
 }
 
+bool migrate_validate_uuid(void)
+{
+    MigrationState *s;
+
+    s = migrate_get_current();
+
+    return s->enabled_capabilities[MIGRATION_CAPABILITY_VALIDATE_UUID];
+}
+
 bool migrate_use_events(void)
 {
     MigrationState *s;
diff --git a/migration/migration.h b/migration/migration.h
index 3e1ea2b5dc..4f2fe193dc 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -290,6 +290,7 @@ bool migrate_postcopy_ram(void);
 bool migrate_zero_blocks(void);
 bool migrate_dirty_bitmaps(void);
 bool migrate_ignore_shared(void);
+bool migrate_validate_uuid(void);
 
 bool migrate_auto_converge(void);
 bool migrate_use_multifd(void);
diff --git a/migration/savevm.c b/migration/savevm.c
index 4a86128ac4..493dc24fd2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -256,6 +256,7 @@ typedef struct SaveState {
     uint32_t target_page_bits;
     uint32_t caps_count;
     MigrationCapability *capabilities;
+    QemuUUID uuid;
 } SaveState;
 
 static SaveState savevm_state = {
@@ -307,6 +308,7 @@ static int configuration_pre_save(void *opaque)
             state->capabilities[j++] = i;
         }
     }
+    state->uuid = qemu_uuid;
 
     return 0;
 }
@@ -464,6 +466,48 @@ static const VMStateDescription vmstate_capabilites = {
     }
 };
 
+static bool vmstate_uuid_needed(void *opaque)
+{
+    return qemu_uuid_set && migrate_validate_uuid();
+}
+
+static int vmstate_uuid_post_load(void *opaque, int version_id)
+{
+    SaveState *state = opaque;
+    char uuid_src[UUID_FMT_LEN + 1];
+    char uuid_dst[UUID_FMT_LEN + 1];
+
+    if (!qemu_uuid_set) {
+        /*
+         * It's warning because user might not know UUID in some cases,
+         * e.g. load an old snapshot
+         */
+        qemu_uuid_unparse(&state->uuid, uuid_src);
+        warn_report("UUID is received %s, but local uuid isn't set",
+                     uuid_src);
+        return 0;
+    }
+    if (!qemu_uuid_is_equal(&state->uuid, &qemu_uuid)) {
+        qemu_uuid_unparse(&state->uuid, uuid_src);
+        qemu_uuid_unparse(&qemu_uuid, uuid_dst);
+        error_report("UUID received is %s and local is %s", uuid_src, uuid_dst);
+        return -EINVAL;
+    }
+    return 0;
+}
+
+static const VMStateDescription vmstate_uuid = {
+    .name = "configuration/uuid",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = vmstate_uuid_needed,
+    .post_load = vmstate_uuid_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8_ARRAY_V(uuid.data, SaveState, sizeof(QemuUUID), 1),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_configuration = {
     .name = "configuration",
     .version_id = 1,
@@ -478,6 +522,7 @@ static const VMStateDescription vmstate_configuration = {
     .subsections = (const VMStateDescription*[]) {
         &vmstate_target_page_bits,
         &vmstate_capabilites,
+        &vmstate_uuid,
         NULL
     }
 };
diff --git a/qapi/migration.json b/qapi/migration.json
index 9cfbaf8c6c..82feb5bd39 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -415,6 +415,9 @@
 #
 # @x-ignore-shared: If enabled, QEMU will not migrate shared memory (since 4.0)
 #
+# @validate-uuid: Send the UUID of the source to allow the destination
+#                 to ensure it is the same. (since 4.2)
+#
 # Since: 1.2
 ##
 { 'enum': 'MigrationCapability',
@@ -422,7 +425,7 @@
            'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
            'block', 'return-path', 'pause-before-switchover', 'multifd',
            'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
-           'x-ignore-shared' ] }
+           'x-ignore-shared', 'validate-uuid' ] }
 
 ##
 # @MigrationCapabilityStatus:
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v2 2/3] tests/libqtest: Allow setting expected exit status
  2019-09-03 16:22 [Qemu-devel] [PATCH v2 0/3] UUID validation during migration Yury Kotov
  2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 1/3] migration: Add validate-uuid capability Yury Kotov
@ 2019-09-03 16:22 ` Yury Kotov
  2019-09-04  4:19   ` Thomas Huth
  2019-09-11  9:46   ` Dr. David Alan Gilbert
  2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 3/3] tests/migration: Add a test for validate-uuid capability Yury Kotov
  2019-09-12 10:20 ` [Qemu-devel] [PATCH v2 0/3] UUID validation during migration Dr. David Alan Gilbert
  3 siblings, 2 replies; 9+ messages in thread
From: Yury Kotov @ 2019-09-03 16:22 UTC (permalink / raw)
  To: Dr. David Alan Gilbert, Eric Blake, Juan Quintela,
	Laurent Vivier, Markus Armbruster, Paolo Bonzini, Thomas Huth,
	Daniel P. Berrangé
  Cc: qemu-devel, yc-core

Add qtest_set_expected_status function to set expected exit status of
child process. By default expected exit status is 0.

Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
---
 tests/libqtest.c | 36 +++++++++++++++++++++---------------
 tests/libqtest.h |  9 +++++++++
 2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 2713b86cf7..a79d4887ae 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -43,6 +43,7 @@ struct QTestState
     int qmp_fd;
     pid_t qemu_pid;  /* our child QEMU process */
     int wstatus;
+    int expected_status;
     bool big_endian;
     bool irq_level[MAX_IRQ];
     GString *rx;
@@ -113,6 +114,11 @@ bool qtest_probe_child(QTestState *s)
     return false;
 }
 
+void qtest_set_expected_status(QTestState *s, int status)
+{
+    s->expected_status = status;
+}
+
 static void kill_qemu(QTestState *s)
 {
     pid_t pid = s->qemu_pid;
@@ -126,24 +132,23 @@ static void kill_qemu(QTestState *s)
     }
 
     /*
-     * We expect qemu to exit with status 0; anything else is
+     * Check whether qemu exited with expected exit status; anything else is
      * fishy and should be logged with as much detail as possible.
      */
     wstatus = s->wstatus;
-    if (wstatus) {
-        if (WIFEXITED(wstatus)) {
-            fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
-                    "process but encountered exit status %d\n",
-                    __FILE__, __LINE__, WEXITSTATUS(wstatus));
-        } else if (WIFSIGNALED(wstatus)) {
-            int sig = WTERMSIG(wstatus);
-            const char *signame = strsignal(sig) ?: "unknown ???";
-            const char *dump = WCOREDUMP(wstatus) ? " (core dumped)" : "";
-
-            fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
-                    "from signal %d (%s)%s\n",
-                    __FILE__, __LINE__, sig, signame, dump);
-        }
+    if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) != s->expected_status) {
+        fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
+                "process but encountered exit status %d (expected %d)\n",
+                __FILE__, __LINE__, WEXITSTATUS(wstatus), s->expected_status);
+        abort();
+    } else if (WIFSIGNALED(wstatus)) {
+        int sig = WTERMSIG(wstatus);
+        const char *signame = strsignal(sig) ?: "unknown ???";
+        const char *dump = WCOREDUMP(wstatus) ? " (core dumped)" : "";
+
+        fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
+                "from signal %d (%s)%s\n",
+                __FILE__, __LINE__, sig, signame, dump);
         abort();
     }
 }
@@ -248,6 +253,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     g_test_message("starting QEMU: %s", command);
 
     s->wstatus = 0;
+    s->expected_status = 0;
     s->qemu_pid = fork();
     if (s->qemu_pid == 0) {
         setenv("QEMU_AUDIO_DRV", "none", true);
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 07ea35867c..c00bca94af 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -997,4 +997,13 @@ void qmp_assert_error_class(QDict *rsp, const char *class);
  */
 bool qtest_probe_child(QTestState *s);
 
+/**
+ * qtest_set_expected_status:
+ * @s: QTestState instance to operate on.
+ * @status: an expected exit status.
+ *
+ * Set expected exit status of the child.
+ */
+void qtest_set_expected_status(QTestState *s, int status);
+
 #endif
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v2 3/3] tests/migration: Add a test for validate-uuid capability
  2019-09-03 16:22 [Qemu-devel] [PATCH v2 0/3] UUID validation during migration Yury Kotov
  2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 1/3] migration: Add validate-uuid capability Yury Kotov
  2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 2/3] tests/libqtest: Allow setting expected exit status Yury Kotov
@ 2019-09-03 16:22 ` Yury Kotov
  2019-09-11 10:02   ` Dr. David Alan Gilbert
  2019-09-12 10:20 ` [Qemu-devel] [PATCH v2 0/3] UUID validation during migration Dr. David Alan Gilbert
  3 siblings, 1 reply; 9+ messages in thread
From: Yury Kotov @ 2019-09-03 16:22 UTC (permalink / raw)
  To: Dr. David Alan Gilbert, Eric Blake, Juan Quintela,
	Laurent Vivier, Markus Armbruster, Paolo Bonzini, Thomas Huth,
	Daniel P. Berrangé
  Cc: qemu-devel, yc-core

Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
---
 tests/migration-test.c | 140 ++++++++++++++++++++++++++++++++---------
 1 file changed, 110 insertions(+), 30 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index b87ba99a9e..524caf773f 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -512,7 +512,8 @@ static void migrate_postcopy_start(QTestState *from, QTestState *to)
 
 static int test_migrate_start(QTestState **from, QTestState **to,
                                const char *uri, bool hide_stderr,
-                               bool use_shmem)
+                               bool use_shmem, const char *opts_src,
+                               const char *opts_dst)
 {
     gchar *cmd_src, *cmd_dst;
     char *bootpath = NULL;
@@ -521,6 +522,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     const char *arch = qtest_get_arch();
     const char *accel = "kvm:tcg";
 
+    opts_src = opts_src ? opts_src : "";
+    opts_dst = opts_dst ? opts_dst : "";
+
     if (use_shmem) {
         if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) {
             g_test_skip("/dev/shm is not supported");
@@ -539,16 +543,16 @@ static int test_migrate_start(QTestState **from, QTestState **to,
         cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
                                   " -name source,debug-threads=on"
                                   " -serial file:%s/src_serial"
-                                  " -drive file=%s,format=raw %s",
+                                  " -drive file=%s,format=raw %s %s",
                                   accel, tmpfs, bootpath,
-                                  extra_opts ? extra_opts : "");
+                                  extra_opts ? extra_opts : "", opts_src);
         cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
                                   " -name target,debug-threads=on"
                                   " -serial file:%s/dest_serial"
                                   " -drive file=%s,format=raw"
-                                  " -incoming %s %s",
+                                  " -incoming %s %s %s",
                                   accel, tmpfs, bootpath, uri,
-                                  extra_opts ? extra_opts : "");
+                                  extra_opts ? extra_opts : "", opts_dst);
         start_address = X86_TEST_MEM_START;
         end_address = X86_TEST_MEM_END;
     } else if (g_str_equal(arch, "s390x")) {
@@ -556,15 +560,15 @@ static int test_migrate_start(QTestState **from, QTestState **to,
         extra_opts = use_shmem ? get_shmem_opts("128M", shmem_path) : NULL;
         cmd_src = g_strdup_printf("-machine accel=%s -m 128M"
                                   " -name source,debug-threads=on"
-                                  " -serial file:%s/src_serial -bios %s %s",
+                                  " -serial file:%s/src_serial -bios %s %s %s",
                                   accel, tmpfs, bootpath,
-                                  extra_opts ? extra_opts : "");
+                                  extra_opts ? extra_opts : "", opts_src);
         cmd_dst = g_strdup_printf("-machine accel=%s -m 128M"
                                   " -name target,debug-threads=on"
                                   " -serial file:%s/dest_serial -bios %s"
-                                  " -incoming %s %s",
+                                  " -incoming %s %s %s",
                                   accel, tmpfs, bootpath, uri,
-                                  extra_opts ? extra_opts : "");
+                                  extra_opts ? extra_opts : "", opts_dst);
         start_address = S390_TEST_MEM_START;
         end_address = S390_TEST_MEM_END;
     } else if (strcmp(arch, "ppc64") == 0) {
@@ -575,14 +579,15 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                   " -prom-env 'use-nvramrc?=true' -prom-env "
                                   "'nvramrc=hex .\" _\" begin %x %x "
                                   "do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
-                                  "until' %s",  accel, tmpfs, end_address,
-                                  start_address, extra_opts ? extra_opts : "");
+                                  "until' %s %s",  accel, tmpfs, end_address,
+                                  start_address, extra_opts ? extra_opts : "",
+                                  opts_src);
         cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
                                   " -name target,debug-threads=on"
                                   " -serial file:%s/dest_serial"
-                                  " -incoming %s %s",
+                                  " -incoming %s %s %s",
                                   accel, tmpfs, uri,
-                                  extra_opts ? extra_opts : "");
+                                  extra_opts ? extra_opts : "", opts_dst);
 
         start_address = PPC_TEST_MEM_START;
         end_address = PPC_TEST_MEM_END;
@@ -592,16 +597,16 @@ static int test_migrate_start(QTestState **from, QTestState **to,
         cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
                                   "-name vmsource,debug-threads=on -cpu max "
                                   "-m 150M -serial file:%s/src_serial "
-                                  "-kernel %s %s",
+                                  "-kernel %s %s %s",
                                   accel, tmpfs, bootpath,
-                                  extra_opts ? extra_opts : "");
+                                  extra_opts ? extra_opts : "", opts_src);
         cmd_dst = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
                                   "-name vmdest,debug-threads=on -cpu max "
                                   "-m 150M -serial file:%s/dest_serial "
                                   "-kernel %s "
-                                  "-incoming %s %s",
+                                  "-incoming %s %s %s",
                                   accel, tmpfs, bootpath, uri,
-                                  extra_opts ? extra_opts : "");
+                                  extra_opts ? extra_opts : "", opts_dst);
 
         start_address = ARM_TEST_MEM_START;
         end_address = ARM_TEST_MEM_END;
@@ -731,7 +736,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
     char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
     QTestState *from, *to;
 
-    if (test_migrate_start(&from, &to, uri, hide_error, false)) {
+    if (test_migrate_start(&from, &to, uri, hide_error, false, NULL, NULL)) {
         return -1;
     }
 
@@ -841,20 +846,16 @@ static void test_postcopy_recovery(void)
     migrate_postcopy_complete(from, to);
 }
 
-static void test_baddest(void)
+static void wait_for_migration_fail(QTestState *from, bool allow_active)
 {
-    QTestState *from, *to;
     QDict *rsp_return;
     char *status;
     bool failed;
 
-    if (test_migrate_start(&from, &to, "tcp:0:0", true, false)) {
-        return;
-    }
-    migrate(from, "tcp:0:0", "{}");
     do {
         status = migrate_query_status(from);
-        g_assert(!strcmp(status, "setup") || !(strcmp(status, "failed")));
+        g_assert(!strcmp(status, "setup") || !strcmp(status, "failed") ||
+                 (allow_active && !strcmp(status, "active")));
         failed = !strcmp(status, "failed");
         g_free(status);
     } while (!failed);
@@ -864,7 +865,17 @@ static void test_baddest(void)
     g_assert(qdict_haskey(rsp_return, "running"));
     g_assert(qdict_get_bool(rsp_return, "running"));
     qobject_unref(rsp_return);
+}
+
+static void test_baddest(void)
+{
+    QTestState *from, *to;
 
+    if (test_migrate_start(&from, &to, "tcp:0:0", true, false, NULL, NULL)) {
+        return;
+    }
+    migrate(from, "tcp:0:0", "{}");
+    wait_for_migration_fail(from, false);
     test_migrate_end(from, to, false);
 }
 
@@ -873,7 +884,7 @@ static void test_precopy_unix(void)
     char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
     QTestState *from, *to;
 
-    if (test_migrate_start(&from, &to, uri, false, false)) {
+    if (test_migrate_start(&from, &to, uri, false, false, NULL, NULL)) {
         return;
     }
 
@@ -916,7 +927,7 @@ static void test_ignore_shared(void)
     char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
     QTestState *from, *to;
 
-    if (test_migrate_start(&from, &to, uri, false, true)) {
+    if (test_migrate_start(&from, &to, uri, false, true, NULL, NULL)) {
         return;
     }
 
@@ -951,7 +962,7 @@ static void test_xbzrle(const char *uri)
 {
     QTestState *from, *to;
 
-    if (test_migrate_start(&from, &to, uri, false, false)) {
+    if (test_migrate_start(&from, &to, uri, false, false, NULL, NULL)) {
         return;
     }
 
@@ -1003,7 +1014,8 @@ static void test_precopy_tcp(void)
     char *uri;
     QTestState *from, *to;
 
-    if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", false, false)) {
+    if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", false, false,
+                           NULL, NULL)) {
         return;
     }
 
@@ -1049,7 +1061,7 @@ static void test_migrate_fd_proto(void)
     QDict *rsp;
     const char *error_desc;
 
-    if (test_migrate_start(&from, &to, "defer", false, false)) {
+    if (test_migrate_start(&from, &to, "defer", false, false, NULL, NULL)) {
         return;
     }
 
@@ -1125,6 +1137,68 @@ static void test_migrate_fd_proto(void)
     test_migrate_end(from, to, true);
 }
 
+static void do_test_validate_uuid(const char *uuid_arg_src,
+                                  const char *uuid_arg_dst,
+                                  bool should_fail, bool hide_stderr)
+{
+    char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+    QTestState *from, *to;
+
+    if (test_migrate_start(&from, &to, uri, hide_stderr, false,
+                           uuid_arg_src, uuid_arg_dst)) {
+        return;
+    }
+
+    /*
+     * UUID validation is at the begin of migration. So, the main process of
+     * migration is not interesting for us here. Thus, set huge downtime for
+     * very fast migration.
+     */
+    migrate_set_parameter_int(from, "downtime-limit", 1000000);
+    migrate_set_capability(from, "validate-uuid", true);
+
+    /* Wait for the first serial output from the source */
+    wait_for_serial("src_serial");
+
+    migrate(from, uri, "{}");
+
+    if (should_fail) {
+        qtest_set_expected_status(to, 1);
+        wait_for_migration_fail(from, true);
+    } else {
+        wait_for_migration_complete(from);
+    }
+
+    test_migrate_end(from, to, false);
+    g_free(uri);
+}
+
+static void test_validate_uuid(void)
+{
+    do_test_validate_uuid("-uuid 11111111-1111-1111-1111-111111111111",
+                          "-uuid 11111111-1111-1111-1111-111111111111",
+                          false, false);
+}
+
+static void test_validate_uuid_error(void)
+{
+    do_test_validate_uuid("-uuid 11111111-1111-1111-1111-111111111111",
+                          "-uuid 22222222-2222-2222-2222-222222222222",
+                          true, true);
+}
+
+static void test_validate_uuid_src_not_set(void)
+{
+    do_test_validate_uuid(NULL, "-uuid 11111111-1111-1111-1111-111111111111",
+                          false, true);
+}
+
+static void test_validate_uuid_dst_not_set(void)
+{
+    do_test_validate_uuid("-uuid 11111111-1111-1111-1111-111111111111", NULL,
+                          false, true);
+}
+
 int main(int argc, char **argv)
 {
     char template[] = "/tmp/migration-test-XXXXXX";
@@ -1180,6 +1254,12 @@ int main(int argc, char **argv)
     /* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
     qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
     qtest_add_func("/migration/fd_proto", test_migrate_fd_proto);
+    qtest_add_func("/migration/validate_uuid", test_validate_uuid);
+    qtest_add_func("/migration/validate_uuid_error", test_validate_uuid_error);
+    qtest_add_func("/migration/validate_uuid_src_not_set",
+                   test_validate_uuid_src_not_set);
+    qtest_add_func("/migration/validate_uuid_dst_not_set",
+                   test_validate_uuid_dst_not_set);
 
     ret = g_test_run();
 
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/3] tests/libqtest: Allow setting expected exit status
  2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 2/3] tests/libqtest: Allow setting expected exit status Yury Kotov
@ 2019-09-04  4:19   ` Thomas Huth
  2019-09-11  9:46   ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 9+ messages in thread
From: Thomas Huth @ 2019-09-04  4:19 UTC (permalink / raw)
  To: Yury Kotov, Dr. David Alan Gilbert, Eric Blake, Juan Quintela,
	Laurent Vivier, Markus Armbruster, Paolo Bonzini,
	Daniel P. Berrangé
  Cc: qemu-devel, yc-core

On 03/09/2019 18.22, Yury Kotov wrote:
> Add qtest_set_expected_status function to set expected exit status of
> child process. By default expected exit status is 0.
> 
> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
> ---
>  tests/libqtest.c | 36 +++++++++++++++++++++---------------
>  tests/libqtest.h |  9 +++++++++
>  2 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 2713b86cf7..a79d4887ae 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -43,6 +43,7 @@ struct QTestState
>      int qmp_fd;
>      pid_t qemu_pid;  /* our child QEMU process */
>      int wstatus;
> +    int expected_status;
>      bool big_endian;
>      bool irq_level[MAX_IRQ];
>      GString *rx;
> @@ -113,6 +114,11 @@ bool qtest_probe_child(QTestState *s)
>      return false;
>  }
>  
> +void qtest_set_expected_status(QTestState *s, int status)
> +{
> +    s->expected_status = status;
> +}
> +
>  static void kill_qemu(QTestState *s)
>  {
>      pid_t pid = s->qemu_pid;
> @@ -126,24 +132,23 @@ static void kill_qemu(QTestState *s)
>      }
>  
>      /*
> -     * We expect qemu to exit with status 0; anything else is
> +     * Check whether qemu exited with expected exit status; anything else is
>       * fishy and should be logged with as much detail as possible.
>       */
>      wstatus = s->wstatus;
> -    if (wstatus) {
> -        if (WIFEXITED(wstatus)) {
> -            fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
> -                    "process but encountered exit status %d\n",
> -                    __FILE__, __LINE__, WEXITSTATUS(wstatus));
> -        } else if (WIFSIGNALED(wstatus)) {
> -            int sig = WTERMSIG(wstatus);
> -            const char *signame = strsignal(sig) ?: "unknown ???";
> -            const char *dump = WCOREDUMP(wstatus) ? " (core dumped)" : "";
> -
> -            fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
> -                    "from signal %d (%s)%s\n",
> -                    __FILE__, __LINE__, sig, signame, dump);
> -        }
> +    if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) != s->expected_status) {
> +        fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
> +                "process but encountered exit status %d (expected %d)\n",
> +                __FILE__, __LINE__, WEXITSTATUS(wstatus), s->expected_status);
> +        abort();
> +    } else if (WIFSIGNALED(wstatus)) {
> +        int sig = WTERMSIG(wstatus);
> +        const char *signame = strsignal(sig) ?: "unknown ???";
> +        const char *dump = WCOREDUMP(wstatus) ? " (core dumped)" : "";
> +
> +        fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
> +                "from signal %d (%s)%s\n",
> +                __FILE__, __LINE__, sig, signame, dump);
>          abort();
>      }
>  }
> @@ -248,6 +253,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>      g_test_message("starting QEMU: %s", command);
>  
>      s->wstatus = 0;
> +    s->expected_status = 0;
>      s->qemu_pid = fork();
>      if (s->qemu_pid == 0) {
>          setenv("QEMU_AUDIO_DRV", "none", true);
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 07ea35867c..c00bca94af 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -997,4 +997,13 @@ void qmp_assert_error_class(QDict *rsp, const char *class);
>   */
>  bool qtest_probe_child(QTestState *s);
>  
> +/**
> + * qtest_set_expected_status:
> + * @s: QTestState instance to operate on.
> + * @status: an expected exit status.
> + *
> + * Set expected exit status of the child.
> + */
> +void qtest_set_expected_status(QTestState *s, int status);
> +
>  #endif
> 

Acked-by: Thomas Huth <thuth@redhat.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/3] migration: Add validate-uuid capability
  2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 1/3] migration: Add validate-uuid capability Yury Kotov
@ 2019-09-11  9:15   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 9+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-11  9:15 UTC (permalink / raw)
  To: Yury Kotov
  Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
	Juan Quintela, Markus Armbruster, qemu-devel, yc-core,
	Paolo Bonzini

* Yury Kotov (yury-kotov@yandex-team.ru) wrote:
> This capability realizes simple source validation by UUID.
> It's useful for live migration between hosts.
> 
> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/migration.c |  9 +++++++++
>  migration/migration.h |  1 +
>  migration/savevm.c    | 45 +++++++++++++++++++++++++++++++++++++++++++
>  qapi/migration.json   |  5 ++++-
>  4 files changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 8b9f2fe30a..2391a8d418 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2140,6 +2140,15 @@ bool migrate_ignore_shared(void)
>      return s->enabled_capabilities[MIGRATION_CAPABILITY_X_IGNORE_SHARED];
>  }
>  
> +bool migrate_validate_uuid(void)
> +{
> +    MigrationState *s;
> +
> +    s = migrate_get_current();
> +
> +    return s->enabled_capabilities[MIGRATION_CAPABILITY_VALIDATE_UUID];
> +}
> +
>  bool migrate_use_events(void)
>  {
>      MigrationState *s;
> diff --git a/migration/migration.h b/migration/migration.h
> index 3e1ea2b5dc..4f2fe193dc 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -290,6 +290,7 @@ bool migrate_postcopy_ram(void);
>  bool migrate_zero_blocks(void);
>  bool migrate_dirty_bitmaps(void);
>  bool migrate_ignore_shared(void);
> +bool migrate_validate_uuid(void);
>  
>  bool migrate_auto_converge(void);
>  bool migrate_use_multifd(void);
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 4a86128ac4..493dc24fd2 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -256,6 +256,7 @@ typedef struct SaveState {
>      uint32_t target_page_bits;
>      uint32_t caps_count;
>      MigrationCapability *capabilities;
> +    QemuUUID uuid;
>  } SaveState;
>  
>  static SaveState savevm_state = {
> @@ -307,6 +308,7 @@ static int configuration_pre_save(void *opaque)
>              state->capabilities[j++] = i;
>          }
>      }
> +    state->uuid = qemu_uuid;
>  
>      return 0;
>  }
> @@ -464,6 +466,48 @@ static const VMStateDescription vmstate_capabilites = {
>      }
>  };
>  
> +static bool vmstate_uuid_needed(void *opaque)
> +{
> +    return qemu_uuid_set && migrate_validate_uuid();
> +}
> +
> +static int vmstate_uuid_post_load(void *opaque, int version_id)
> +{
> +    SaveState *state = opaque;
> +    char uuid_src[UUID_FMT_LEN + 1];
> +    char uuid_dst[UUID_FMT_LEN + 1];
> +
> +    if (!qemu_uuid_set) {
> +        /*
> +         * It's warning because user might not know UUID in some cases,
> +         * e.g. load an old snapshot
> +         */
> +        qemu_uuid_unparse(&state->uuid, uuid_src);
> +        warn_report("UUID is received %s, but local uuid isn't set",
> +                     uuid_src);
> +        return 0;
> +    }
> +    if (!qemu_uuid_is_equal(&state->uuid, &qemu_uuid)) {
> +        qemu_uuid_unparse(&state->uuid, uuid_src);
> +        qemu_uuid_unparse(&qemu_uuid, uuid_dst);
> +        error_report("UUID received is %s and local is %s", uuid_src, uuid_dst);
> +        return -EINVAL;
> +    }
> +    return 0;
> +}
> +
> +static const VMStateDescription vmstate_uuid = {
> +    .name = "configuration/uuid",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = vmstate_uuid_needed,
> +    .post_load = vmstate_uuid_post_load,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8_ARRAY_V(uuid.data, SaveState, sizeof(QemuUUID), 1),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_configuration = {
>      .name = "configuration",
>      .version_id = 1,
> @@ -478,6 +522,7 @@ static const VMStateDescription vmstate_configuration = {
>      .subsections = (const VMStateDescription*[]) {
>          &vmstate_target_page_bits,
>          &vmstate_capabilites,
> +        &vmstate_uuid,
>          NULL
>      }
>  };
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 9cfbaf8c6c..82feb5bd39 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -415,6 +415,9 @@
>  #
>  # @x-ignore-shared: If enabled, QEMU will not migrate shared memory (since 4.0)
>  #
> +# @validate-uuid: Send the UUID of the source to allow the destination
> +#                 to ensure it is the same. (since 4.2)
> +#
>  # Since: 1.2
>  ##
>  { 'enum': 'MigrationCapability',
> @@ -422,7 +425,7 @@
>             'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
>             'block', 'return-path', 'pause-before-switchover', 'multifd',
>             'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
> -           'x-ignore-shared' ] }
> +           'x-ignore-shared', 'validate-uuid' ] }
>  
>  ##
>  # @MigrationCapabilityStatus:
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/3] tests/libqtest: Allow setting expected exit status
  2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 2/3] tests/libqtest: Allow setting expected exit status Yury Kotov
  2019-09-04  4:19   ` Thomas Huth
@ 2019-09-11  9:46   ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 9+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-11  9:46 UTC (permalink / raw)
  To: Yury Kotov
  Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
	Juan Quintela, Markus Armbruster, qemu-devel, yc-core,
	Paolo Bonzini

* Yury Kotov (yury-kotov@yandex-team.ru) wrote:
> Add qtest_set_expected_status function to set expected exit status of
> child process. By default expected exit status is 0.
> 
> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  tests/libqtest.c | 36 +++++++++++++++++++++---------------
>  tests/libqtest.h |  9 +++++++++
>  2 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 2713b86cf7..a79d4887ae 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -43,6 +43,7 @@ struct QTestState
>      int qmp_fd;
>      pid_t qemu_pid;  /* our child QEMU process */
>      int wstatus;
> +    int expected_status;
>      bool big_endian;
>      bool irq_level[MAX_IRQ];
>      GString *rx;
> @@ -113,6 +114,11 @@ bool qtest_probe_child(QTestState *s)
>      return false;
>  }
>  
> +void qtest_set_expected_status(QTestState *s, int status)
> +{
> +    s->expected_status = status;
> +}
> +
>  static void kill_qemu(QTestState *s)
>  {
>      pid_t pid = s->qemu_pid;
> @@ -126,24 +132,23 @@ static void kill_qemu(QTestState *s)
>      }
>  
>      /*
> -     * We expect qemu to exit with status 0; anything else is
> +     * Check whether qemu exited with expected exit status; anything else is
>       * fishy and should be logged with as much detail as possible.
>       */
>      wstatus = s->wstatus;
> -    if (wstatus) {
> -        if (WIFEXITED(wstatus)) {
> -            fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
> -                    "process but encountered exit status %d\n",
> -                    __FILE__, __LINE__, WEXITSTATUS(wstatus));
> -        } else if (WIFSIGNALED(wstatus)) {
> -            int sig = WTERMSIG(wstatus);
> -            const char *signame = strsignal(sig) ?: "unknown ???";
> -            const char *dump = WCOREDUMP(wstatus) ? " (core dumped)" : "";
> -
> -            fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
> -                    "from signal %d (%s)%s\n",
> -                    __FILE__, __LINE__, sig, signame, dump);
> -        }
> +    if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) != s->expected_status) {
> +        fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
> +                "process but encountered exit status %d (expected %d)\n",
> +                __FILE__, __LINE__, WEXITSTATUS(wstatus), s->expected_status);
> +        abort();
> +    } else if (WIFSIGNALED(wstatus)) {
> +        int sig = WTERMSIG(wstatus);
> +        const char *signame = strsignal(sig) ?: "unknown ???";
> +        const char *dump = WCOREDUMP(wstatus) ? " (core dumped)" : "";
> +
> +        fprintf(stderr, "%s:%d: kill_qemu() detected QEMU death "
> +                "from signal %d (%s)%s\n",
> +                __FILE__, __LINE__, sig, signame, dump);
>          abort();
>      }
>  }
> @@ -248,6 +253,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>      g_test_message("starting QEMU: %s", command);
>  
>      s->wstatus = 0;
> +    s->expected_status = 0;
>      s->qemu_pid = fork();
>      if (s->qemu_pid == 0) {
>          setenv("QEMU_AUDIO_DRV", "none", true);
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 07ea35867c..c00bca94af 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -997,4 +997,13 @@ void qmp_assert_error_class(QDict *rsp, const char *class);
>   */
>  bool qtest_probe_child(QTestState *s);
>  
> +/**
> + * qtest_set_expected_status:
> + * @s: QTestState instance to operate on.
> + * @status: an expected exit status.
> + *
> + * Set expected exit status of the child.
> + */
> +void qtest_set_expected_status(QTestState *s, int status);
> +
>  #endif
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v2 3/3] tests/migration: Add a test for validate-uuid capability
  2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 3/3] tests/migration: Add a test for validate-uuid capability Yury Kotov
@ 2019-09-11 10:02   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 9+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-11 10:02 UTC (permalink / raw)
  To: Yury Kotov
  Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
	Juan Quintela, Markus Armbruster, qemu-devel, yc-core,
	Paolo Bonzini

* Yury Kotov (yury-kotov@yandex-team.ru) wrote:
> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  tests/migration-test.c | 140 ++++++++++++++++++++++++++++++++---------
>  1 file changed, 110 insertions(+), 30 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index b87ba99a9e..524caf773f 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -512,7 +512,8 @@ static void migrate_postcopy_start(QTestState *from, QTestState *to)
>  
>  static int test_migrate_start(QTestState **from, QTestState **to,
>                                 const char *uri, bool hide_stderr,
> -                               bool use_shmem)
> +                               bool use_shmem, const char *opts_src,
> +                               const char *opts_dst)
>  {
>      gchar *cmd_src, *cmd_dst;
>      char *bootpath = NULL;
> @@ -521,6 +522,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>      const char *arch = qtest_get_arch();
>      const char *accel = "kvm:tcg";
>  
> +    opts_src = opts_src ? opts_src : "";
> +    opts_dst = opts_dst ? opts_dst : "";
> +
>      if (use_shmem) {
>          if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) {
>              g_test_skip("/dev/shm is not supported");
> @@ -539,16 +543,16 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>          cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
>                                    " -name source,debug-threads=on"
>                                    " -serial file:%s/src_serial"
> -                                  " -drive file=%s,format=raw %s",
> +                                  " -drive file=%s,format=raw %s %s",
>                                    accel, tmpfs, bootpath,
> -                                  extra_opts ? extra_opts : "");
> +                                  extra_opts ? extra_opts : "", opts_src);
>          cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
>                                    " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -drive file=%s,format=raw"
> -                                  " -incoming %s %s",
> +                                  " -incoming %s %s %s",
>                                    accel, tmpfs, bootpath, uri,
> -                                  extra_opts ? extra_opts : "");
> +                                  extra_opts ? extra_opts : "", opts_dst);
>          start_address = X86_TEST_MEM_START;
>          end_address = X86_TEST_MEM_END;
>      } else if (g_str_equal(arch, "s390x")) {
> @@ -556,15 +560,15 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>          extra_opts = use_shmem ? get_shmem_opts("128M", shmem_path) : NULL;
>          cmd_src = g_strdup_printf("-machine accel=%s -m 128M"
>                                    " -name source,debug-threads=on"
> -                                  " -serial file:%s/src_serial -bios %s %s",
> +                                  " -serial file:%s/src_serial -bios %s %s %s",
>                                    accel, tmpfs, bootpath,
> -                                  extra_opts ? extra_opts : "");
> +                                  extra_opts ? extra_opts : "", opts_src);
>          cmd_dst = g_strdup_printf("-machine accel=%s -m 128M"
>                                    " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial -bios %s"
> -                                  " -incoming %s %s",
> +                                  " -incoming %s %s %s",
>                                    accel, tmpfs, bootpath, uri,
> -                                  extra_opts ? extra_opts : "");
> +                                  extra_opts ? extra_opts : "", opts_dst);
>          start_address = S390_TEST_MEM_START;
>          end_address = S390_TEST_MEM_END;
>      } else if (strcmp(arch, "ppc64") == 0) {
> @@ -575,14 +579,15 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                    " -prom-env 'use-nvramrc?=true' -prom-env "
>                                    "'nvramrc=hex .\" _\" begin %x %x "
>                                    "do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
> -                                  "until' %s",  accel, tmpfs, end_address,
> -                                  start_address, extra_opts ? extra_opts : "");
> +                                  "until' %s %s",  accel, tmpfs, end_address,
> +                                  start_address, extra_opts ? extra_opts : "",
> +                                  opts_src);
>          cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
>                                    " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
> -                                  " -incoming %s %s",
> +                                  " -incoming %s %s %s",
>                                    accel, tmpfs, uri,
> -                                  extra_opts ? extra_opts : "");
> +                                  extra_opts ? extra_opts : "", opts_dst);
>  
>          start_address = PPC_TEST_MEM_START;
>          end_address = PPC_TEST_MEM_END;
> @@ -592,16 +597,16 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>          cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
>                                    "-name vmsource,debug-threads=on -cpu max "
>                                    "-m 150M -serial file:%s/src_serial "
> -                                  "-kernel %s %s",
> +                                  "-kernel %s %s %s",
>                                    accel, tmpfs, bootpath,
> -                                  extra_opts ? extra_opts : "");
> +                                  extra_opts ? extra_opts : "", opts_src);
>          cmd_dst = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
>                                    "-name vmdest,debug-threads=on -cpu max "
>                                    "-m 150M -serial file:%s/dest_serial "
>                                    "-kernel %s "
> -                                  "-incoming %s %s",
> +                                  "-incoming %s %s %s",
>                                    accel, tmpfs, bootpath, uri,
> -                                  extra_opts ? extra_opts : "");
> +                                  extra_opts ? extra_opts : "", opts_dst);
>  
>          start_address = ARM_TEST_MEM_START;
>          end_address = ARM_TEST_MEM_END;
> @@ -731,7 +736,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
>      char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
>      QTestState *from, *to;
>  
> -    if (test_migrate_start(&from, &to, uri, hide_error, false)) {
> +    if (test_migrate_start(&from, &to, uri, hide_error, false, NULL, NULL)) {
>          return -1;
>      }
>  
> @@ -841,20 +846,16 @@ static void test_postcopy_recovery(void)
>      migrate_postcopy_complete(from, to);
>  }
>  
> -static void test_baddest(void)
> +static void wait_for_migration_fail(QTestState *from, bool allow_active)
>  {
> -    QTestState *from, *to;
>      QDict *rsp_return;
>      char *status;
>      bool failed;
>  
> -    if (test_migrate_start(&from, &to, "tcp:0:0", true, false)) {
> -        return;
> -    }
> -    migrate(from, "tcp:0:0", "{}");
>      do {
>          status = migrate_query_status(from);
> -        g_assert(!strcmp(status, "setup") || !(strcmp(status, "failed")));
> +        g_assert(!strcmp(status, "setup") || !strcmp(status, "failed") ||
> +                 (allow_active && !strcmp(status, "active")));
>          failed = !strcmp(status, "failed");
>          g_free(status);
>      } while (!failed);
> @@ -864,7 +865,17 @@ static void test_baddest(void)
>      g_assert(qdict_haskey(rsp_return, "running"));
>      g_assert(qdict_get_bool(rsp_return, "running"));
>      qobject_unref(rsp_return);
> +}
> +
> +static void test_baddest(void)
> +{
> +    QTestState *from, *to;
>  
> +    if (test_migrate_start(&from, &to, "tcp:0:0", true, false, NULL, NULL)) {
> +        return;
> +    }
> +    migrate(from, "tcp:0:0", "{}");
> +    wait_for_migration_fail(from, false);
>      test_migrate_end(from, to, false);
>  }
>  
> @@ -873,7 +884,7 @@ static void test_precopy_unix(void)
>      char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
>      QTestState *from, *to;
>  
> -    if (test_migrate_start(&from, &to, uri, false, false)) {
> +    if (test_migrate_start(&from, &to, uri, false, false, NULL, NULL)) {
>          return;
>      }
>  
> @@ -916,7 +927,7 @@ static void test_ignore_shared(void)
>      char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
>      QTestState *from, *to;
>  
> -    if (test_migrate_start(&from, &to, uri, false, true)) {
> +    if (test_migrate_start(&from, &to, uri, false, true, NULL, NULL)) {
>          return;
>      }
>  
> @@ -951,7 +962,7 @@ static void test_xbzrle(const char *uri)
>  {
>      QTestState *from, *to;
>  
> -    if (test_migrate_start(&from, &to, uri, false, false)) {
> +    if (test_migrate_start(&from, &to, uri, false, false, NULL, NULL)) {
>          return;
>      }
>  
> @@ -1003,7 +1014,8 @@ static void test_precopy_tcp(void)
>      char *uri;
>      QTestState *from, *to;
>  
> -    if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", false, false)) {
> +    if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", false, false,
> +                           NULL, NULL)) {
>          return;
>      }
>  
> @@ -1049,7 +1061,7 @@ static void test_migrate_fd_proto(void)
>      QDict *rsp;
>      const char *error_desc;
>  
> -    if (test_migrate_start(&from, &to, "defer", false, false)) {
> +    if (test_migrate_start(&from, &to, "defer", false, false, NULL, NULL)) {
>          return;
>      }
>  
> @@ -1125,6 +1137,68 @@ static void test_migrate_fd_proto(void)
>      test_migrate_end(from, to, true);
>  }
>  
> +static void do_test_validate_uuid(const char *uuid_arg_src,
> +                                  const char *uuid_arg_dst,
> +                                  bool should_fail, bool hide_stderr)
> +{
> +    char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> +    QTestState *from, *to;
> +
> +    if (test_migrate_start(&from, &to, uri, hide_stderr, false,
> +                           uuid_arg_src, uuid_arg_dst)) {
> +        return;
> +    }
> +
> +    /*
> +     * UUID validation is at the begin of migration. So, the main process of
> +     * migration is not interesting for us here. Thus, set huge downtime for
> +     * very fast migration.
> +     */
> +    migrate_set_parameter_int(from, "downtime-limit", 1000000);
> +    migrate_set_capability(from, "validate-uuid", true);
> +
> +    /* Wait for the first serial output from the source */
> +    wait_for_serial("src_serial");
> +
> +    migrate(from, uri, "{}");
> +
> +    if (should_fail) {
> +        qtest_set_expected_status(to, 1);
> +        wait_for_migration_fail(from, true);
> +    } else {
> +        wait_for_migration_complete(from);
> +    }
> +
> +    test_migrate_end(from, to, false);
> +    g_free(uri);
> +}
> +
> +static void test_validate_uuid(void)
> +{
> +    do_test_validate_uuid("-uuid 11111111-1111-1111-1111-111111111111",
> +                          "-uuid 11111111-1111-1111-1111-111111111111",
> +                          false, false);
> +}
> +
> +static void test_validate_uuid_error(void)
> +{
> +    do_test_validate_uuid("-uuid 11111111-1111-1111-1111-111111111111",
> +                          "-uuid 22222222-2222-2222-2222-222222222222",
> +                          true, true);
> +}
> +
> +static void test_validate_uuid_src_not_set(void)
> +{
> +    do_test_validate_uuid(NULL, "-uuid 11111111-1111-1111-1111-111111111111",
> +                          false, true);
> +}
> +
> +static void test_validate_uuid_dst_not_set(void)
> +{
> +    do_test_validate_uuid("-uuid 11111111-1111-1111-1111-111111111111", NULL,
> +                          false, true);
> +}
> +
>  int main(int argc, char **argv)
>  {
>      char template[] = "/tmp/migration-test-XXXXXX";
> @@ -1180,6 +1254,12 @@ int main(int argc, char **argv)
>      /* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
>      qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
>      qtest_add_func("/migration/fd_proto", test_migrate_fd_proto);
> +    qtest_add_func("/migration/validate_uuid", test_validate_uuid);
> +    qtest_add_func("/migration/validate_uuid_error", test_validate_uuid_error);
> +    qtest_add_func("/migration/validate_uuid_src_not_set",
> +                   test_validate_uuid_src_not_set);
> +    qtest_add_func("/migration/validate_uuid_dst_not_set",
> +                   test_validate_uuid_dst_not_set);
>  
>      ret = g_test_run();
>  
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/3] UUID validation during migration
  2019-09-03 16:22 [Qemu-devel] [PATCH v2 0/3] UUID validation during migration Yury Kotov
                   ` (2 preceding siblings ...)
  2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 3/3] tests/migration: Add a test for validate-uuid capability Yury Kotov
@ 2019-09-12 10:20 ` Dr. David Alan Gilbert
  3 siblings, 0 replies; 9+ messages in thread
From: Dr. David Alan Gilbert @ 2019-09-12 10:20 UTC (permalink / raw)
  To: Yury Kotov
  Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
	Juan Quintela, Markus Armbruster, qemu-devel, yc-core,
	Paolo Bonzini

* Yury Kotov (yury-kotov@yandex-team.ru) wrote:
> Hi,

> V2:
> * Remove x- prefix from capability name
> * Fix expected status checking
> * Fix description of capability
> 
> This series adds an UUID validation at the start of the migration
> on the target side. The idea is to identify the source of migration.
> 
> Possible case of problem:
> 1. There are 3 servers: A, B and C
> 2. Server A has a VM 1, server B has a VM 2
> 3. VM 1 and VM 2 want to migrate to the server C
> 4. Target of VM 1 starts on the server C and dies too quickly for some reason
> 5. Target of VM 2 starts just after that and listen the same tcp port X, which
>    the target of VM 1 wanted to use
> 6. Source of VM 1 connects to the tcp port X, and migrates to VM 2 source
> 7. It's possible that migration might be successful (e.g., devices are the same)
> 8. So, the target of VM 2 is in undefined state
> 
> The series adds a capability to prevent successful (by mistake) migration.
> 
> The new capability validate-uuid only affects the source so that it sends
> its UUID to the target. The target will validate the received UUID and stop
> the migration if UUIDs are not equal.
> 
> Regards,
> Yury

Queued

> Yury Kotov (3):
>   migration: Add validate-uuid capability
>   tests/libqtest: Allow setting expected exit status
>   tests/migration: Add a test for validate-uuid capability
> 
>  migration/migration.c  |   9 +++
>  migration/migration.h  |   1 +
>  migration/savevm.c     |  45 +++++++++++++
>  qapi/migration.json    |   5 +-
>  tests/libqtest.c       |  36 ++++++-----
>  tests/libqtest.h       |   9 +++
>  tests/migration-test.c | 140 ++++++++++++++++++++++++++++++++---------
>  7 files changed, 199 insertions(+), 46 deletions(-)
> 
> -- 
> 2.17.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-09-12 10:22 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-03 16:22 [Qemu-devel] [PATCH v2 0/3] UUID validation during migration Yury Kotov
2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 1/3] migration: Add validate-uuid capability Yury Kotov
2019-09-11  9:15   ` Dr. David Alan Gilbert
2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 2/3] tests/libqtest: Allow setting expected exit status Yury Kotov
2019-09-04  4:19   ` Thomas Huth
2019-09-11  9:46   ` Dr. David Alan Gilbert
2019-09-03 16:22 ` [Qemu-devel] [PATCH v2 3/3] tests/migration: Add a test for validate-uuid capability Yury Kotov
2019-09-11 10:02   ` Dr. David Alan Gilbert
2019-09-12 10:20 ` [Qemu-devel] [PATCH v2 0/3] UUID validation during migration Dr. David Alan Gilbert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).