[Qemu-devel] [PATCH v4 00/11] Add make check tests for Migration

[Qemu-devel] [PATCH v4 00/11] Add make check tests for Migration

[Juan Quintela]

Hi

In v4:

Based-on: 20180105205109.683-1-quintela@redhat.com

Changes:
- rebase on top on v4 info_migrate patches
- Tune sleeps to make patches fast
- Create a deprecated test for deprecated commands (i.e. make peterxu happy)
- create migrate_start_postcopy function
- fix naming/sizes between power and x86
- cleanup comments to match code

Please, review.


[v3]

Changes:

- No more tests for deprecated parameters. Now I only use
  migrate_set_parameter.  If there is a deprecated command for that,
  we tests it there.
- free "result" string, always good to return memory (Peter found it)
- use the new tcp_port parameter from info migrate.  So we are
  handling well the tcp case.
- lots of code movement around to make everything consistent.
- Several patches already integrated upstream.

Later, Juan.


[v2]
- to make review easier, I started renaming postcopy-test.c to migration-test.c
- Did cleanups/refactoring there
- Don't use global-qtest anymore
- check that the parameters that we sent got really set
- RFH: comrpress threads tests is not working for some weird reason.  Using the same code on command line works.
  still investigating why.

ToDoo:

- tcp: after discussions with dave, we ended in conclusion that we
  need to use the 0 port and let the system gives us a free one

  But .... that means that we need to be able to get that port back somehow.
  "info migrate" woring on destination side?

- compression threads.  There is some weird interaction with the test
  hardness and every migration thread get waiting in a different
  semaphore.  Investigating if it is a race/bug/whateverr

- deprecated commands: There was a suggestion to make
  migrate_set_parameter look at the parameter name and test old/new
  depending on something.  Not sure what to do here.

- testing commands: Is there a way to launch qemu and just sent
  qmp/hmp commands without having to really run anything else?

[v1]
- add test for precopy for unix/tcp
  exec and fd to came, don't know how to test rdma without hardware
- add tests using deprecated interfaces
- add test for xbzrle
  Note to myself, there is no way to set the cache size with migraton_set_parameters
- Add test for compress threads
  disabled on the series, right now it appears that compression is not working at all
- Move postcopy to use new results
  Idea is to move it on top of migration-test.c, but first I want some reviews on basic idea

Juan Quintela (11):
  tests: Remove deprecated migration tests commands
  tests: Migration ppc test was missing arguments
  tests: Consolidate accelerators declaration
  tests: Use consistent names and sizes for migration
  tests: Add deprecated commands migration test
  tests: Add migration precopy test
  tests: Add basic migration precopy tcp test
  tests: Add migration xbzrle test
  tests: Create migrate-start-postcopy command
  tests: Adjust sleeps for migration test
  [RFH] tests: Add migration compress threads tests

 tests/migration-test.c | 379 +++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 319 insertions(+), 60 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH v4 01/11] tests: Remove deprecated migration tests commands

[Juan Quintela]

We move to use migration_set_parameter() for everything.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/migration-test.c | 33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 9fd5dadc0d..0448bc77dc 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -381,37 +381,20 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
     QDECREF(rsp);
 }
 
-static void migrate_set_downtime(QTestState *who, const double value)
+static void migrate_set_parameter(QTestState *who, const char *parameter,
+                                  const char *value)
 {
     QDict *rsp;
     gchar *cmd;
-    char *expected;
-    int64_t result_int;
 
-    cmd = g_strdup_printf("{ 'execute': 'migrate_set_downtime',"
-                          "'arguments': { 'value': %g } }", value);
+    cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
+                          "'arguments': { '%s': %s } }",
+                          parameter, value);
     rsp = qtest_qmp(who, cmd);
     g_free(cmd);
     g_assert(qdict_haskey(rsp, "return"));
     QDECREF(rsp);
-    result_int = value * 1000L;
-    expected = g_strdup_printf("%" PRId64, result_int);
-    migrate_check_parameter(who, "downtime-limit", expected);
-    g_free(expected);
-}
-
-static void migrate_set_speed(QTestState *who, const char *value)
-{
-    QDict *rsp;
-    gchar *cmd;
-
-    cmd = g_strdup_printf("{ 'execute': 'migrate_set_speed',"
-                          "'arguments': { 'value': %s } }", value);
-    rsp = qtest_qmp(who, cmd);
-    g_free(cmd);
-    g_assert(qdict_haskey(rsp, "return"));
-    QDECREF(rsp);
-    migrate_check_parameter(who, "max-bandwidth", value);
+    migrate_check_parameter(who, parameter, value);
 }
 
 static void migrate_set_capability(QTestState *who, const char *capability,
@@ -543,8 +526,8 @@ static void test_migrate(void)
      * quickly, but that it doesn't complete precopy even on a slow
      * machine, so also set the downtime.
      */
-    migrate_set_speed(from, "100000000");
-    migrate_set_downtime(from, 0.001);
+    migrate_set_parameter(from, "max-bandwidth", "100000000");
+    migrate_set_parameter(from, "downtime-limit", "1");
 
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
-- 
2.14.3

[Qemu-devel] [PATCH v4 02/11] tests: Migration ppc test was missing arguments

[Juan Quintela]

Argument file is also needed there.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/migration-test.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 0448bc77dc..32f3bb86a8 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -464,8 +464,9 @@ static void test_migrate_start(QTestState **from, QTestState **to,
         cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
                                   " -name pcdest,debug-threads=on"
                                   " -serial file:%s/dest_serial"
+                                  " -drive file=%s,if=pflash,format=raw"
                                   " -incoming %s",
-                                  accel, tmpfs, uri);
+                                  accel, tmpfs, bootpath, uri);
     } else {
         g_assert_not_reached();
     }
-- 
2.14.3

[Qemu-devel] [PATCH v4 03/11] tests: Consolidate accelerators declaration

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/migration-test.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 32f3bb86a8..d81f22118b 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -434,27 +434,29 @@ static void test_migrate_start(QTestState **from, QTestState **to,
     gchar *cmd_src, *cmd_dst;
     char *bootpath = g_strdup_printf("%s/bootsect", tmpfs);
     const char *arch = qtest_get_arch();
+    const char *accel = "kvm:tcg";
 
     got_stop = false;
 
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         init_bootfile_x86(bootpath);
-        cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
+        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
                                   " -name pcsource,debug-threads=on"
                                   " -serial file:%s/src_serial"
                                   " -drive file=%s,format=raw",
-                                  tmpfs, bootpath);
-        cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
+                                  accel, tmpfs, bootpath);
+        cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
                                   " -name pcdest,debug-threads=on"
                                   " -serial file:%s/dest_serial"
                                   " -drive file=%s,format=raw"
                                   " -incoming %s",
-                                  tmpfs, bootpath, uri);
+                                  accel, tmpfs, bootpath, uri);
     } else if (strcmp(arch, "ppc64") == 0) {
-        const char *accel;
 
         /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */
-        accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg";
+        if (access("/sys/module/kvm_hv", F_OK)) {
+            accel = "tcg";
+        }
         init_bootfile_ppc(bootpath);
         cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
                                   " -name pcsource,debug-threads=on"
-- 
2.14.3

[Qemu-devel] [PATCH v4 04/11] tests: Use consistent names and sizes for migration

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/migration-test.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index d81f22118b..f469235d0b 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
 
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         init_bootfile_x86(bootpath);
-        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
-                                  " -name pcsource,debug-threads=on"
+        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
+                                  " -name source,debug-threads=on"
                                   " -serial file:%s/src_serial"
                                   " -drive file=%s,format=raw",
                                   accel, tmpfs, bootpath);
-        cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
-                                  " -name pcdest,debug-threads=on"
+        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
+                                  " -name target,debug-threads=on"
                                   " -serial file:%s/dest_serial"
                                   " -drive file=%s,format=raw"
                                   " -incoming %s",
@@ -459,12 +459,12 @@ static void test_migrate_start(QTestState **from, QTestState **to,
         }
         init_bootfile_ppc(bootpath);
         cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
-                                  " -name pcsource,debug-threads=on"
+                                  " -name source,debug-threads=on"
                                   " -serial file:%s/src_serial"
                                   " -drive file=%s,if=pflash,format=raw",
                                   accel, tmpfs, bootpath);
         cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
-                                  " -name pcdest,debug-threads=on"
+                                  " -name target,debug-threads=on"
                                   " -serial file:%s/dest_serial"
                                   " -drive file=%s,if=pflash,format=raw"
                                   " -incoming %s",
-- 
2.14.3

[Qemu-devel] [PATCH v4 05/11] tests: Add deprecated commands migration test

[Juan Quintela]

We add deprecated commands on a new test, so we don't have to add it
on normal tests.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/migration-test.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index f469235d0b..bcb0a82d42 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -513,6 +513,64 @@ static void test_migrate_end(QTestState *from, QTestState *to)
     cleanup("dest_serial");
 }
 
+static void deprecated_set_downtime(QTestState *who, const double value)
+{
+    QDict *rsp;
+    gchar *cmd;
+    char *expected;
+    int64_t result_int;
+
+    cmd = g_strdup_printf("{ 'execute': 'migrate_set_downtime',"
+                          "'arguments': { 'value': %g } }", value);
+    rsp = qtest_qmp(who, cmd);
+    g_free(cmd);
+    g_assert(qdict_haskey(rsp, "return"));
+    QDECREF(rsp);
+    result_int = value * 1000L;
+    expected = g_strdup_printf("%" PRId64, result_int);
+    migrate_check_parameter(who, "downtime-limit", expected);
+    g_free(expected);
+}
+
+static void deprecated_set_speed(QTestState *who, const char *value)
+{
+    QDict *rsp;
+    gchar *cmd;
+
+    cmd = g_strdup_printf("{ 'execute': 'migrate_set_speed',"
+                          "'arguments': { 'value': %s } }", value);
+    rsp = qtest_qmp(who, cmd);
+    g_free(cmd);
+    g_assert(qdict_haskey(rsp, "return"));
+    QDECREF(rsp);
+    migrate_check_parameter(who, "max-bandwidth", value);
+}
+
+static void test_deprecated(void)
+{
+    char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+    QTestState *from, *to;
+
+    /* Create source and destination guests.  This way we can reuse
+     * test_migrate_start */
+    test_migrate_start(&from, &to, uri);
+
+    deprecated_set_downtime(from, 0.12345);
+    deprecated_set_downtime(to, 0.12345);
+    deprecated_set_speed(from, "12345");
+    deprecated_set_speed(to, "12345");
+
+    g_free(uri);
+
+    qtest_quit(from);
+    qtest_quit(to);
+
+    cleanup("bootsect");
+    cleanup("migsocket");
+    cleanup("src_serial");
+    cleanup("dest_serial");
+}
+
 static void test_migrate(void)
 {
     char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
@@ -580,6 +638,7 @@ int main(int argc, char **argv)
     module_call_init(MODULE_INIT_QOM);
 
     qtest_add_func("/migration/postcopy/unix", test_migrate);
+    qtest_add_func("/migration/deprecated", test_deprecated);
 
     ret = g_test_run();
 
-- 
2.14.3

[Qemu-devel] [PATCH v4 06/11] tests: Add migration precopy test

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tests/migration-test.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index bcb0a82d42..88653ad767 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -571,7 +571,7 @@ static void test_deprecated(void)
     cleanup("dest_serial");
 }
 
-static void test_migrate(void)
+static void test_postcopy(void)
 {
     char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
     QTestState *from, *to;
@@ -618,6 +618,45 @@ static void test_migrate(void)
     test_migrate_end(from, to);
 }
 
+static void test_precopy_unix(void)
+{
+    char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+    QTestState *from, *to;
+
+    test_migrate_start(&from, &to, uri);
+
+    /* We want to pick a speed slow enough that the test completes
+     * quickly, but that it doesn't complete precopy even on a slow
+     * machine, so also set the downtime.
+     */
+    /* 1 ms should make it not converge*/
+    migrate_set_parameter(from, "downtime-limit", "1");
+    /* 1GB/s */
+    migrate_set_parameter(from, "max-bandwidth", "1000000000");
+
+    /* Wait for the first serial output from the source */
+    wait_for_serial("src_serial");
+
+    migrate(from, uri);
+
+    wait_for_migration_pass(from);
+
+    /* 300 ms should converge */
+    migrate_set_parameter(from, "downtime-limit", "300");
+
+    if (!got_stop) {
+        qtest_qmp_eventwait(from, "STOP");
+    }
+
+    qtest_qmp_eventwait(to, "RESUME");
+
+    wait_for_serial("dest_serial");
+    wait_for_migration_complete(from);
+
+    test_migrate_end(from, to);
+    g_free(uri);
+}
+
 int main(int argc, char **argv)
 {
     char template[] = "/tmp/migration-test-XXXXXX";
@@ -637,8 +676,9 @@ int main(int argc, char **argv)
 
     module_call_init(MODULE_INIT_QOM);
 
-    qtest_add_func("/migration/postcopy/unix", test_migrate);
+    qtest_add_func("/migration/precopy/unix", test_precopy_unix);
     qtest_add_func("/migration/deprecated", test_deprecated);
+    qtest_add_func("/migration/postcopy/unix", test_postcopy);
 
     ret = g_test_run();
 
-- 
2.14.3

[Qemu-devel] [PATCH v4 07/11] tests: Add basic migration precopy tcp test

[Juan Quintela]

Not sharing code from precopy/unix because we have to read back the
tcp parameter.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tests/migration-test.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 3 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 88653ad767..024b6675fb 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -366,8 +366,7 @@ static void cleanup(const char *filename)
     g_free(path);
 }
 
-static void migrate_check_parameter(QTestState *who, const char *parameter,
-                                    const char *value)
+static char *migrate_get_parameter(QTestState *who, const char *parameter)
 {
     QDict *rsp, *rsp_return;
     char *result;
@@ -376,9 +375,18 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
     rsp_return = qdict_get_qdict(rsp, "return");
     result = g_strdup_printf("%" PRId64,
                              qdict_get_try_int(rsp_return,  parameter, -1));
+    QDECREF(rsp);
+    return result;
+}
+
+static void migrate_check_parameter(QTestState *who, const char *parameter,
+                                    const char *value)
+{
+    char *result;
+
+    result = migrate_get_parameter(who, parameter);
     g_assert_cmpstr(result, ==, value);
     g_free(result);
-    QDECREF(rsp);
 }
 
 static void migrate_set_parameter(QTestState *who, const char *parameter,
@@ -657,6 +665,50 @@ static void test_precopy_unix(void)
     g_free(uri);
 }
 
+static void test_precopy_tcp(void)
+{
+    char *uri;
+    char *port;
+    QTestState *from, *to;
+
+    test_migrate_start(&from, &to, "tcp:127.0.0.1:0");
+
+    /* We want to pick a speed slow enough that the test completes
+     * quickly, but that it doesn't complete precopy even on a slow
+     * machine, so also set the downtime.
+     */
+    /* 1 ms should make it not converge*/
+    migrate_set_parameter(from, "downtime-limit", "1");
+    /* 1GB/s */
+    migrate_set_parameter(from, "max-bandwidth", "1000000000");
+
+    /* Wait for the first serial output from the source */
+    wait_for_serial("src_serial");
+
+    port = migrate_get_parameter(to, "tcp-port");
+    uri = g_strdup_printf("tcp:127.0.0.1:%s", port);
+
+    migrate(from, uri);
+
+    wait_for_migration_pass(from);
+
+    /* 300 ms should converge */
+    migrate_set_parameter(from, "downtime-limit", "300");
+
+    if (!got_stop) {
+        qtest_qmp_eventwait(from, "STOP");
+    }
+
+    qtest_qmp_eventwait(to, "RESUME");
+
+    wait_for_serial("dest_serial");
+    wait_for_migration_complete(from);
+
+    test_migrate_end(from, to);
+    g_free(uri);
+    g_free(port);
+}
+
 int main(int argc, char **argv)
 {
     char template[] = "/tmp/migration-test-XXXXXX";
@@ -677,6 +729,7 @@ int main(int argc, char **argv)
     module_call_init(MODULE_INIT_QOM);
 
     qtest_add_func("/migration/precopy/unix", test_precopy_unix);
+    qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
     qtest_add_func("/migration/deprecated", test_deprecated);
     qtest_add_func("/migration/postcopy/unix", test_postcopy);
 
-- 
2.14.3

[Qemu-devel] [PATCH v4 08/11] tests: Add migration xbzrle test

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/migration-test.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 024b6675fb..c25c9dd7b1 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -554,6 +554,20 @@ static void deprecated_set_speed(QTestState *who, const char *value)
     migrate_check_parameter(who, "max-bandwidth", value);
 }
 
+static void deprecated_set_cache_size(QTestState *who, const char *value)
+{
+    QDict *rsp;
+    gchar *cmd;
+
+    cmd = g_strdup_printf("{ 'execute': 'migrate-set-cache-size',"
+                          "'arguments': { 'value': %s } }", value);
+    rsp = qtest_qmp(who, cmd);
+    g_free(cmd);
+    g_assert(qdict_haskey(rsp, "return"));
+    QDECREF(rsp);
+    migrate_check_parameter(who, "xbzrle-cache-size", value);
+}
+
 static void test_deprecated(void)
 {
     char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
@@ -567,7 +581,8 @@ static void test_deprecated(void)
     deprecated_set_downtime(to, 0.12345);
     deprecated_set_speed(from, "12345");
     deprecated_set_speed(to, "12345");
-
+    deprecated_set_cache_size(from, "4096");
+    deprecated_set_cache_size(to, "4096");
     g_free(uri);
 
     qtest_quit(from);
@@ -709,6 +724,55 @@ static void test_precopy_tcp(void)
     g_free(port);
 }
 
+static void test_xbzrle(const char *uri)
+{
+    QTestState *from, *to;
+
+    test_migrate_start(&from, &to, uri);
+
+    /* We want to pick a speed slow enough that the test completes
+     * quickly, but that it doesn't complete precopy even on a slow
+     * machine, so also set the downtime.
+     */
+    /* 1 ms should make it not converge*/
+    migrate_set_parameter(from, "downtime-limit", "1");
+    /* 1GB/s */
+    migrate_set_parameter(from, "max-bandwidth", "1000000000");
+
+    migrate_set_parameter(from, "xbzrle-cache-size", "33554432");
+
+    migrate_set_capability(from, "xbzrle", "true");
+    migrate_set_capability(to, "xbzrle", "true");
+    /* Wait for the first serial output from the source */
+    wait_for_serial("src_serial");
+
+    migrate(from, uri);
+
+    wait_for_migration_pass(from);
+
+    /* 300ms should converge */
+    migrate_set_parameter(from, "downtime-limit", "300");
+
+    if (!got_stop) {
+        qtest_qmp_eventwait(from, "STOP");
+    }
+    qtest_qmp_eventwait(to, "RESUME");
+
+    wait_for_serial("dest_serial");
+    wait_for_migration_complete(from);
+
+    test_migrate_end(from, to);
+}
+
+static void test_xbzrle_unix(void)
+{
+    char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+
+    test_xbzrle(uri);
+    g_free(uri);
+}
+
+
 int main(int argc, char **argv)
 {
     char template[] = "/tmp/migration-test-XXXXXX";
@@ -732,6 +796,7 @@ int main(int argc, char **argv)
     qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
     qtest_add_func("/migration/deprecated", test_deprecated);
     qtest_add_func("/migration/postcopy/unix", test_postcopy);
+    qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
 
     ret = g_test_run();
 
-- 
2.14.3

[Qemu-devel] [PATCH v4 09/11] tests: Create migrate-start-postcopy command

[Juan Quintela]

This way, it is like the rest of commands

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/migration-test.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index c25c9dd7b1..70b3c0870b 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -436,6 +436,15 @@ static void migrate(QTestState *who, const char *uri)
     QDECREF(rsp);
 }
 
+static void migrate_start_postcopy(QTestState *who)
+{
+    QDict *rsp;
+
+    rsp = wait_command(who, "{ 'execute': 'migrate-start-postcopy' }");
+    g_assert(qdict_haskey(rsp, "return"));
+    QDECREF(rsp);
+}
+
 static void test_migrate_start(QTestState **from, QTestState **to,
                                const char *uri)
 {
@@ -598,7 +607,6 @@ static void test_postcopy(void)
 {
     char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
     QTestState *from, *to;
-    QDict *rsp;
 
     test_migrate_start(&from, &to, uri);
 
@@ -620,9 +628,7 @@ static void test_postcopy(void)
 
     wait_for_migration_pass(from);
 
-    rsp = wait_command(from, "{ 'execute': 'migrate-start-postcopy' }");
-    g_assert(qdict_haskey(rsp, "return"));
-    QDECREF(rsp);
+    migrate_start_postcopy(from);
 
     if (!got_stop) {
         qtest_qmp_eventwait(from, "STOP");
-- 
2.14.3

[Qemu-devel] [PATCH v4 10/11] tests: Adjust sleeps for migration test

[Juan Quintela]

Also reorder code to not sleep when event already happened.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/migration-test.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 70b3c0870b..8914829006 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -280,10 +280,9 @@ static void read_blocktime(QTestState *who)
 
 static void wait_for_migration_complete(QTestState *who)
 {
-    QDict *rsp, *rsp_return;
-    bool completed;
-
-    do {
+    while (true) {
+        QDict *rsp, *rsp_return;
+        bool completed;
         const char *status;
 
         rsp = wait_command(who, "{ 'execute': 'query-migrate' }");
@@ -292,8 +291,11 @@ static void wait_for_migration_complete(QTestState *who)
         completed = strcmp(status, "completed") == 0;
         g_assert_cmpstr(status, !=,  "failed");
         QDECREF(rsp);
-        usleep(1000 * 100);
-    } while (!completed);
+        if (completed) {
+            return;
+        }
+        usleep(1000);
+    }
 }
 
 static void wait_for_migration_pass(QTestState *who)
@@ -302,16 +304,13 @@ static void wait_for_migration_pass(QTestState *who)
     uint64_t pass;
 
     /* Wait for the 1st sync */
-    do {
+    while (!got_stop && !initial_pass) {
+        usleep(1000);
         initial_pass = get_migration_pass(who);
-        if (got_stop || initial_pass) {
-            break;
-        }
-        usleep(1000 * 100);
-    } while (true);
+    }
 
     do {
-        usleep(1000 * 100);
+        usleep(1000);
         pass = get_migration_pass(who);
     } while (pass == initial_pass && !got_stop);
 }
@@ -510,13 +509,13 @@ static void test_migrate_end(QTestState *from, QTestState *to)
     /* Destination still running, wait for a byte to change */
     do {
         qtest_memread(to, start_address, &dest_byte_b, 1);
-        usleep(10 * 1000);
+        usleep(1000 * 10);
     } while (dest_byte_a == dest_byte_b);
 
     qtest_qmp_discard_response(to, "{ 'execute' : 'stop'}");
     /* With it stopped, check nothing changes */
     qtest_memread(to, start_address, &dest_byte_c, 1);
-    sleep(1);
+    usleep(1000 * 200);
     qtest_memread(to, start_address, &dest_byte_d, 1);
     g_assert_cmpint(dest_byte_c, ==, dest_byte_d);
 
-- 
2.14.3

[Qemu-devel] [PATCH v4 11/11] [RFH] tests: Add migration compress threads tests

[Juan Quintela]

Yeap, it is still not working. trying to learn how to debug threads
for guests running from the testt hardness.

For some reason, compression is not working at the moment, test is
disabled until I found why.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/migration-test.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 8914829006..eb44a95aa9 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -777,6 +777,54 @@ static void test_xbzrle_unix(void)
     g_free(uri);
 }
 
+static void test_compress(const char *uri)
+{
+    QTestState *from, *to;
+
+    test_migrate_start(&from, &to, uri);
+
+    /* We want to pick a speed slow enough that the test completes
+     * quickly, but that it doesn't complete precopy even on a slow
+     * machine, so also set the downtime.
+     */
+    /* 1 ms should make it not converge*/
+    migrate_set_parameter(from, "downtime-limit", "1");
+    /* 1GB/s */
+    migrate_set_parameter(from, "max-bandwidth", "100000000");
+
+    migrate_set_parameter(from, "compress-threads", "4");
+    migrate_set_parameter(to, "decompress-threads", "3");
+
+    migrate_set_capability(from, "compress", "true");
+    migrate_set_capability(to, "compress", "true");
+    /* Wait for the first serial output from the source */
+    wait_for_serial("src_serial");
+
+    migrate(from, uri);
+
+    wait_for_migration_pass(from);
+
+    /* 300ms should converge */
+    migrate_set_parameter(from, "downtime-limit", "300");
+
+    if (!got_stop) {
+        qtest_qmp_eventwait(from, "STOP");
+    }
+    qtest_qmp_eventwait(to, "RESUME");
+
+    wait_for_serial("dest_serial");
+    wait_for_migration_complete(from);
+
+    test_migrate_end(from, to);
+}
+
+static void test_compress_unix(void)
+{
+    char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+
+    test_compress(uri);
+    g_free(uri);
+}
 
 int main(int argc, char **argv)
 {
@@ -802,6 +850,9 @@ int main(int argc, char **argv)
     qtest_add_func("/migration/deprecated", test_deprecated);
     qtest_add_func("/migration/postcopy/unix", test_postcopy);
     qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
+    if (0) {
+        qtest_add_func("/migration/compress/unix", test_compress_unix);
+    }
 
     ret = g_test_run();
 
-- 
2.14.3

Re: [Qemu-devel] [PATCH v4 02/11] tests: Migration ppc test was missing arguments

[Peter Xu]

On Fri, Jan 05, 2018 at 10:52:37PM +0100, Juan Quintela wrote:
> Argument file is also needed there.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

Does it also mean this?

Fixes: aaf89c8a49a8c ("test: port postcopy test to ppc64")

> ---
>  tests/migration-test.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 0448bc77dc..32f3bb86a8 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -464,8 +464,9 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>          cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
>                                    " -name pcdest,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
> +                                  " -drive file=%s,if=pflash,format=raw"
>                                    " -incoming %s",
> -                                  accel, tmpfs, uri);
> +                                  accel, tmpfs, bootpath, uri);
>      } else {
>          g_assert_not_reached();
>      }
> -- 
> 2.14.3
> 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 04/11] tests: Use consistent names and sizes for migration

[Peter Xu]

On Fri, Jan 05, 2018 at 10:52:39PM +0100, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/migration-test.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index d81f22118b..f469235d0b 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>  
>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>          init_bootfile_x86(bootpath);
> -        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
> -                                  " -name pcsource,debug-threads=on"
> +        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> +                                  " -name source,debug-threads=on"

A pure question: when will the name matter?

>                                    " -serial file:%s/src_serial"
>                                    " -drive file=%s,format=raw",
>                                    accel, tmpfs, bootpath);
> -        cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
> -                                  " -name pcdest,debug-threads=on"
> +        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
> +                                  " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -drive file=%s,format=raw"
>                                    " -incoming %s",
> @@ -459,12 +459,12 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>          }
>          init_bootfile_ppc(bootpath);
>          cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> -                                  " -name pcsource,debug-threads=on"
> +                                  " -name source,debug-threads=on"
>                                    " -serial file:%s/src_serial"
>                                    " -drive file=%s,if=pflash,format=raw",
>                                    accel, tmpfs, bootpath);
>          cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
> -                                  " -name pcdest,debug-threads=on"
> +                                  " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -drive file=%s,if=pflash,format=raw"
>                                    " -incoming %s",
> -- 
> 2.14.3
> 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 05/11] tests: Add deprecated commands migration test

[Peter Xu]

On Fri, Jan 05, 2018 at 10:52:40PM +0100, Juan Quintela wrote:
> We add deprecated commands on a new test, so we don't have to add it
> on normal tests.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/migration-test.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index f469235d0b..bcb0a82d42 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -513,6 +513,64 @@ static void test_migrate_end(QTestState *from, QTestState *to)
>      cleanup("dest_serial");
>  }
>  
> +static void deprecated_set_downtime(QTestState *who, const double value)
> +{
> +    QDict *rsp;
> +    gchar *cmd;
> +    char *expected;
> +    int64_t result_int;
> +
> +    cmd = g_strdup_printf("{ 'execute': 'migrate_set_downtime',"
> +                          "'arguments': { 'value': %g } }", value);
> +    rsp = qtest_qmp(who, cmd);
> +    g_free(cmd);
> +    g_assert(qdict_haskey(rsp, "return"));
> +    QDECREF(rsp);
> +    result_int = value * 1000L;
> +    expected = g_strdup_printf("%" PRId64, result_int);
> +    migrate_check_parameter(who, "downtime-limit", expected);
> +    g_free(expected);
> +}
> +
> +static void deprecated_set_speed(QTestState *who, const char *value)
> +{
> +    QDict *rsp;
> +    gchar *cmd;
> +
> +    cmd = g_strdup_printf("{ 'execute': 'migrate_set_speed',"
> +                          "'arguments': { 'value': %s } }", value);
> +    rsp = qtest_qmp(who, cmd);
> +    g_free(cmd);
> +    g_assert(qdict_haskey(rsp, "return"));
> +    QDECREF(rsp);
> +    migrate_check_parameter(who, "max-bandwidth", value);
> +}
> +
> +static void test_deprecated(void)
> +{
> +    char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> +    QTestState *from, *to;
> +
> +    /* Create source and destination guests.  This way we can reuse
> +     * test_migrate_start */
> +    test_migrate_start(&from, &to, uri);
> +
> +    deprecated_set_downtime(from, 0.12345);
> +    deprecated_set_downtime(to, 0.12345);
> +    deprecated_set_speed(from, "12345");
> +    deprecated_set_speed(to, "12345");
> +
> +    g_free(uri);
> +
> +    qtest_quit(from);
> +    qtest_quit(to);
> +
> +    cleanup("bootsect");
> +    cleanup("migsocket");
> +    cleanup("src_serial");
> +    cleanup("dest_serial");

I thought calling qtest_start() with a single VM would be even
simpler, but this is good enough for me... Thanks for that.  :-)

Reviewed-by: Peter Xu <peterx@redhat.com>

> +}
> +
>  static void test_migrate(void)
>  {
>      char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> @@ -580,6 +638,7 @@ int main(int argc, char **argv)
>      module_call_init(MODULE_INIT_QOM);
>  
>      qtest_add_func("/migration/postcopy/unix", test_migrate);
> +    qtest_add_func("/migration/deprecated", test_deprecated);
>  
>      ret = g_test_run();
>  
> -- 
> 2.14.3
> 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 00/11] Add make check tests for Migration

[Peter Xu]

On Fri, Jan 05, 2018 at 10:52:35PM +0100, Juan Quintela wrote:
> 
> Hi
> 
> In v4:
> 
> Based-on: 20180105205109.683-1-quintela@redhat.com
> 
> Changes:
> - rebase on top on v4 info_migrate patches
> - Tune sleeps to make patches fast
> - Create a deprecated test for deprecated commands (i.e. make peterxu happy)
> - create migrate_start_postcopy function
> - fix naming/sizes between power and x86
> - cleanup comments to match code
> 
> Please, review.

I left some comments on specific patches, for the rest I think the
series looks good to me.  Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 04/11] tests: Use consistent names and sizes for migration

[Juan Quintela]

Peter Xu <peterx@redhat.com> wrote:
> On Fri, Jan 05, 2018 at 10:52:39PM +0100, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  tests/migration-test.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>> 
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index d81f22118b..f469235d0b 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>>  
>>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>          init_bootfile_x86(bootpath);
>> -        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
>> -                                  " -name pcsource,debug-threads=on"
>> +        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
>> +                                  " -name source,debug-threads=on"
>
> A pure question: when will the name matter?

It don't matter, but ARM didn't want to use the pcname, and decided for
yet a different command line.  I would like to have the same command
line for all architectures.  At least the less gratuitous differences.

As you can guess, name don't matter at all., but telling ARNM people to
be consistent with things that are not consistent .... O:-)


Later, Juan.


>
>>                                    " -serial file:%s/src_serial"
>>                                    " -drive file=%s,format=raw",
>>                                    accel, tmpfs, bootpath);
>> -        cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
>> -                                  " -name pcdest,debug-threads=on"
>> +        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
>> +                                  " -name target,debug-threads=on"
>>                                    " -serial file:%s/dest_serial"
>>                                    " -drive file=%s,format=raw"
>>                                    " -incoming %s",
>> @@ -459,12 +459,12 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>>          }
>>          init_bootfile_ppc(bootpath);
>>          cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
>> -                                  " -name pcsource,debug-threads=on"
>> +                                  " -name source,debug-threads=on"
>>                                    " -serial file:%s/src_serial"
>>                                    " -drive file=%s,if=pflash,format=raw",
>>                                    accel, tmpfs, bootpath);
>>          cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
>> -                                  " -name pcdest,debug-threads=on"
>> +                                  " -name target,debug-threads=on"
>>                                    " -serial file:%s/dest_serial"
>>                                    " -drive file=%s,if=pflash,format=raw"
>>                                    " -incoming %s",
>> -- 
>> 2.14.3
>> 

Re: [Qemu-devel] [PATCH v4 05/11] tests: Add deprecated commands migration test

[Juan Quintela]

Peter Xu <peterx@redhat.com> wrote:
> On Fri, Jan 05, 2018 at 10:52:40PM +0100, Juan Quintela wrote:
>> We add deprecated commands on a new test, so we don't have to add it
>> on normal tests.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  tests/migration-test.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 59 insertions(+)
>> 
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index f469235d0b..bcb0a82d42 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -513,6 +513,64 @@ static void test_migrate_end(QTestState *from, QTestState *to)
>>      cleanup("dest_serial");
>>  }
>>  
>> +static void deprecated_set_downtime(QTestState *who, const double value)
>> +{
>> +    QDict *rsp;
>> +    gchar *cmd;
>> +    char *expected;
>> +    int64_t result_int;
>> +
>> +    cmd = g_strdup_printf("{ 'execute': 'migrate_set_downtime',"
>> +                          "'arguments': { 'value': %g } }", value);
>> +    rsp = qtest_qmp(who, cmd);
>> +    g_free(cmd);
>> +    g_assert(qdict_haskey(rsp, "return"));
>> +    QDECREF(rsp);
>> +    result_int = value * 1000L;
>> +    expected = g_strdup_printf("%" PRId64, result_int);
>> +    migrate_check_parameter(who, "downtime-limit", expected);
>> +    g_free(expected);
>> +}
>> +
>> +static void deprecated_set_speed(QTestState *who, const char *value)
>> +{
>> +    QDict *rsp;
>> +    gchar *cmd;
>> +
>> +    cmd = g_strdup_printf("{ 'execute': 'migrate_set_speed',"
>> +                          "'arguments': { 'value': %s } }", value);
>> +    rsp = qtest_qmp(who, cmd);
>> +    g_free(cmd);
>> +    g_assert(qdict_haskey(rsp, "return"));
>> +    QDECREF(rsp);
>> +    migrate_check_parameter(who, "max-bandwidth", value);
>> +}
>> +
>> +static void test_deprecated(void)
>> +{
>> +    char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
>> +    QTestState *from, *to;
>> +
>> +    /* Create source and destination guests.  This way we can reuse
>> +     * test_migrate_start */
>> +    test_migrate_start(&from, &to, uri);
>> +
>> +    deprecated_set_downtime(from, 0.12345);
>> +    deprecated_set_downtime(to, 0.12345);
>> +    deprecated_set_speed(from, "12345");
>> +    deprecated_set_speed(to, "12345");
>> +
>> +    g_free(uri);
>> +
>> +    qtest_quit(from);
>> +    qtest_quit(to);
>> +
>> +    cleanup("bootsect");
>> +    cleanup("migsocket");
>> +    cleanup("src_serial");
>> +    cleanup("dest_serial");
>
> I thought calling qtest_start() with a single VM would be even
> simpler, but this is good enough for me... Thanks for that.  :-)

I had two options:
- reuse the test_migrate_start function, and create two guests
- create a function that only creates one guest, but repeat the
  architecture dependent part.

Take your poison.

> Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks, Juan.

Re: [Qemu-devel] [PATCH v4 02/11] tests: Migration ppc test was missing arguments

[Juan Quintela]

Peter Xu <peterx@redhat.com> wrote:
> On Fri, Jan 05, 2018 at 10:52:37PM +0100, Juan Quintela wrote:
>> Argument file is also needed there.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
>
> Does it also mean this?
>
> Fixes: aaf89c8a49a8c ("test: port postcopy test to ppc64")

Dunno.  I was trying to consolidate the command line options for ppc and
x86 when I found this problem.  I haven't tested of ppc.

Thanks, Juan.

Re: [Qemu-devel] [PATCH v4 04/11] tests: Use consistent names and sizes for migration

[Peter Xu]

On Wed, Jan 10, 2018 at 09:43:48AM +0100, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > On Fri, Jan 05, 2018 at 10:52:39PM +0100, Juan Quintela wrote:
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >>  tests/migration-test.c | 12 ++++++------
> >>  1 file changed, 6 insertions(+), 6 deletions(-)
> >> 
> >> diff --git a/tests/migration-test.c b/tests/migration-test.c
> >> index d81f22118b..f469235d0b 100644
> >> --- a/tests/migration-test.c
> >> +++ b/tests/migration-test.c
> >> @@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
> >>  
> >>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> >>          init_bootfile_x86(bootpath);
> >> -        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
> >> -                                  " -name pcsource,debug-threads=on"
> >> +        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> >> +                                  " -name source,debug-threads=on"
> >
> > A pure question: when will the name matter?
> 
> It don't matter, but ARM didn't want to use the pcname, and decided for
> yet a different command line.  I would like to have the same command
> line for all architectures.  At least the less gratuitous differences.
> 
> As you can guess, name don't matter at all., but telling ARNM people to
> be consistent with things that are not consistent .... O:-)

Ah, fine. :)

This test only support x86 and ppc for now, right?

(Btw, AFAIK debug-threads=on is only useful if any of us wants to
 glance at thread names.  In other words, would it be even simpler to
 just remove that "-name" line? :-)

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 05/11] tests: Add deprecated commands migration test

[Peter Xu]

On Wed, Jan 10, 2018 at 09:46:49AM +0100, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > On Fri, Jan 05, 2018 at 10:52:40PM +0100, Juan Quintela wrote:
> >> We add deprecated commands on a new test, so we don't have to add it
> >> on normal tests.
> >> 
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >>  tests/migration-test.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 59 insertions(+)
> >> 
> >> diff --git a/tests/migration-test.c b/tests/migration-test.c
> >> index f469235d0b..bcb0a82d42 100644
> >> --- a/tests/migration-test.c
> >> +++ b/tests/migration-test.c
> >> @@ -513,6 +513,64 @@ static void test_migrate_end(QTestState *from, QTestState *to)
> >>      cleanup("dest_serial");
> >>  }
> >>  
> >> +static void deprecated_set_downtime(QTestState *who, const double value)
> >> +{
> >> +    QDict *rsp;
> >> +    gchar *cmd;
> >> +    char *expected;
> >> +    int64_t result_int;
> >> +
> >> +    cmd = g_strdup_printf("{ 'execute': 'migrate_set_downtime',"
> >> +                          "'arguments': { 'value': %g } }", value);
> >> +    rsp = qtest_qmp(who, cmd);
> >> +    g_free(cmd);
> >> +    g_assert(qdict_haskey(rsp, "return"));
> >> +    QDECREF(rsp);
> >> +    result_int = value * 1000L;
> >> +    expected = g_strdup_printf("%" PRId64, result_int);
> >> +    migrate_check_parameter(who, "downtime-limit", expected);
> >> +    g_free(expected);
> >> +}
> >> +
> >> +static void deprecated_set_speed(QTestState *who, const char *value)
> >> +{
> >> +    QDict *rsp;
> >> +    gchar *cmd;
> >> +
> >> +    cmd = g_strdup_printf("{ 'execute': 'migrate_set_speed',"
> >> +                          "'arguments': { 'value': %s } }", value);
> >> +    rsp = qtest_qmp(who, cmd);
> >> +    g_free(cmd);
> >> +    g_assert(qdict_haskey(rsp, "return"));
> >> +    QDECREF(rsp);
> >> +    migrate_check_parameter(who, "max-bandwidth", value);
> >> +}
> >> +
> >> +static void test_deprecated(void)
> >> +{
> >> +    char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> >> +    QTestState *from, *to;
> >> +
> >> +    /* Create source and destination guests.  This way we can reuse
> >> +     * test_migrate_start */
> >> +    test_migrate_start(&from, &to, uri);
> >> +
> >> +    deprecated_set_downtime(from, 0.12345);
> >> +    deprecated_set_downtime(to, 0.12345);
> >> +    deprecated_set_speed(from, "12345");
> >> +    deprecated_set_speed(to, "12345");
> >> +
> >> +    g_free(uri);
> >> +
> >> +    qtest_quit(from);
> >> +    qtest_quit(to);
> >> +
> >> +    cleanup("bootsect");
> >> +    cleanup("migsocket");
> >> +    cleanup("src_serial");
> >> +    cleanup("dest_serial");
> >
> > I thought calling qtest_start() with a single VM would be even
> > simpler, but this is good enough for me... Thanks for that.  :-)
> 
> I had two options:
> - reuse the test_migrate_start function, and create two guests
> - create a function that only creates one guest, but repeat the
>   architecture dependent part.
> 
> Take your poison.

Ah I see your point.  However, do we need those arch-dependent thing
for this QMP-only test?  Would a simplest qtest_start("") suffice?

(I mean, we just start a QEMU without extra QEMU parameters, as long
 as it has QMP)

> 
> > Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> Thanks, Juan.

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 00/11] Add make check tests for Migration

[Peter Xu]

On Wed, Jan 10, 2018 at 02:42:52PM +0800, Peter Xu wrote:
> On Fri, Jan 05, 2018 at 10:52:35PM +0100, Juan Quintela wrote:
> > 
> > Hi
> > 
> > In v4:
> > 
> > Based-on: 20180105205109.683-1-quintela@redhat.com
> > 
> > Changes:
> > - rebase on top on v4 info_migrate patches
> > - Tune sleeps to make patches fast
> > - Create a deprecated test for deprecated commands (i.e. make peterxu happy)
> > - create migrate_start_postcopy function
> > - fix naming/sizes between power and x86
> > - cleanup comments to match code
> > 
> > Please, review.
> 
> I left some comments on specific patches, for the rest I think the
> series looks good to me.  Thanks,

I believe Dave would be a better person to ack, but as a potential
user of the series (who wants to use it soon), I'd like to provide my
r-b before Dave provides (possibly) more comments, so:

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 02/11] tests: Migration ppc test was missing arguments

[Laurent Vivier]

On 10/01/2018 09:47, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
>> On Fri, Jan 05, 2018 at 10:52:37PM +0100, Juan Quintela wrote:
>>> Argument file is also needed there.
>>>
>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>
>> Reviewed-by: Peter Xu <peterx@redhat.com>
>>
>> Does it also mean this?
>>
>> Fixes: aaf89c8a49a8c ("test: port postcopy test to ppc64")
> 
> Dunno.  I was trying to consolidate the command line options for ppc and
> x86 when I found this problem.  I haven't tested of ppc.

I don't think it is needed. I think the content of the nvram is migrated
(otherwise the test wouldn't work at all).

The nvram is created by default, we need the command line parameter only
to populate it from a file.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v4 02/11] tests: Migration ppc test was missing arguments

[Laurent Vivier]

On 10/01/2018 10:21, Laurent Vivier wrote:
> On 10/01/2018 09:47, Juan Quintela wrote:
>> Peter Xu <peterx@redhat.com> wrote:
>>> On Fri, Jan 05, 2018 at 10:52:37PM +0100, Juan Quintela wrote:
>>>> Argument file is also needed there.
>>>>
>>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>>
>>> Reviewed-by: Peter Xu <peterx@redhat.com>
>>>
>>> Does it also mean this?
>>>
>>> Fixes: aaf89c8a49a8c ("test: port postcopy test to ppc64")
>>
>> Dunno.  I was trying to consolidate the command line options for ppc and
>> x86 when I found this problem.  I haven't tested of ppc.
> 
> I don't think it is needed. I think the content of the nvram is migrated
> (otherwise the test wouldn't work at all).
> 
> The nvram is created by default, we need the command line parameter only
> to populate it from a file.

A better change would be to use "-prom-env" instead of "-driver
if=pflash". I can send the patch if you want to add it in your series.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v4 02/11] tests: Migration ppc test was missing arguments

[Juan Quintela]

Laurent Vivier <lvivier@redhat.com> wrote:
> On 10/01/2018 10:21, Laurent Vivier wrote:
>> On 10/01/2018 09:47, Juan Quintela wrote:
>>> Peter Xu <peterx@redhat.com> wrote:
>>>> On Fri, Jan 05, 2018 at 10:52:37PM +0100, Juan Quintela wrote:
>>>>> Argument file is also needed there.
>>>>>
>>>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>>>
>>>> Reviewed-by: Peter Xu <peterx@redhat.com>
>>>>
>>>> Does it also mean this?
>>>>
>>>> Fixes: aaf89c8a49a8c ("test: port postcopy test to ppc64")
>>>
>>> Dunno.  I was trying to consolidate the command line options for ppc and
>>> x86 when I found this problem.  I haven't tested of ppc.
>> 
>> I don't think it is needed. I think the content of the nvram is migrated
>> (otherwise the test wouldn't work at all).
>> 
>> The nvram is created by default, we need the command line parameter only
>> to populate it from a file.
>
> A better change would be to use "-prom-env" instead of "-driver
> if=pflash". I can send the patch if you want to add it in your series.

Told the command line and I will add to the line.

And using the same command in both sides makes easier to see that it is
correct.

Later, Juan.

Re: [Qemu-devel] [PATCH v4 02/11] tests: Migration ppc test was missing arguments

[Laurent Vivier]

On 10/01/2018 11:43, Juan Quintela wrote:
> Laurent Vivier <lvivier@redhat.com> wrote:
>> On 10/01/2018 10:21, Laurent Vivier wrote:
>>> On 10/01/2018 09:47, Juan Quintela wrote:
>>>> Peter Xu <peterx@redhat.com> wrote:
>>>>> On Fri, Jan 05, 2018 at 10:52:37PM +0100, Juan Quintela wrote:
>>>>>> Argument file is also needed there.
>>>>>>
>>>>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>>>>
>>>>> Reviewed-by: Peter Xu <peterx@redhat.com>
>>>>>
>>>>> Does it also mean this?
>>>>>
>>>>> Fixes: aaf89c8a49a8c ("test: port postcopy test to ppc64")
>>>>
>>>> Dunno.  I was trying to consolidate the command line options for ppc and
>>>> x86 when I found this problem.  I haven't tested of ppc.
>>>
>>> I don't think it is needed. I think the content of the nvram is migrated
>>> (otherwise the test wouldn't work at all).
>>>
>>> The nvram is created by default, we need the command line parameter only
>>> to populate it from a file.
>>
>> A better change would be to use "-prom-env" instead of "-driver
>> if=pflash". I can send the patch if you want to add it in your series.
> 
> Told the command line and I will add to the line.
> 
> And using the same command in both sides makes easier to see that it is
> correct.
> 
> Later, Juan.
> 

("-machine accel=%s -m 256M"
 " -name pcsource,debug-threads=on"
 " -serial file:%s/src_serial"
 " -prom-env '"
 "boot-command=hex .\" _\" begin %x %x "
 "do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
 "until'",  accel, tmpfs, end_address, start_address);

Don't forget to remove include of "hw/nvram/chrp_nvram.h",
MIN_NVRAM_SIZE and init_bootfile_ppc().

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v4 04/11] tests: Use consistent names and sizes for migration

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/migration-test.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index d81f22118b..f469235d0b 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>  
>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>          init_bootfile_x86(bootpath);
> -        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
> -                                  " -name pcsource,debug-threads=on"
> +        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> +                                  " -name source,debug-threads=on"
>                                    " -serial file:%s/src_serial"
>                                    " -drive file=%s,format=raw",
>                                    accel, tmpfs, bootpath);
> -        cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
> -                                  " -name pcdest,debug-threads=on"
> +        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
> +                                  " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -drive file=%s,format=raw"
>                                    " -incoming %s",
> @@ -459,12 +459,12 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>          }
>          init_bootfile_ppc(bootpath);
>          cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> -                                  " -name pcsource,debug-threads=on"
> +                                  " -name source,debug-threads=on"
>                                    " -serial file:%s/src_serial"
>                                    " -drive file=%s,if=pflash,format=raw",
>                                    accel, tmpfs, bootpath);
>          cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
> -                                  " -name pcdest,debug-threads=on"
> +                                  " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -drive file=%s,if=pflash,format=raw"
>                                    " -incoming %s",

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

(I'm OK with increasing the RAM size but we don't need to, some may find
it a bit wasteful in tests, but it's only 106M extra)

> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 04/11] tests: Use consistent names and sizes for migration

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> On Wed, Jan 10, 2018 at 09:43:48AM +0100, Juan Quintela wrote:
> > Peter Xu <peterx@redhat.com> wrote:
> > > On Fri, Jan 05, 2018 at 10:52:39PM +0100, Juan Quintela wrote:
> > >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> > >> ---
> > >>  tests/migration-test.c | 12 ++++++------
> > >>  1 file changed, 6 insertions(+), 6 deletions(-)
> > >> 
> > >> diff --git a/tests/migration-test.c b/tests/migration-test.c
> > >> index d81f22118b..f469235d0b 100644
> > >> --- a/tests/migration-test.c
> > >> +++ b/tests/migration-test.c
> > >> @@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
> > >>  
> > >>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> > >>          init_bootfile_x86(bootpath);
> > >> -        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
> > >> -                                  " -name pcsource,debug-threads=on"
> > >> +        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> > >> +                                  " -name source,debug-threads=on"
> > >
> > > A pure question: when will the name matter?
> > 
> > It don't matter, but ARM didn't want to use the pcname, and decided for
> > yet a different command line.  I would like to have the same command
> > line for all architectures.  At least the less gratuitous differences.
> > 
> > As you can guess, name don't matter at all., but telling ARNM people to
> > be consistent with things that are not consistent .... O:-)
> 
> Ah, fine. :)
> 
> This test only support x86 and ppc for now, right?
> 
> (Btw, AFAIK debug-threads=on is only useful if any of us wants to
>  glance at thread names.  In other words, would it be even simpler to
>  just remove that "-name" line? :-)

It makes debugging hangs/crashes easier sometimes, so keep it.

Dave

> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 04/11] tests: Use consistent names and sizes for migration

[Juan Quintela]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Peter Xu (peterx@redhat.com) wrote:
>> On Wed, Jan 10, 2018 at 09:43:48AM +0100, Juan Quintela wrote:
>> > Peter Xu <peterx@redhat.com> wrote:
>> > > On Fri, Jan 05, 2018 at 10:52:39PM +0100, Juan Quintela wrote:
>> > >> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> > >> ---
>> > >>  tests/migration-test.c | 12 ++++++------
>> > >>  1 file changed, 6 insertions(+), 6 deletions(-)
>> > >> 
>> > >> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> > >> index d81f22118b..f469235d0b 100644
>> > >> --- a/tests/migration-test.c
>> > >> +++ b/tests/migration-test.c
>> > >> @@ -440,13 +440,13 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>> > >>  
>> > >>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> > >>          init_bootfile_x86(bootpath);
>> > >> -        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
>> > >> -                                  " -name pcsource,debug-threads=on"
>> > >> +        cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
>> > >> +                                  " -name source,debug-threads=on"
>> > >
>> > > A pure question: when will the name matter?
>> > 
>> > It don't matter, but ARM didn't want to use the pcname, and decided for
>> > yet a different command line.  I would like to have the same command
>> > line for all architectures.  At least the less gratuitous differences.
>> > 
>> > As you can guess, name don't matter at all., but telling ARNM people to
>> > be consistent with things that are not consistent .... O:-)
>> 
>> Ah, fine. :)
>> 
>> This test only support x86 and ppc for now, right?
>> 
>> (Btw, AFAIK debug-threads=on is only useful if any of us wants to
>>  glance at thread names.  In other words, would it be even simpler to
>>  just remove that "-name" line? :-)
>
> It makes debugging hangs/crashes easier sometimes, so keep it.

Agreed.  I think that this should be default on qemu O:-)

Later, Juan.

Re: [Qemu-devel] [PATCH v4 09/11] tests: Create migrate-start-postcopy command

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> This way, it is like the rest of commands
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

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

> ---
>  tests/migration-test.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index c25c9dd7b1..70b3c0870b 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -436,6 +436,15 @@ static void migrate(QTestState *who, const char *uri)
>      QDECREF(rsp);
>  }
>  
> +static void migrate_start_postcopy(QTestState *who)
> +{
> +    QDict *rsp;
> +
> +    rsp = wait_command(who, "{ 'execute': 'migrate-start-postcopy' }");
> +    g_assert(qdict_haskey(rsp, "return"));
> +    QDECREF(rsp);
> +}
> +
>  static void test_migrate_start(QTestState **from, QTestState **to,
>                                 const char *uri)
>  {
> @@ -598,7 +607,6 @@ static void test_postcopy(void)
>  {
>      char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
>      QTestState *from, *to;
> -    QDict *rsp;
>  
>      test_migrate_start(&from, &to, uri);
>  
> @@ -620,9 +628,7 @@ static void test_postcopy(void)
>  
>      wait_for_migration_pass(from);
>  
> -    rsp = wait_command(from, "{ 'execute': 'migrate-start-postcopy' }");
> -    g_assert(qdict_haskey(rsp, "return"));
> -    QDECREF(rsp);
> +    migrate_start_postcopy(from);
>  
>      if (!got_stop) {
>          qtest_qmp_eventwait(from, "STOP");
> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 10/11] tests: Adjust sleeps for migration test

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Also reorder code to not sleep when event already happened.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/migration-test.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 70b3c0870b..8914829006 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -280,10 +280,9 @@ static void read_blocktime(QTestState *who)
>  
>  static void wait_for_migration_complete(QTestState *who)
>  {
> -    QDict *rsp, *rsp_return;
> -    bool completed;
> -
> -    do {
> +    while (true) {
> +        QDict *rsp, *rsp_return;
> +        bool completed;
>          const char *status;
>  
>          rsp = wait_command(who, "{ 'execute': 'query-migrate' }");
> @@ -292,8 +291,11 @@ static void wait_for_migration_complete(QTestState *who)
>          completed = strcmp(status, "completed") == 0;
>          g_assert_cmpstr(status, !=,  "failed");
>          QDECREF(rsp);
> -        usleep(1000 * 100);
> -    } while (!completed);
> +        if (completed) {
> +            return;
> +        }
> +        usleep(1000);
> +    }

OK, so that's mostly about avoiding a sleep in the exit from the normal
path.

>  }
>  
>  static void wait_for_migration_pass(QTestState *who)
> @@ -302,16 +304,13 @@ static void wait_for_migration_pass(QTestState *who)
>      uint64_t pass;
>  
>      /* Wait for the 1st sync */
> -    do {
> +    while (!got_stop && !initial_pass) {
> +        usleep(1000);
>          initial_pass = get_migration_pass(who);
> -        if (got_stop || initial_pass) {
> -            break;
> -        }
> -        usleep(1000 * 100);
> -    } while (true);
> +    }
>  
>      do {
> -        usleep(1000 * 100);
> +        usleep(1000);
>          pass = get_migration_pass(who);
>      } while (pass == initial_pass && !got_stop);
>  }

OK

> @@ -510,13 +509,13 @@ static void test_migrate_end(QTestState *from, QTestState *to)
>      /* Destination still running, wait for a byte to change */
>      do {
>          qtest_memread(to, start_address, &dest_byte_b, 1);
> -        usleep(10 * 1000);
> +        usleep(1000 * 10);

Profound :-)

>      } while (dest_byte_a == dest_byte_b);
>  
>      qtest_qmp_discard_response(to, "{ 'execute' : 'stop'}");
>      /* With it stopped, check nothing changes */
>      qtest_memread(to, start_address, &dest_byte_c, 1);
> -    sleep(1);
> +    usleep(1000 * 200);

OK

>      qtest_memread(to, start_address, &dest_byte_d, 1);
>      g_assert_cmpint(dest_byte_c, ==, dest_byte_d);



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

> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 01/11] tests: Remove deprecated migration tests commands

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> We move to use migration_set_parameter() for everything.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

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

> ---
>  tests/migration-test.c | 33 ++++++++-------------------------
>  1 file changed, 8 insertions(+), 25 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 9fd5dadc0d..0448bc77dc 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -381,37 +381,20 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
>      QDECREF(rsp);
>  }
>  
> -static void migrate_set_downtime(QTestState *who, const double value)
> +static void migrate_set_parameter(QTestState *who, const char *parameter,
> +                                  const char *value)
>  {
>      QDict *rsp;
>      gchar *cmd;
> -    char *expected;
> -    int64_t result_int;
>  
> -    cmd = g_strdup_printf("{ 'execute': 'migrate_set_downtime',"
> -                          "'arguments': { 'value': %g } }", value);
> +    cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
> +                          "'arguments': { '%s': %s } }",
> +                          parameter, value);
>      rsp = qtest_qmp(who, cmd);
>      g_free(cmd);
>      g_assert(qdict_haskey(rsp, "return"));
>      QDECREF(rsp);
> -    result_int = value * 1000L;
> -    expected = g_strdup_printf("%" PRId64, result_int);
> -    migrate_check_parameter(who, "downtime-limit", expected);
> -    g_free(expected);
> -}
> -
> -static void migrate_set_speed(QTestState *who, const char *value)
> -{
> -    QDict *rsp;
> -    gchar *cmd;
> -
> -    cmd = g_strdup_printf("{ 'execute': 'migrate_set_speed',"
> -                          "'arguments': { 'value': %s } }", value);
> -    rsp = qtest_qmp(who, cmd);
> -    g_free(cmd);
> -    g_assert(qdict_haskey(rsp, "return"));
> -    QDECREF(rsp);
> -    migrate_check_parameter(who, "max-bandwidth", value);
> +    migrate_check_parameter(who, parameter, value);
>  }
>  
>  static void migrate_set_capability(QTestState *who, const char *capability,
> @@ -543,8 +526,8 @@ static void test_migrate(void)
>       * quickly, but that it doesn't complete precopy even on a slow
>       * machine, so also set the downtime.
>       */
> -    migrate_set_speed(from, "100000000");
> -    migrate_set_downtime(from, 0.001);
> +    migrate_set_parameter(from, "max-bandwidth", "100000000");
> +    migrate_set_parameter(from, "downtime-limit", "1");
>  
>      /* Wait for the first serial output from the source */
>      wait_for_serial("src_serial");
> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 03/11] tests: Consolidate accelerators declaration

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>

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

> ---
>  tests/migration-test.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 32f3bb86a8..d81f22118b 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -434,27 +434,29 @@ static void test_migrate_start(QTestState **from, QTestState **to,
>      gchar *cmd_src, *cmd_dst;
>      char *bootpath = g_strdup_printf("%s/bootsect", tmpfs);
>      const char *arch = qtest_get_arch();
> +    const char *accel = "kvm:tcg";
>  
>      got_stop = false;
>  
>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>          init_bootfile_x86(bootpath);
> -        cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> +        cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
>                                    " -name pcsource,debug-threads=on"
>                                    " -serial file:%s/src_serial"
>                                    " -drive file=%s,format=raw",
> -                                  tmpfs, bootpath);
> -        cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 150M"
> +                                  accel, tmpfs, bootpath);
> +        cmd_dst = g_strdup_printf("-machine accel=%s -m 150M"
>                                    " -name pcdest,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -drive file=%s,format=raw"
>                                    " -incoming %s",
> -                                  tmpfs, bootpath, uri);
> +                                  accel, tmpfs, bootpath, uri);
>      } else if (strcmp(arch, "ppc64") == 0) {
> -        const char *accel;
>  
>          /* On ppc64, the test only works with kvm-hv, but not with kvm-pr */
> -        accel = access("/sys/module/kvm_hv", F_OK) ? "tcg" : "kvm:tcg";
> +        if (access("/sys/module/kvm_hv", F_OK)) {
> +            accel = "tcg";
> +        }
>          init_bootfile_ppc(bootpath);
>          cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
>                                    " -name pcsource,debug-threads=on"
> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK