qtest: add migration testing

PATCH Superseded Jason Baron 4 participants 14 messages Dec 13, 2012 – Dec 19, 2012

lore

Share & tools

lore

Jason Baron

author

2012-12-13 22:02 UTC #0 lore raw

cover qtest: add migration testing

Hi,

Add a basic qtest for migration testing. Currently, it just tests a migrate of machine 'pc' on the same host. Would be nice to extend to multiple machine versions, but that requires multiple binaries, which could be done, but is perhaps a bit awkward from qtest? Testing different machine versions within the same binary doesn't seem like a real world test case to me. Currently, the test aborts, if the migrate takes more than 2 minutes.

In any case, the test currently fails for q35, since ahci migration suport isn't in place. Thus, I intend to add q35 testing here, once those ahci migration patches are accepted.

Thanks,

-Jason

Jason Baron (3):
qtest: Enable creation of multiple qemu instances
qtest: extend qtest_qmp() to fill in the reply
qtest: add migrate-test

 tests/Makefile       |    8 ++-
 tests/libqtest.c     |   48 ++++++++++--------
 tests/libqtest.h     |    4 +-
 tests/migrate-test.c |  140 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 175 insertions(+), 25 deletions(-)
 create mode 100644 tests/migrate-test.c

Jason Baron

author

2012-12-13 22:02 UTC #1 lore raw

PATCH 1/3 qtest: Enable creation of multiple qemu instances

From: Jason Baron <[email protected]>

Currently, the qtest harness can only spawn 1 qemu instance at a time because the parent pid is used to create the socket files. Use the child pid instead, so we can remove that limitation.

Signed-off-by Jason Baron <[email protected]>

---
 tests/libqtest.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

tests/libqtest.c +17 ~~-14~~

index 71b84c1..f3dd4e4 100644
@@ -101,6 +101,10 @@ static pid_t qtest_qemu_pid(QTestState *s)
     return pid;
 }
 
+#define QTEST_FILE_TEMP "/tmp/qtest-%d.sock"
+#define QTEST_QMP_FILE_TEMP "/tmp/qtest-%d.qmp"
+#define QTEST_PID_FILE_TEMP "/tmp/qtest-%d.pid"
+
 QTestState *qtest_init(const char *extra_args)
 {
     QTestState *s;

@@ -113,25 +117,16 @@ QTestState *qtest_init(const char *extra_args)
     qemu_binary = getenv("QTEST_QEMU_BINARY");
     g_assert(qemu_binary != NULL);
 
-    s = g_malloc(sizeof(*s));
-
-    s->socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
-    s->qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
-    pid_file = g_strdup_printf("/tmp/qtest-%d.pid", getpid());
-
-    sock = init_socket(s->socket_path);
-    qmpsock = init_socket(s->qmp_socket_path);
-
     pid = fork();
     if (pid == 0) {
         command = g_strdup_printf("%s "
-                                  "-qtest unix:%s,nowait "
+                                  "-qtest unix:" QTEST_FILE_TEMP ",nowait "
                                   "-qtest-log /dev/null "
-                                  "-qmp unix:%s,nowait "
-                                  "-pidfile %s "
+                                  "-qmp unix:" QTEST_QMP_FILE_TEMP ",nowait "
+                                  "-pidfile " QTEST_PID_FILE_TEMP " "
                                   "-machine accel=qtest "
-                                  "%s", qemu_binary, s->socket_path,
-                                  s->qmp_socket_path, pid_file,
+                                  "%s", qemu_binary, getpid(),
+                                  getpid(), getpid(),
                                   extra_args ?: "");
 
         ret = system(command);

@@ -139,6 +134,14 @@ QTestState *qtest_init(const char *extra_args)
         g_free(command);
     }
 
+    s = g_malloc(sizeof(*s));
+    s->socket_path = g_strdup_printf(QTEST_FILE_TEMP, pid);
+    s->qmp_socket_path = g_strdup_printf(QTEST_QMP_FILE_TEMP, pid);
+    pid_file = g_strdup_printf(QTEST_PID_FILE_TEMP, pid);
+
+    sock = init_socket(s->socket_path);
+    qmpsock = init_socket(s->qmp_socket_path);
+
     s->fd = socket_accept(sock);
     s->qmp_fd = socket_accept(qmpsock);

Signature · 1 line

1.7.1

Blue Swirl

2012-12-14 20:30 UTC #2 lore raw

Re: [Qemu-devel] [PATCH 1/3] qtest: Enable creation of multiple qemu instances

Quoted 16 lines · Jason Baron

From: Jason Baron <[email protected]>
Currently, the qtest harness can only spawn 1 qemu instance at a time because the parent pid is used to create the socket files. Use the child pid instead, so we can remove that limitation.
Signed-off-by Jason Baron <[email protected]>
---
 tests/libqtest.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)
tests/libqtest.c +0 -0
index 71b84c1..f3dd4e4 100644

tests/libqtest.c static pid_t qtest_qemu_pid(QTestState *s) · line 106

Quoted 6 lines

tests/libqtest.c +3 -0

@@ -101,6 +101,10 @@ static pid_t qtest_qemu_pid(QTestState *s)
     return pid;
 }
 
+#define QTEST_FILE_TEMP "/tmp/qtest-%d.sock"
+#define QTEST_QMP_FILE_TEMP "/tmp/qtest-%d.qmp"
+#define QTEST_PID_FILE_TEMP "/tmp/qtest-%d.pid"

These filenames are too predictable from security point of view, please change the code to use mkstemp() instead. That would also solve the original file name conflict problem.

Quoted 54 lines

tests/libqtest.c +14 ~~-15~~

+
 QTestState *qtest_init(const char *extra_args)
 {
     QTestState *s;

@@ -113,25 +117,16 @@ QTestState *qtest_init(const char *extra_args)
     qemu_binary = getenv("QTEST_QEMU_BINARY");
     g_assert(qemu_binary != NULL);
 
-    s = g_malloc(sizeof(*s));
-
-    s->socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
-    s->qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
-    pid_file = g_strdup_printf("/tmp/qtest-%d.pid", getpid());
-
-    sock = init_socket(s->socket_path);
-    qmpsock = init_socket(s->qmp_socket_path);
-
     pid = fork();
     if (pid == 0) {
         command = g_strdup_printf("%s "
-                                  "-qtest unix:%s,nowait "
+                                  "-qtest unix:" QTEST_FILE_TEMP ",nowait "
                                   "-qtest-log /dev/null "
-                                  "-qmp unix:%s,nowait "
-                                  "-pidfile %s "
+                                  "-qmp unix:" QTEST_QMP_FILE_TEMP ",nowait "
+                                  "-pidfile " QTEST_PID_FILE_TEMP " "
                                   "-machine accel=qtest "
-                                  "%s", qemu_binary, s->socket_path,
-                                  s->qmp_socket_path, pid_file,
+                                  "%s", qemu_binary, getpid(),
+                                  getpid(), getpid(),
                                   extra_args ?: "");
 
         ret = system(command);

@@ -139,6 +134,14 @@ QTestState *qtest_init(const char *extra_args)
         g_free(command);
     }
 
+    s = g_malloc(sizeof(*s));
+    s->socket_path = g_strdup_printf(QTEST_FILE_TEMP, pid);
+    s->qmp_socket_path = g_strdup_printf(QTEST_QMP_FILE_TEMP, pid);
+    pid_file = g_strdup_printf(QTEST_PID_FILE_TEMP, pid);
+
+    sock = init_socket(s->socket_path);
+    qmpsock = init_socket(s->qmp_socket_path);
+
     s->fd = socket_accept(sock);
     s->qmp_fd = socket_accept(qmpsock);
 
--

1.7.1

Paolo Bonzini

2012-12-15 09:14 UTC #3 lore raw

Re: [Qemu-devel] [PATCH 1/3] qtest: Enable creation of multiple qemu instances

Quoted 5 lines

tests/libqtest.c static pid_t qtest_qemu_pid(QTestState *s) · line 106
Quoted 3 lines
tests/libqtest.c +3 -0
+#define QTEST_FILE_TEMP "/tmp/qtest-%d.sock"
+#define QTEST_QMP_FILE_TEMP "/tmp/qtest-%d.qmp"
+#define QTEST_PID_FILE_TEMP "/tmp/qtest-%d.pid"
These filenames are too predictable from security point of view,

This need not be secure as long as the file is created with 0600 permissions. In fact, inspecting the pid file from the shell can be useful.

However, using mkstemp() on a prefix that includes the parent pid can indeed be the best of both worlds.

Paolo

Blue Swirl

2012-12-15 09:20 UTC #4 lore raw

Re: [Qemu-devel] [PATCH 1/3] qtest: Enable creation of multiple qemu instances

Quoted 9 lines · Paolo Bonzini

Quoted 5 lines
tests/libqtest.c static pid_t qtest_qemu_pid(QTestState *s) · line 106
Quoted 3 lines
tests/libqtest.c +3 -0
+#define QTEST_FILE_TEMP "/tmp/qtest-%d.sock"
+#define QTEST_QMP_FILE_TEMP "/tmp/qtest-%d.qmp"
+#define QTEST_PID_FILE_TEMP "/tmp/qtest-%d.pid"
These filenames are too predictable from security point of view,
This need not be secure as long as the file is created with 0600 permissions. In fact, inspecting the pid file from the shell can be useful.

Permissions do not help at all because the attacker could for example target overwriting of a critical file.

Quoted 3 lines

However, using mkstemp() on a prefix that includes the parent pid can indeed be the best of both worlds.

Yes.

Quoted 2 lines

Paolo

Jason Baron

author

2012-12-17 17:14 UTC #5 lore raw

Re: [Qemu-devel] [PATCH 1/3] qtest: Enable creation of multiple qemu instances

Quoted 11 lines · Blue Swirl

Quoted 9 lines · Paolo Bonzini
Quoted 5 lines
tests/libqtest.c static pid_t qtest_qemu_pid(QTestState *s) · line 106
Quoted 3 lines
tests/libqtest.c +3 -0
+#define QTEST_FILE_TEMP "/tmp/qtest-%d.sock"
+#define QTEST_QMP_FILE_TEMP "/tmp/qtest-%d.qmp"
+#define QTEST_PID_FILE_TEMP "/tmp/qtest-%d.pid"
These filenames are too predictable from security point of view,
This need not be secure as long as the file is created with 0600 permissions. In fact, inspecting the pid file from the shell can be useful.

Quoted 12 lines

Permissions do not help at all because the attacker could for example target overwriting of a critical file.
Quoted 3 lines
However, using mkstemp() on a prefix that includes the parent pid can indeed be the best of both worlds.
Yes.
Quoted 2 lines
Paolo

Yes, but mkstemp() creates the file, and bind() returns EADDRINUSE, if the file already exists.

Using mktemp() in this case, with bind() should be ok, since bind() checks if the file exists and then creates it, if not, all within the bind() system call (so its atomic).

Thanks,

-Jason

Blue Swirl

2012-12-19 19:43 UTC #6 lore raw

Re: [Qemu-devel] [PATCH 1/3] qtest: Enable creation of multiple qemu instances

Quoted 25 lines · Jason Baron

Quoted 11 lines · Blue Swirl
Quoted 9 lines · Paolo Bonzini
Quoted 5 lines
tests/libqtest.c static pid_t qtest_qemu_pid(QTestState *s) · line 106
Quoted 3 lines
tests/libqtest.c +3 -0
+#define QTEST_FILE_TEMP "/tmp/qtest-%d.sock"
+#define QTEST_QMP_FILE_TEMP "/tmp/qtest-%d.qmp"
+#define QTEST_PID_FILE_TEMP "/tmp/qtest-%d.pid"
These filenames are too predictable from security point of view,
This need not be secure as long as the file is created with 0600 permissions. In fact, inspecting the pid file from the shell can be useful.
Quoted 12 lines
Permissions do not help at all because the attacker could for example target overwriting of a critical file.
Quoted 3 lines
However, using mkstemp() on a prefix that includes the parent pid can indeed be the best of both worlds.
Yes.
Quoted 2 lines
Paolo

Quoted 6 lines

Yes, but mkstemp() creates the file, and bind() returns EADDRINUSE, if the file already exists.
Using mktemp() in this case, with bind() should be ok, since bind() checks if the file exists and then creates it, if not, all within the bind() system call (so its atomic).

mktemp() manual page warns against using it, tempnam() looks like a better choice.

Quoted 4 lines

Thanks,
-Jason

Jason Baron

author

2012-12-13 22:02 UTC #7 lore raw

PATCH 2/3 qtest: extend qtest_qmp() to fill in the reply

In reply to Jason Baron

From: Jason Baron <[email protected]>

Introduce:

Add void qtest_qmp_resp(QTestState *s, QString *resp, const char *fmt, ...)

which allows a response string to be filled in.

Signed-off-by Jason Baron <[email protected]>

---
 tests/Makefile   |    6 +++---
 tests/libqtest.c |   17 ++++++++++-------
 tests/libqtest.h |    4 +++-
 3 files changed, 16 insertions(+), 11 deletions(-)

tests/Makefile +3 -3

index b60f0fb..30a101d 100644
@@ -74,10 +74,10 @@ tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-
 tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y)
 tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y)
 
-tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y)
+tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y) qstring.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y)
-tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y)
-tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y)
+tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y) qstring.o
+tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y) qstring.o
 
 # QTest rules

tests/libqtest.c +10 -7

index f3dd4e4..71c9eb4 100644
@@ -288,7 +288,7 @@ redo:
     return words;
 }
 
-void qtest_qmp(QTestState *s, const char *fmt, ...)
+void qtest_qmp_resp(QTestState *s, QString *resp, const char *fmt, ...)
 {
     va_list ap;
     bool has_reply = false;

@@ -313,16 +313,19 @@ void qtest_qmp(QTestState *s, const char *fmt, ...)
             fprintf(stderr, "Broken pipe\n");
             exit(1);
         }
-
-        switch (c) {
-        case '{':
+        if (c == '{') {
             nesting++;
             has_reply = true;
-            break;
-        case '}':
+        }
+        if (c == '}') {
             nesting--;
-            break;
         }
+        if (has_reply && resp) {
+            qstring_append_chr(resp, c);
+        }
+    }
+    if (has_reply && resp) {
+        qstring_append_chr(resp, '\0');
     }
 }

tests/libqtest.h +3 -1

index c8ade85..6441e50 100644
@@ -18,6 +18,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include <sys/types.h>
+#include "qstring.h"
 
 typedef struct QTestState QTestState;

@@ -44,7 +45,8 @@ void qtest_quit(QTestState *s);
  *
  * Sends a QMP message to QEMU
  */
-void qtest_qmp(QTestState *s, const char *fmt, ...);
+void qtest_qmp_resp(QTestState *s, QString *resp, const char *fmt, ...);
+#define qtest_qmp(s, fmt, ...) qtest_qmp_resp(s, NULL, fmt, ## __VA_ARGS__)
 
 /**
  * qtest_get_irq:

Signature · 1 line

1.7.1

Andreas Färber

2012-12-14 00:07 UTC #8 lore raw

Re: [Qemu-devel] [PATCH 2/3] qtest: extend qtest_qmp() to fill in the reply

Am 13.12.2012 23:02, schrieb Jason Baron:

Quoted 5 lines

tests/Makefile +0 -0
index b60f0fb..30a101d 100644

tests/Makefile tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi- · line 80

Quoted 10 lines

tests/Makefile +3 -3

@@ -74,10 +74,10 @@ tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-
 tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y)
 tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y)
 
-tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y)
+tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y) qstring.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y)
-tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y)
-tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y)
+tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y) qstring.o
+tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y) qstring.o

Adding a dependency to every qtest is calling for a $(qtest-obj-y) to administer that list in a central location. I am expecting the number of tests to grow significantly over time.

Andreas

Signature · 2 lines

SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Jason Baron

author

2012-12-14 16:10 UTC #9 lore raw

Re: [Qemu-devel] [PATCH 2/3] qtest: extend qtest_qmp() to fill in the reply

Quoted 21 lines · Andreas Färber

Am 13.12.2012 23:02, schrieb Jason Baron:
Quoted 5 lines
tests/Makefile +0 -0
index b60f0fb..30a101d 100644
tests/Makefile tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi- · line 80
Quoted 10 lines
tests/Makefile +3 -3
@@ -74,10 +74,10 @@ tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-
 tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y)
 tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y)
 
-tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y)
+tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y) qstring.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y)
-tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y)
-tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y)
+tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y) qstring.o
+tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y) qstring.o
Adding a dependency to every qtest is calling for a $(qtest-obj-y) to administer that list in a central location. I am expecting the number of tests to grow significantly over time.

Quoted 2 lines

Andreas

ok, seems as though we can add $(test-qapi-obj-y) $(qom-obj-y), etc. to the qtest-obj-y target, to simplify things a bit. I'll re-post that as a separate patch.

Thanks,

-Jason

Jason Baron

author

2012-12-13 22:02 UTC #10 lore raw

PATCH 3/3 qtest: add migrate-test

In reply to Jason Baron

From: Jason Baron <[email protected]>

Tests a single 'pc' machine migration on the same host. Currently, the test fail for q35 since the ahci controller doesn't yet migrate. Will add support for q35 once the ahci support is accepted.

Would be nice to extend the test matrix to various machine versions, but that requires building multiple qemu binaries, which is a bit awkward in the context of qtest. Testing migration between different machine versions with the same binary doesn't seem too useful.

Signed-off-by Jason Baron <[email protected]>

---
 tests/Makefile       |    2 +
 tests/migrate-test.c |  140 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 142 insertions(+), 0 deletions(-)
 create mode 100644 tests/migrate-test.c

tests/Makefile +2 -0

index 30a101d..d50dff0 100644
@@ -25,6 +25,7 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 check-qtest-i386-y = tests/fdc-test$(EXESUF)
 check-qtest-i386-y += tests/hd-geo-test$(EXESUF)
 check-qtest-i386-y += tests/rtc-test$(EXESUF)
+check-qtest-i386-y += tests/migrate-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
 check-qtest-sparc64-y = tests/m48t59-test$(EXESUF)

@@ -78,6 +79,7 @@ tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y) qstring.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y)
 tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y) qstring.o
 tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y) qstring.o
+tests/migrate-test$(EXESUF): tests/migrate-test.o $(test-qapi-obj-y) $(qom-obj-y)
 
 # QTest rules

tests/migrate-test.c new file +140 -0

140 lines not on this page · raw message

Signature · 1 line

1.7.1

Paolo Bonzini

2012-12-14 09:13 UTC #11 lore raw

Re: [Qemu-devel] [PATCH 3/3] qtest: add migrate-test

----- Messaggio originale -----

Quoted 154 lines

Da: "Jason Baron" <[email protected]>
A: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Inviato: Giovedì, 13 dicembre 2012 23:02:22
Oggetto: [PATCH 3/3] qtest: add migrate-test
From: Jason Baron <[email protected]>
Tests a single 'pc' machine migration on the same host. Currently, the test
fail for q35 since the ahci controller doesn't yet migrate. Will add support
for q35 once the ahci support is accepted.
Would be nice to extend the test matrix to various machine versions, but that
requires building multiple qemu binaries, which is a bit awkward in the
context of qtest. Testing migration between different machine versions with the
same binary doesn't seem too useful.
Signed-off-by Jason Baron <[email protected]>
---
 tests/Makefile       |    2 +
 tests/migrate-test.c |  140
 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 142 insertions(+), 0 deletions(-)
 create mode 100644 tests/migrate-test.c
tests/Makefile +0 -0
index 30a101d..d50dff0 100644
@@ -25,6 +25,7 @@ check-block-$(CONFIG_POSIX) +=
tests/qemu-iotests-quick.sh
check-qtest-i386-y = tests/fdc-test$(EXESUF)
check-qtest-i386-y += tests/hd-geo-test$(EXESUF)
check-qtest-i386-y += tests/rtc-test$(EXESUF)
tests/Makefile +1 -0
+check-qtest-i386-y += tests/migrate-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
 check-qtest-sparc64-y = tests/m48t59-test$(EXESUF)
@@ -78,6 +79,7 @@ tests/rtc-test$(EXESUF): tests/rtc-test.o
$(trace-obj-y) qstring.o
tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y) tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y) qstring.o
tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y) qstring.o
+tests/migrate-test$(EXESUF): tests/migrate-test.o $(test-qapi-obj-y) $(qom-obj-y)
# QTest rules
tests/migrate-test.c new file +9 -0
new file mode 100644
index 0000000..c62d5af
@@ -0,0 +1,140 @@
+/*
+ * Migration tests
+ *
+ * Copyright Red Hat, Inc. 2012
+ *
+ * Authors:
+ *  Jason Baron   <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2
or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
tests/migrate-test.c new file +58 -0
+#include "libqtest.h"
+
+#include <glib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "qjson.h"
+#include "error.h"
+#include "qemu/object.h"
+#include "qdict.h"
+#include "qbool.h"
+
+#define migrate_assert(cond) \
+    if (!(cond)) {           \
+        migrate_cleanup();   \
+        fprintf(stderr, "%s:%d %s\n", __FILE__, __LINE__, #cond); \
+        abort();             \
+    }                        \
+
+static QTestState *mach_a;
+static QTestState *mach_b;
+
+static void migrate_cleanup(void)
+{
+    if (mach_a) {
+        qtest_quit(mach_a);
+    }
+    if (mach_b) {
+        qtest_quit(mach_b);
+    }
+}
+
+static int expected_qobject(QObject *obj, qtype_code type)
+{
+    if (!obj) {
+        return 0;
+    }
+    return (qobject_type(obj) == type);
+}
+
+/*
+ * Return vals:
+ * 1: yes
+ * 0: no
+ * -1: retry
+ */
+static int is_running(QTestState *mch)
+{
+    QString *resp = qstring_new();
+    QObject *resp_obj;
+    QObject *ret_obj;
+    QObject *run_obj;
+    int ret;
+
+    resp = qstring_new();
+    qtest_qmp_resp(mch, resp, "{ 'execute': 'query-status' }",
NULL);
tests/migrate-test.c new file +21 -0
+
+    resp_obj = qobject_from_json(qstring_get_str(resp));
+    if (!expected_qobject(resp_obj, QTYPE_QDICT)) {
+        ret = -1;
+        goto out;
+    }
+
+    ret_obj = qdict_get(qobject_to_qdict(resp_obj), "return");
+    if (!expected_qobject(ret_obj, QTYPE_QDICT)) {
+        ret = -1;
+        goto out;
+    }
+
+    run_obj = qdict_get(qobject_to_qdict(ret_obj), "running");
+    if (!expected_qobject(run_obj, QTYPE_QBOOL)) {
+        ret = -1;
+        goto out;
+    }
+    ret = qbool_get_int(qobject_to_qbool(run_obj));
+
+out:

tests/migrate-test.c line 101

Quoted 10 lines

tests/migrate-test.c new file +10 -0

+    qobject_decref(resp_obj);
+    QDECREF(resp);
+    return ret;
+}
+
+#define SLEEP_INTERVAL 2
+/* Abort after 2 minutes */
+#define SLEEP_MAX (60 * 2)
+
+static void migrate_a_to_b(void)

Do you think this function could be turned into a libqtest call? It would take mach_a as an argument, add -incoming tcp:localhost:4444 to the command line of mach_a, use that to spawn mach_b, and return mach_b as the return value (or perhaps change mach_a to refer to the new machine).

The reason is that I can anticipate having many migration qtests, at least one for every subsection we ever had to add.

Paolo

Quoted 44 lines

tests/migrate-test.c new file +32 -0

+{
+    int a_run = 0;
+    int b_run = 0;
+    int iter = 0;
+
+    /* is running on A ? */
+    migrate_assert(is_running(mach_a));
+
+    /* do migrate */
+    qtest_qmp(mach_a, "{ 'execute': 'migrate',"
+              "'arguments': { 'uri': 'tcp:0:4444' } }", NULL);
+
+    while (iter < SLEEP_MAX) {
+        a_run = is_running(mach_a);
+        b_run = is_running(mach_b);
+        if ((a_run == 0) && (b_run == 1)) {
+            break;
+        }
+        sleep(SLEEP_INTERVAL);
+        iter += SLEEP_INTERVAL;
+    }
+    migrate_assert((a_run == 0) && (b_run == 1));
+}
+
+int main(int argc, char **argv)
+{
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    mach_a = qtest_start("-display none -machine pc");
+    mach_b = qtest_start("-display none -machine pc -incoming

tcp:0:4444");

tests/migrate-test.c new file +7 -1

+
+    qtest_add_func("/migrate/a-to-b", migrate_a_to_b);
+    ret = g_test_run();
+
+    migrate_cleanup();
+    return ret;
+}
--

1.7.1

Jason Baron

author

2012-12-14 16:14 UTC #12 lore raw

Re: [Qemu-devel] [PATCH 3/3] qtest: add migrate-test

Quoted 150 lines · Paolo Bonzini

Quoted 150 lines

Tests a single 'pc' machine migration on the same host. Currently, the test
fail for q35 since the ahci controller doesn't yet migrate. Will add support
for q35 once the ahci support is accepted.

Would be nice to extend the test matrix to various machine versions, but that
requires building multiple qemu binaries, which is a bit awkward in the
context of qtest. Testing migration between different machine versions with the
same binary doesn't seem too useful.

Signed-off-by Jason Baron <[email protected]>

---
 tests/Makefile       |    2 +
 tests/migrate-test.c |  140
 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 142 insertions(+), 0 deletions(-)
 create mode 100644 tests/migrate-test.c

tests/Makefile +0 -0

index 30a101d..d50dff0 100644
@@ -25,6 +25,7 @@ check-block-$(CONFIG_POSIX) +=

tests/qemu-iotests-quick.sh
check-qtest-i386-y = tests/fdc-test$(EXESUF)
check-qtest-i386-y += tests/hd-geo-test$(EXESUF)
check-qtest-i386-y += tests/rtc-test$(EXESUF)

tests/Makefile +1 -0

+check-qtest-i386-y += tests/migrate-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
 check-qtest-sparc64-y = tests/m48t59-test$(EXESUF)

@@ -78,6 +79,7 @@ tests/rtc-test$(EXESUF): tests/rtc-test.o

$(trace-obj-y) qstring.o
tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y) tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y) qstring.o
tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y) qstring.o
+tests/migrate-test$(EXESUF): tests/migrate-test.o $(test-qapi-obj-y) $(qom-obj-y)

# QTest rules

tests/migrate-test.c new file +9 -0

new file mode 100644
index 0000000..c62d5af
@@ -0,0 +1,140 @@
+/*
+ * Migration tests
+ *
+ * Copyright Red Hat, Inc. 2012
+ *
+ * Authors:
+ *  Jason Baron   <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2

or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */

tests/migrate-test.c new file +58 -0

+#include "libqtest.h"
+
+#include <glib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "qjson.h"
+#include "error.h"
+#include "qemu/object.h"
+#include "qdict.h"
+#include "qbool.h"
+
+#define migrate_assert(cond) \
+    if (!(cond)) {           \
+        migrate_cleanup();   \
+        fprintf(stderr, "%s:%d %s\n", __FILE__, __LINE__, #cond); \
+        abort();             \
+    }                        \
+
+static QTestState *mach_a;
+static QTestState *mach_b;
+
+static void migrate_cleanup(void)
+{
+    if (mach_a) {
+        qtest_quit(mach_a);
+    }
+    if (mach_b) {
+        qtest_quit(mach_b);
+    }
+}
+
+static int expected_qobject(QObject *obj, qtype_code type)
+{
+    if (!obj) {
+        return 0;
+    }
+    return (qobject_type(obj) == type);
+}
+
+/*
+ * Return vals:
+ * 1: yes
+ * 0: no
+ * -1: retry
+ */
+static int is_running(QTestState *mch)
+{
+    QString *resp = qstring_new();
+    QObject *resp_obj;
+    QObject *ret_obj;
+    QObject *run_obj;
+    int ret;
+
+    resp = qstring_new();
+    qtest_qmp_resp(mch, resp, "{ 'execute': 'query-status' }",

NULL);

tests/migrate-test.c new file +25 -0

+
+    resp_obj = qobject_from_json(qstring_get_str(resp));
+    if (!expected_qobject(resp_obj, QTYPE_QDICT)) {
+        ret = -1;
+        goto out;
+    }
+
+    ret_obj = qdict_get(qobject_to_qdict(resp_obj), "return");
+    if (!expected_qobject(ret_obj, QTYPE_QDICT)) {
+        ret = -1;
+        goto out;
+    }
+
+    run_obj = qdict_get(qobject_to_qdict(ret_obj), "running");
+    if (!expected_qobject(run_obj, QTYPE_QBOOL)) {
+        ret = -1;
+        goto out;
+    }
+    ret = qbool_get_int(qobject_to_qbool(run_obj));
+
+out:
+    qobject_decref(resp_obj);
+    QDECREF(resp);
+    return ret;
+}

Quoted 8 lines

tests/migrate-test.c line 101
Quoted 6 lines
tests/migrate-test.c new file +6 -0
+
+#define SLEEP_INTERVAL 2
+/* Abort after 2 minutes */
+#define SLEEP_MAX (60 * 2)
+
+static void migrate_a_to_b(void)
Do you think this function could be turned into a libqtest call?

Seems like a good idea.

Quoted 2 lines

It would take mach_a as an argument, add -incoming tcp:localhost:4444 to the command line of mach_a, use that to spawn mach_b, and

why add to mach_a? I thought -incoming is just for the destination.

Quoted 2 lines

return mach_b as the return value (or perhaps change mach_a to refer to the new machine).

I think it makes sense for the caller to create and pass the machines and then just call a library function to do the migrate. That way the caller 'owns' the machines. But maybe I'm missing something.

Quoted 54 lines

The reason is that I can anticipate having many migration qtests, at least one for every subsection we ever had to add.

Paolo

Quoted 44 lines

tests/migrate-test.c new file +32 -0

+{
+    int a_run = 0;
+    int b_run = 0;
+    int iter = 0;
+
+    /* is running on A ? */
+    migrate_assert(is_running(mach_a));
+
+    /* do migrate */
+    qtest_qmp(mach_a, "{ 'execute': 'migrate',"
+              "'arguments': { 'uri': 'tcp:0:4444' } }", NULL);
+
+    while (iter < SLEEP_MAX) {
+        a_run = is_running(mach_a);
+        b_run = is_running(mach_b);
+        if ((a_run == 0) && (b_run == 1)) {
+            break;
+        }
+        sleep(SLEEP_INTERVAL);
+        iter += SLEEP_INTERVAL;
+    }
+    migrate_assert((a_run == 0) && (b_run == 1));
+}
+
+int main(int argc, char **argv)
+{
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    mach_a = qtest_start("-display none -machine pc");
+    mach_b = qtest_start("-display none -machine pc -incoming

tcp:0:4444");

tests/migrate-test.c new file +7 -1

+
+    qtest_add_func("/migrate/a-to-b", migrate_a_to_b);
+    ret = g_test_run();
+
+    migrate_cleanup();
+    return ret;
+}
--

1.7.1

Paolo Bonzini

2012-12-14 17:26 UTC #13 lore raw

Re: [Qemu-devel] [PATCH 3/3] qtest: add migrate-test

Quoted 10 lines

Quoted 2 lines
Do you think this function could be turned into a libqtest call?
Seems like a good idea.
Quoted 3 lines
It would take mach_a as an argument, add -incoming tcp:localhost:4444
to the command line of mach_a, use that to spawn mach_b, and
why add to mach_a? I thought -incoming is just for the destination.

Yep. Tack it at the end of mach_a's command line (actually at the end of the parameter of qtest_start) and use the result to start mach_b. The command-lines for the two machines must match (apart from -incoming of course), it's not necessary to pass it twice.

Quoted 6 lines

Quoted 2 lines
return mach_b as the return value (or perhaps change mach_a to refer to the new machine).
I think it makes sense for the caller to create and pass the machines and then just call a library function to do the migrate. That way the caller 'owns' the machines. But maybe I'm missing something.

Yeah, owning the machines makes sense. In this case migration would just be a constructor for QTestState.

The alternative is to kill mach_a during migration and only proceed with mach_b, since mach_a is effectively not going to be used.

Paolo