[Qemu-devel] [PATCH v3 0/6] Add make check tests for Migration

[Qemu-devel] [PATCH v3 0/6] Add make check tests for Migration

[Juan Quintela]

Hi

This is on top of my info_migrate series.

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?

Please review,.

Later, Juan.

[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

Please, review.

Juan.

Juan Quintela (6):
  migration: free result string
  tests: migration test deprecated commands
  tests: Add migration precopy test
  tests: Add basic migration precopy tcp test
  tests: Add migration xbzrle test
  [RFH] tests: Add migration compress threads tests

 tests/migration-test.c | 257 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 247 insertions(+), 10 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH v3 1/6] migration: free result string

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reported-by: Peter Xu <peterx@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 be598d3257..799e24ebc6 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -358,13 +358,14 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
                                     const char *value)
 {
     QDict *rsp, *rsp_return;
-    const char *result;
+    char *result;
 
     rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }");
     rsp_return = qdict_get_qdict(rsp, "return");
     result = g_strdup_printf("%" PRId64,
                              qdict_get_try_int(rsp_return,  parameter, -1));
     g_assert_cmpstr(result, ==, value);
+    g_free(result);
     QDECREF(rsp);
 }
 
-- 
2.14.3

[Qemu-devel] [PATCH v3 2/6] tests: migration test deprecated commands

[Juan Quintela]

We now test the deprecated commands everytime that we test the new
commands.  This makes unnecesary to add tests for deprecated commands.

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

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 799e24ebc6..51f49c74e9 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
     QDECREF(rsp);
 }
 
-static void migrate_set_downtime(QTestState *who, const double value)
+static void deprecated_set_downtime(QTestState *who, const double value)
 {
     QDict *rsp;
     gchar *cmd;
@@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
     g_free(expected);
 }
 
-static void migrate_set_speed(QTestState *who, const char *value)
+static void deprecated_set_speed(QTestState *who, const char *value)
 {
     QDict *rsp;
     gchar *cmd;
@@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
     migrate_check_parameter(who, "max-bandwidth", value);
 }
 
+static void migrate_set_parameter(QTestState *who, const char *parameter,
+                                  const char *value)
+{
+    QDict *rsp;
+    gchar *cmd;
+
+    if (strcmp(parameter, "downtime-limit") == 0) {
+        deprecated_set_downtime(who, 0.12345);
+    }
+
+    if (strcmp(parameter, "max-bandwidth") == 0) {
+        deprecated_set_speed(who, "12345");
+    }
+
+    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);
+    migrate_check_parameter(who, parameter, value);
+}
+
 static void migrate_set_capability(QTestState *who, const char *capability,
                                    const char *value)
 {
@@ -530,8 +554,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 v3 3/6] tests: Add migration precopy test

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@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 51f49c74e9..3f3f056be6 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -539,7 +539,7 @@ static void test_migrate_end(QTestState *from, QTestState *to)
     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;
@@ -582,6 +582,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 */
+    migrate_set_parameter(from, "downtime-limit", "1");
+    /* 1GB/s slow*/
+    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";
@@ -601,7 +640,8 @@ 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/postcopy/unix", test_postcopy);
 
     ret = g_test_run();
 
-- 
2.14.3

[Qemu-devel] [PATCH v3 4/6] 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>
---
 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 3f3f056be6..841c89e28a 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -354,8 +354,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;
@@ -364,9 +363,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 deprecated_set_downtime(QTestState *who, const double value)
@@ -621,6 +629,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 */
+    migrate_set_parameter(from, "downtime-limit", "1");
+    /* 1GB/s slow*/
+    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";
@@ -641,6 +693,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/postcopy/unix", test_postcopy);
 
     ret = g_test_run();
-- 
2.14.3

[Qemu-devel] [PATCH v3 5/6] tests: Add migration xbzrle test

[Juan Quintela]

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

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 841c89e28a..41dee78a9a 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -410,6 +410,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 migrate_set_parameter(QTestState *who, const char *parameter,
                                   const char *value)
 {
@@ -424,6 +438,10 @@ static void migrate_set_parameter(QTestState *who, const char *parameter,
         deprecated_set_speed(who, "12345");
     }
 
+    if (strcmp(parameter, "xbzrle-cache-size") == 0) {
+        deprecated_set_cache_size(who, "4096");
+    }
+
     cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
                           "'arguments': { '%s': %s } }",
                           parameter, value);
@@ -673,6 +691,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.
+     */
+    /* 100 ms */
+    migrate_set_parameter(from, "downtime-limit", "1");
+    /* 1MB/s slow*/
+    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 it 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";
@@ -695,6 +762,7 @@ int main(int argc, char **argv)
     qtest_add_func("/migration/precopy/unix", test_precopy_unix);
     qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
     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 v3 6/6] [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 41dee78a9a..eab3b146a4 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -739,6 +739,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.
+     */
+    /* 100 ms */
+    migrate_set_parameter(from, "downtime-limit", "1");
+    /* 1MB/s slow*/
+    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 it 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)
 {
@@ -763,6 +811,9 @@ int main(int argc, char **argv)
     qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
     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 v3 0/6] Add make check tests for Migration

[Eric Blake]

On 12/01/2017 06:58 AM, Juan Quintela wrote:
> Hi
> 
> This is on top of my info_migrate series.

Let patchew know about it:

Based-on: <20171201125750.1372-1-quintela@redhat.com>
([PATCH v3 0/2] Improve info migrate output on destination)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 0/6] Add make check tests for Migration

[Laurent Vivier]

On 01/12/2017 19:30, Eric Blake wrote:
> On 12/01/2017 06:58 AM, Juan Quintela wrote:
>> Hi
>>
>> This is on top of my info_migrate series.
> 
> Let patchew know about it:
> 
> Based-on: <20171201125750.1372-1-quintela@redhat.com>
> ([PATCH v3 0/2] Improve info migrate output on destination)

Is this tag documented somewhere?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 0/6] Add make check tests for Migration

[Eric Blake]

On 12/01/2017 12:38 PM, Laurent Vivier wrote:
> On 01/12/2017 19:30, Eric Blake wrote:
>> On 12/01/2017 06:58 AM, Juan Quintela wrote:
>>> Hi
>>>
>>> This is on top of my info_migrate series.
>>
>> Let patchew know about it:
>>
>> Based-on: <20171201125750.1372-1-quintela@redhat.com>
>> ([PATCH v3 0/2] Improve info migrate output on destination)
> 
> Is this tag documented somewhere?
> 

https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01288.html

But yes, it should also be documented in the wiki and/or other places 
where we document patch submission guidelines.

Volunteers?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v3 2/6] tests: migration test deprecated commands

[Peter Xu]

On Fri, Dec 01, 2017 at 01:58:09PM +0100, Juan Quintela wrote:
> We now test the deprecated commands everytime that we test the new
> commands.  This makes unnecesary to add tests for deprecated commands.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 799e24ebc6..51f49c74e9 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
>      QDECREF(rsp);
>  }
>  
> -static void migrate_set_downtime(QTestState *who, const double value)
> +static void deprecated_set_downtime(QTestState *who, const double value)
>  {
>      QDict *rsp;
>      gchar *cmd;
> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
>      g_free(expected);
>  }
>  
> -static void migrate_set_speed(QTestState *who, const char *value)
> +static void deprecated_set_speed(QTestState *who, const char *value)
>  {
>      QDict *rsp;
>      gchar *cmd;
> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
>      migrate_check_parameter(who, "max-bandwidth", value);
>  }
>  
> +static void migrate_set_parameter(QTestState *who, const char *parameter,
> +                                  const char *value)
> +{
> +    QDict *rsp;
> +    gchar *cmd;
> +
> +    if (strcmp(parameter, "downtime-limit") == 0) {
> +        deprecated_set_downtime(who, 0.12345);
> +    }
> +
> +    if (strcmp(parameter, "max-bandwidth") == 0) {
> +        deprecated_set_speed(who, "12345");
> +    }

I'm fine with current approach, but I would really prefer to put them
all into a standalone test, by just calling them one by one with some
specific numbers and that's all.

(luckily we only have two deprecated commands and limited tests,
 otherwise extra commands will be M*N, say "number of deprecated
 command" * "number of test mirations")

Thanks,

> +
> +    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);
> +    migrate_check_parameter(who, parameter, value);
> +}
> +
>  static void migrate_set_capability(QTestState *who, const char *capability,
>                                     const char *value)
>  {
> @@ -530,8 +554,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
> 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v3 1/6] migration: free result string

[Dr. David Alan Gilbert]

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

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

(same as Marc-André's)

> ---
>  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 be598d3257..799e24ebc6 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -358,13 +358,14 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
>                                      const char *value)
>  {
>      QDict *rsp, *rsp_return;
> -    const char *result;
> +    char *result;
>  
>      rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }");
>      rsp_return = qdict_get_qdict(rsp, "return");
>      result = g_strdup_printf("%" PRId64,
>                               qdict_get_try_int(rsp_return,  parameter, -1));
>      g_assert_cmpstr(result, ==, value);
> +    g_free(result);
>      QDECREF(rsp);
>  }
>  
> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 2/6] tests: migration test deprecated commands

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> We now test the deprecated commands everytime that we test the new
> commands.  This makes unnecesary to add tests for deprecated commands.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 799e24ebc6..51f49c74e9 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
>      QDECREF(rsp);
>  }
>  
> -static void migrate_set_downtime(QTestState *who, const double value)
> +static void deprecated_set_downtime(QTestState *who, const double value)
>  {
>      QDict *rsp;
>      gchar *cmd;
> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
>      g_free(expected);
>  }
>  
> -static void migrate_set_speed(QTestState *who, const char *value)
> +static void deprecated_set_speed(QTestState *who, const char *value)
>  {
>      QDict *rsp;
>      gchar *cmd;
> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
>      migrate_check_parameter(who, "max-bandwidth", value);
>  }
>  
> +static void migrate_set_parameter(QTestState *who, const char *parameter,
> +                                  const char *value)
> +{
> +    QDict *rsp;
> +    gchar *cmd;
> +
> +    if (strcmp(parameter, "downtime-limit") == 0) {
> +        deprecated_set_downtime(who, 0.12345);
> +    }
> +
> +    if (strcmp(parameter, "max-bandwidth") == 0) {
> +        deprecated_set_speed(who, "12345");
> +    }

I find that odd; you call migrate_set_parameter to set a particular
value, but we set them to a different arbitrary value first?

Dave

> +    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);
> +    migrate_check_parameter(who, parameter, value);
> +}
> +
>  static void migrate_set_capability(QTestState *who, const char *capability,
>                                     const char *value)
>  {
> @@ -530,8 +554,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 v3 3/6] tests: Add migration precopy test

[Dr. David Alan Gilbert]

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

OK, although you seem to have chosen a bandwidth 10x what I use
in the postcopy test but for the same reason.


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 51f49c74e9..3f3f056be6 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -539,7 +539,7 @@ static void test_migrate_end(QTestState *from, QTestState *to)
>      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;
> @@ -582,6 +582,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 */
> +    migrate_set_parameter(from, "downtime-limit", "1");
> +    /* 1GB/s slow*/
> +    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";
> @@ -601,7 +640,8 @@ 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/postcopy/unix", test_postcopy);
>  
>      ret = g_test_run();
>  
> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 4/6] tests: Add basic migration precopy tcp test

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> 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 3f3f056be6..841c89e28a 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -354,8 +354,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;
> @@ -364,9 +363,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 deprecated_set_downtime(QTestState *who, const double value)
> @@ -621,6 +629,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 */
> +    migrate_set_parameter(from, "downtime-limit", "1");
> +    /* 1GB/s slow*/
> +    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";
> @@ -641,6 +693,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/postcopy/unix", test_postcopy);
>  
>      ret = g_test_run();
> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 5/6] tests: Add migration xbzrle test

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/migration-test.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 841c89e28a..41dee78a9a 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -410,6 +410,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 migrate_set_parameter(QTestState *who, const char *parameter,
>                                    const char *value)
>  {
> @@ -424,6 +438,10 @@ static void migrate_set_parameter(QTestState *who, const char *parameter,
>          deprecated_set_speed(who, "12345");
>      }
>  
> +    if (strcmp(parameter, "xbzrle-cache-size") == 0) {
> +        deprecated_set_cache_size(who, "4096");
> +    }
> +
>      cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
>                            "'arguments': { '%s': %s } }",
>                            parameter, value);
> @@ -673,6 +691,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.
> +     */
> +    /* 100 ms */
> +    migrate_set_parameter(from, "downtime-limit", "1");

? That's 1 ms not 100ms as the comment says?

> +    /* 1MB/s slow*/
> +    migrate_set_parameter(from, "max-bandwidth", "1000000000");

That's still 1GB/s isn't it?
Don't you need to reduce that if xbzrle is actually working otherwise
it'll complete?

> +    migrate_set_parameter(from, "xbzrle-cache-size", "33554432");

That's 32M - why?
The test continually rewrites 100M of data; only changing one byte/page
- so actually the fun way to do the xbzrle test is to leave this as 32M
here.....

> +    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 it should converge */
> +    migrate_set_parameter(from, "downtime-limit", "300");

but first increase the xvzrle cache to 120M and give it a couple of
seconds delay; you might even find it completes without the downtime
limit schange; but you should still do that after a few seconds.
You can check the stats as well, you should get some xbzrle cache hits
as long as you raised the cache size.

Dave

> +    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";
> @@ -695,6 +762,7 @@ int main(int argc, char **argv)
>      qtest_add_func("/migration/precopy/unix", test_precopy_unix);
>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
>      qtest_add_func("/migration/postcopy/unix", test_postcopy);
> +    qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
>  
>      ret = g_test_run();
>  
> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 6/6] [RFH] tests: Add migration compress threads tests

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> 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.

How does it fail?

Dave

> 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 41dee78a9a..eab3b146a4 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -739,6 +739,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.
> +     */
> +    /* 100 ms */
> +    migrate_set_parameter(from, "downtime-limit", "1");
> +    /* 1MB/s slow*/
> +    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 it 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)
>  {
> @@ -763,6 +811,9 @@ int main(int argc, char **argv)
>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
>      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
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 2/6] tests: migration test deprecated commands

[Juan Quintela]

Peter Xu <peterx@redhat.com> wrote:
> On Fri, Dec 01, 2017 at 01:58:09PM +0100, Juan Quintela wrote:
>> We now test the deprecated commands everytime that we test the new
>> commands.  This makes unnecesary to add tests for deprecated commands.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
>>  1 file changed, 28 insertions(+), 4 deletions(-)
>> 
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index 799e24ebc6..51f49c74e9 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
>>      QDECREF(rsp);
>>  }
>>  
>> -static void migrate_set_downtime(QTestState *who, const double value)
>> +static void deprecated_set_downtime(QTestState *who, const double value)
>>  {
>>      QDict *rsp;
>>      gchar *cmd;
>> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
>>      g_free(expected);
>>  }
>>  
>> -static void migrate_set_speed(QTestState *who, const char *value)
>> +static void deprecated_set_speed(QTestState *who, const char *value)
>>  {
>>      QDict *rsp;
>>      gchar *cmd;
>> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
>>      migrate_check_parameter(who, "max-bandwidth", value);
>>  }
>>  
>> +static void migrate_set_parameter(QTestState *who, const char *parameter,
>> +                                  const char *value)
>> +{
>> +    QDict *rsp;
>> +    gchar *cmd;
>> +
>> +    if (strcmp(parameter, "downtime-limit") == 0) {
>> +        deprecated_set_downtime(who, 0.12345);
>> +    }
>> +
>> +    if (strcmp(parameter, "max-bandwidth") == 0) {
>> +        deprecated_set_speed(who, "12345");
>> +    }
>
> I'm fine with current approach, but I would really prefer to put them
> all into a standalone test, by just calling them one by one with some
> specific numbers and that's all.

That means another test (at least), and we have, also at least three
deprecated comands:
- migrate_set_speed
- migrate_set_downtime
- migrate_set_cache_size

And each test makes things slower.  So I *thought* it would we wiser to
just check _always_ use the deprecated an the not deprecated one.

> (luckily we only have two deprecated commands and limited tests,
>  otherwise extra commands will be M*N, say "number of deprecated
>  command" * "number of test mirations")

Each test takes time, so adding tests make everything much slower.
Notice that setting a new setting is fast.

This was the way that I understood Dave he wanted.

Later, Juan.


>
> Thanks,
>
>> +
>> +    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);
>> +    migrate_check_parameter(who, parameter, value);
>> +}
>> +
>>  static void migrate_set_capability(QTestState *who, const char *capability,
>>                                     const char *value)
>>  {
>> @@ -530,8 +554,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
>> 

Re: [Qemu-devel] [PATCH v3 2/6] tests: migration test deprecated commands

[Juan Quintela]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> We now test the deprecated commands everytime that we test the new
>> commands.  This makes unnecesary to add tests for deprecated commands.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
>>  1 file changed, 28 insertions(+), 4 deletions(-)
>> 
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index 799e24ebc6..51f49c74e9 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
>>      QDECREF(rsp);
>>  }
>>  
>> -static void migrate_set_downtime(QTestState *who, const double value)
>> +static void deprecated_set_downtime(QTestState *who, const double value)
>>  {
>>      QDict *rsp;
>>      gchar *cmd;
>> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
>>      g_free(expected);
>>  }
>>  
>> -static void migrate_set_speed(QTestState *who, const char *value)
>> +static void deprecated_set_speed(QTestState *who, const char *value)
>>  {
>>      QDict *rsp;
>>      gchar *cmd;
>> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
>>      migrate_check_parameter(who, "max-bandwidth", value);
>>  }
>>  
>> +static void migrate_set_parameter(QTestState *who, const char *parameter,
>> +                                  const char *value)
>> +{
>> +    QDict *rsp;
>> +    gchar *cmd;
>> +
>> +    if (strcmp(parameter, "downtime-limit") == 0) {
>> +        deprecated_set_downtime(who, 0.12345);
>> +    }
>> +
>> +    if (strcmp(parameter, "max-bandwidth") == 0) {
>> +        deprecated_set_speed(who, "12345");
>> +    }
>
> I find that odd; you call migrate_set_parameter to set a particular
> value, but we set them to a different arbitrary value first?

If I do:

migrate_deprecated_command(real_value)
migrate_non_deprecated_command(real_value)
  here the value is already set, so I am not sure that it is working.

This other way, if there is a _deprecated_ command,  I set it to a
known value, check that it was set correctly, and then set the real
value.

Later, Juan.

>
> Dave
>
>> +    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);
>> +    migrate_check_parameter(who, parameter, value);
>> +}
>> +
>>  static void migrate_set_capability(QTestState *who, const char *capability,
>>                                     const char *value)
>>  {
>> @@ -530,8 +554,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 v3 3/6] tests: Add migration precopy test

[Juan Quintela]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> OK, although you seem to have chosen a bandwidth 10x what I use
> in the postcopy test but for the same reason.

I tried to make it as fast as possible while making sure that we wait
enough time.   In my home machine each test takes around 2-3 seconds,
and I want to add tests, so better that they are fast.

Later, Juan.

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

Thanks, Juan.


>
>> ---
>>  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 51f49c74e9..3f3f056be6 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -539,7 +539,7 @@ static void test_migrate_end(QTestState *from, QTestState *to)
>>      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;
>> @@ -582,6 +582,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 */
>> +    migrate_set_parameter(from, "downtime-limit", "1");
>> +    /* 1GB/s slow*/
>> +    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";
>> @@ -601,7 +640,8 @@ 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/postcopy/unix", test_postcopy);
>>  
>>      ret = g_test_run();
>>  
>> -- 
>> 2.14.3
>> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 5/6] tests: Add migration xbzrle test

[Juan Quintela]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  tests/migration-test.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 68 insertions(+)
>> 
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index 841c89e28a..41dee78a9a 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -410,6 +410,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 migrate_set_parameter(QTestState *who, const char *parameter,
>>                                    const char *value)
>>  {
>> @@ -424,6 +438,10 @@ static void migrate_set_parameter(QTestState *who, const char *parameter,
>>          deprecated_set_speed(who, "12345");
>>      }
>>  
>> +    if (strcmp(parameter, "xbzrle-cache-size") == 0) {
>> +        deprecated_set_cache_size(who, "4096");
>> +    }
>> +
>>      cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
>>                            "'arguments': { '%s': %s } }",
>>                            parameter, value);
>> @@ -673,6 +691,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.
>> +     */
>> +    /* 100 ms */
>> +    migrate_set_parameter(from, "downtime-limit", "1");
>
> ? That's 1 ms not 100ms as the comment says?

You are right, fixed all the comments and forget this one.
Fixed.

>> +    /* 1MB/s slow*/
>> +    migrate_set_parameter(from, "max-bandwidth", "1000000000");
>
> That's still 1GB/s isn't it?
> Don't you need to reduce that if xbzrle is actually working otherwise
> it'll complete?

I was doing it differently before:

300ms downtime
1MB/s

But this was not reliable to make me wait for it.

So, I changed it to be:

1GB/s (always full speed)
1ms downtime  (so it never converge)

and I change the downtime to something higher later.


>
>> +    migrate_set_parameter(from, "xbzrle-cache-size", "33554432");
>
> That's 32M - why?

It is a value smaller than the full amount of memory.  I want to test
the xbzrle cache size command.  I don't care about *which* value we are
using, really.

> The test continually rewrites 100M of data; only changing one byte/page
> - so actually the fun way to do the xbzrle test is to leave this as 32M
> here.....

>> +    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 it should converge */
>> +    migrate_set_parameter(from, "downtime-limit", "300");
>
> but first increase the xvzrle cache to 120M and give it a couple of
> seconds delay; you might even find it completes without the downtime
> limit schange; but you should still do that after a few seconds.
> You can check the stats as well, you should get some xbzrle cache hits
> as long as you raised the cache size.

ok, will think something more about this.

Thanks, Juan.


> Dave
>
>> +    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";
>> @@ -695,6 +762,7 @@ int main(int argc, char **argv)
>>      qtest_add_func("/migration/precopy/unix", test_precopy_unix);
>>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
>>      qtest_add_func("/migration/postcopy/unix", test_postcopy);
>> +    qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
>>  
>>      ret = g_test_run();
>>  
>> -- 
>> 2.14.3
>> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 6/6] [RFH] tests: Add migration compress threads tests

[Juan Quintela]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> 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.
>
> How does it fail?

Source and destination hang.  Running exactly the same commands without
the test harnness work as expected.

attaching gdb show that every thread is waiting, but haven't found
anything obvious.  Happens in both sides, so I am not sure really which
one is not working (or if both are broken).

Later, Juan.


>
> Dave
>
>> 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 41dee78a9a..eab3b146a4 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -739,6 +739,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.
>> +     */
>> +    /* 100 ms */
>> +    migrate_set_parameter(from, "downtime-limit", "1");
>> +    /* 1MB/s slow*/
>> +    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 it 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)
>>  {
>> @@ -763,6 +811,9 @@ int main(int argc, char **argv)
>>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
>>      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
>> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 2/6] tests: migration test deprecated commands

[Peter Xu]

On Tue, Dec 26, 2017 at 08:51:10PM +0100, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > On Fri, Dec 01, 2017 at 01:58:09PM +0100, Juan Quintela wrote:
> >> We now test the deprecated commands everytime that we test the new
> >> commands.  This makes unnecesary to add tests for deprecated commands.
> >> 
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
> >>  1 file changed, 28 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/tests/migration-test.c b/tests/migration-test.c
> >> index 799e24ebc6..51f49c74e9 100644
> >> --- a/tests/migration-test.c
> >> +++ b/tests/migration-test.c
> >> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
> >>      QDECREF(rsp);
> >>  }
> >>  
> >> -static void migrate_set_downtime(QTestState *who, const double value)
> >> +static void deprecated_set_downtime(QTestState *who, const double value)
> >>  {
> >>      QDict *rsp;
> >>      gchar *cmd;
> >> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
> >>      g_free(expected);
> >>  }
> >>  
> >> -static void migrate_set_speed(QTestState *who, const char *value)
> >> +static void deprecated_set_speed(QTestState *who, const char *value)
> >>  {
> >>      QDict *rsp;
> >>      gchar *cmd;
> >> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
> >>      migrate_check_parameter(who, "max-bandwidth", value);
> >>  }
> >>  
> >> +static void migrate_set_parameter(QTestState *who, const char *parameter,
> >> +                                  const char *value)
> >> +{
> >> +    QDict *rsp;
> >> +    gchar *cmd;
> >> +
> >> +    if (strcmp(parameter, "downtime-limit") == 0) {
> >> +        deprecated_set_downtime(who, 0.12345);
> >> +    }
> >> +
> >> +    if (strcmp(parameter, "max-bandwidth") == 0) {
> >> +        deprecated_set_speed(who, "12345");
> >> +    }
> >
> > I'm fine with current approach, but I would really prefer to put them
> > all into a standalone test, by just calling them one by one with some
> > specific numbers and that's all.
> 
> That means another test (at least), and we have, also at least three
> deprecated comands:
> - migrate_set_speed
> - migrate_set_downtime
> - migrate_set_cache_size
> 
> And each test makes things slower.  So I *thought* it would we wiser to
> just check _always_ use the deprecated an the not deprecated one.
> 
> > (luckily we only have two deprecated commands and limited tests,
> >  otherwise extra commands will be M*N, say "number of deprecated
> >  command" * "number of test mirations")
> 
> Each test takes time, so adding tests make everything much slower.
> Notice that setting a new setting is fast.
> 
> This was the way that I understood Dave he wanted.

Do you mean every test is slow, or just migration tests?  Here I mean
to only test setting the parameters without doing real migration tests
(then query to make sure the settings were taking effect).  I assume
that should be fast too?  Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v3 2/6] tests: migration test deprecated commands

[Juan Quintela]

Peter Xu <peterx@redhat.com> wrote:
> On Tue, Dec 26, 2017 at 08:51:10PM +0100, Juan Quintela wrote:
>> Peter Xu <peterx@redhat.com> wrote:
>> > On Fri, Dec 01, 2017 at 01:58:09PM +0100, Juan Quintela wrote:
>> >> We now test the deprecated commands everytime that we test the new
>> >> commands.  This makes unnecesary to add tests for deprecated commands.
>> >> 
>> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> >> ---
>> >>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
>> >>  1 file changed, 28 insertions(+), 4 deletions(-)
>> >> 
>> >> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> >> index 799e24ebc6..51f49c74e9 100644
>> >> --- a/tests/migration-test.c
>> >> +++ b/tests/migration-test.c
>> >> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
>> >>      QDECREF(rsp);
>> >>  }
>> >>  
>> >> -static void migrate_set_downtime(QTestState *who, const double value)
>> >> +static void deprecated_set_downtime(QTestState *who, const double value)
>> >>  {
>> >>      QDict *rsp;
>> >>      gchar *cmd;
>> >> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
>> >>      g_free(expected);
>> >>  }
>> >>  
>> >> -static void migrate_set_speed(QTestState *who, const char *value)
>> >> +static void deprecated_set_speed(QTestState *who, const char *value)
>> >>  {
>> >>      QDict *rsp;
>> >>      gchar *cmd;
>> >> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
>> >>      migrate_check_parameter(who, "max-bandwidth", value);
>> >>  }
>> >>  
>> >> +static void migrate_set_parameter(QTestState *who, const char *parameter,
>> >> +                                  const char *value)
>> >> +{
>> >> +    QDict *rsp;
>> >> +    gchar *cmd;
>> >> +
>> >> +    if (strcmp(parameter, "downtime-limit") == 0) {
>> >> +        deprecated_set_downtime(who, 0.12345);
>> >> +    }
>> >> +
>> >> +    if (strcmp(parameter, "max-bandwidth") == 0) {
>> >> +        deprecated_set_speed(who, "12345");
>> >> +    }
>> >
>> > I'm fine with current approach, but I would really prefer to put them
>> > all into a standalone test, by just calling them one by one with some
>> > specific numbers and that's all.
>> 
>> That means another test (at least), and we have, also at least three
>> deprecated comands:
>> - migrate_set_speed
>> - migrate_set_downtime
>> - migrate_set_cache_size
>> 
>> And each test makes things slower.  So I *thought* it would we wiser to
>> just check _always_ use the deprecated an the not deprecated one.
>> 
>> > (luckily we only have two deprecated commands and limited tests,
>> >  otherwise extra commands will be M*N, say "number of deprecated
>> >  command" * "number of test mirations")
>> 
>> Each test takes time, so adding tests make everything much slower.
>> Notice that setting a new setting is fast.
>> 
>> This was the way that I understood Dave he wanted.
>
> Do you mean every test is slow, or just migration tests?

Each migration test adds around 2 seconds on my machine.  So I decided
that it was easier that each time that we check one command, we test the
deprecated and non-deprecated versions of the command.  Amount of time
added to the test is negigible, and we are sure that we always test both
functions.

If we ever remove the deprecated method, we can always remove only that
part of th etest.


> Here I mean
> to only test setting the parameters without doing real migration tests
> (then query to make sure the settings were taking effect).  I assume
> that should be fast too?  Thanks,

We could create a new test for that, but we need to start in on
source/destination, I thought it just made things more complicated.

If your preffer that way, please suggest how/what?

Thanks, Juan.

Re: [Qemu-devel] [PATCH v3 2/6] tests: migration test deprecated commands

[Peter Xu]

On Wed, Dec 27, 2017 at 10:41:02AM +0100, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > On Tue, Dec 26, 2017 at 08:51:10PM +0100, Juan Quintela wrote:
> >> Peter Xu <peterx@redhat.com> wrote:
> >> > On Fri, Dec 01, 2017 at 01:58:09PM +0100, Juan Quintela wrote:
> >> >> We now test the deprecated commands everytime that we test the new
> >> >> commands.  This makes unnecesary to add tests for deprecated commands.
> >> >> 
> >> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> >> ---
> >> >>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
> >> >>  1 file changed, 28 insertions(+), 4 deletions(-)
> >> >> 
> >> >> diff --git a/tests/migration-test.c b/tests/migration-test.c
> >> >> index 799e24ebc6..51f49c74e9 100644
> >> >> --- a/tests/migration-test.c
> >> >> +++ b/tests/migration-test.c
> >> >> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
> >> >>      QDECREF(rsp);
> >> >>  }
> >> >>  
> >> >> -static void migrate_set_downtime(QTestState *who, const double value)
> >> >> +static void deprecated_set_downtime(QTestState *who, const double value)
> >> >>  {
> >> >>      QDict *rsp;
> >> >>      gchar *cmd;
> >> >> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
> >> >>      g_free(expected);
> >> >>  }
> >> >>  
> >> >> -static void migrate_set_speed(QTestState *who, const char *value)
> >> >> +static void deprecated_set_speed(QTestState *who, const char *value)
> >> >>  {
> >> >>      QDict *rsp;
> >> >>      gchar *cmd;
> >> >> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
> >> >>      migrate_check_parameter(who, "max-bandwidth", value);
> >> >>  }
> >> >>  
> >> >> +static void migrate_set_parameter(QTestState *who, const char *parameter,
> >> >> +                                  const char *value)
> >> >> +{
> >> >> +    QDict *rsp;
> >> >> +    gchar *cmd;
> >> >> +
> >> >> +    if (strcmp(parameter, "downtime-limit") == 0) {
> >> >> +        deprecated_set_downtime(who, 0.12345);
> >> >> +    }
> >> >> +
> >> >> +    if (strcmp(parameter, "max-bandwidth") == 0) {
> >> >> +        deprecated_set_speed(who, "12345");
> >> >> +    }
> >> >
> >> > I'm fine with current approach, but I would really prefer to put them
> >> > all into a standalone test, by just calling them one by one with some
> >> > specific numbers and that's all.
> >> 
> >> That means another test (at least), and we have, also at least three
> >> deprecated comands:
> >> - migrate_set_speed
> >> - migrate_set_downtime
> >> - migrate_set_cache_size
> >> 
> >> And each test makes things slower.  So I *thought* it would we wiser to
> >> just check _always_ use the deprecated an the not deprecated one.
> >> 
> >> > (luckily we only have two deprecated commands and limited tests,
> >> >  otherwise extra commands will be M*N, say "number of deprecated
> >> >  command" * "number of test mirations")
> >> 
> >> Each test takes time, so adding tests make everything much slower.
> >> Notice that setting a new setting is fast.
> >> 
> >> This was the way that I understood Dave he wanted.
> >
> > Do you mean every test is slow, or just migration tests?
> 
> Each migration test adds around 2 seconds on my machine.  So I decided
> that it was easier that each time that we check one command, we test the
> deprecated and non-deprecated versions of the command.  Amount of time
> added to the test is negigible, and we are sure that we always test both
> functions.
> 
> If we ever remove the deprecated method, we can always remove only that
> part of th etest.
> 
> 
> > Here I mean
> > to only test setting the parameters without doing real migration tests
> > (then query to make sure the settings were taking effect).  I assume
> > that should be fast too?  Thanks,
> 
> We could create a new test for that, but we need to start in on
> source/destination, I thought it just made things more complicated.
> 
> If your preffer that way, please suggest how/what?

I mean, an isolated test that only runs a single VM instance rather
than two (so no test_migrate_start() at all), but only: qtest_init(),
then qmp() to send the deprecated commands, then query to see whether
the values are set.  That's all.  Can that work for us?

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v3 0/6] Add make check tests for Migration

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 975 bytes --]

On 12/01/2017 12:44 PM, Eric Blake wrote:
> On 12/01/2017 12:38 PM, Laurent Vivier wrote:
>> On 01/12/2017 19:30, Eric Blake wrote:
>>> On 12/01/2017 06:58 AM, Juan Quintela wrote:
>>>> Hi
>>>>
>>>> This is on top of my info_migrate series.
>>>
>>> Let patchew know about it:
>>>
>>> Based-on: <20171201125750.1372-1-quintela@redhat.com>
>>> ([PATCH v3 0/2] Improve info migrate output on destination)
>>
>> Is this tag documented somewhere?
>>
> 
> https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01288.html
> 
> But yes, it should also be documented in the wiki and/or other places
> where we document patch submission guidelines.
> 
> Volunteers?

I've updated https://wiki.qemu.org/Contribute/SubmitAPatch to mention
patchew and the Based-on tag line, but there may still be other places
worth updating.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v3 5/6] tests: Add migration xbzrle test

[Juan Quintela]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  tests/migration-test.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 68 insertions(+)
>> 
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index 841c89e28a..41dee78a9a 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -410,6 +410,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 migrate_set_parameter(QTestState *who, const char *parameter,
>>                                    const char *value)
>>  {
>> @@ -424,6 +438,10 @@ static void migrate_set_parameter(QTestState *who, const char *parameter,
>>          deprecated_set_speed(who, "12345");
>>      }
>>  
>> +    if (strcmp(parameter, "xbzrle-cache-size") == 0) {
>> +        deprecated_set_cache_size(who, "4096");
>> +    }
>> +
>>      cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
>>                            "'arguments': { '%s': %s } }",
>>                            parameter, value);
>> @@ -673,6 +691,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.
>> +     */
>> +    /* 100 ms */
>> +    migrate_set_parameter(from, "downtime-limit", "1");
>
> ? That's 1 ms not 100ms as the comment says?

Changed comment.

>
>> +    /* 1MB/s slow*/
>> +    migrate_set_parameter(from, "max-bandwidth", "1000000000");
>
> That's still 1GB/s isn't it?

Changed also.

> Don't you need to reduce that if xbzrle is actually working otherwise
> it'll complete?

It don't complete on my setup (dunno with a faster machine).

>> +    migrate_set_parameter(from, "xbzrle-cache-size", "33554432");
>
> That's 32M - why?

I need to check that the command is running, no clue about _which_ one
is better here.

> The test continually rewrites 100M of data; only changing one byte/page
> - so actually the fun way to do the xbzrle test is to leave this as 32M
> here.....

My idea here is that we somke test all the transports/commands.  Not
that I fully test everything.

See on my next series, It already takes around 15 seconds, just adding
stuff will make things go longer really fast.

BTW, is there a trivial way to convince qtest/getestr whatevre to run
all the migration tests in parallel?

Later, Juan.



>> +    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 it should converge */
>> +    migrate_set_parameter(from, "downtime-limit", "300");
>
> but first increase the xvzrle cache to 120M and give it a couple of
> seconds delay; you might even find it completes without the downtime
> limit schange; but you should still do that after a few seconds.
> You can check the stats as well, you should get some xbzrle cache hits
> as long as you raised the cache size.
>
> Dave
>
>> +    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";
>> @@ -695,6 +762,7 @@ int main(int argc, char **argv)
>>      qtest_add_func("/migration/precopy/unix", test_precopy_unix);
>>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
>>      qtest_add_func("/migration/postcopy/unix", test_postcopy);
>> +    qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
>>  
>>      ret = g_test_run();
>>  
>> -- 
>> 2.14.3
>> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 5/6] tests: Add migration xbzrle test

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > * Juan Quintela (quintela@redhat.com) wrote:
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >>  tests/migration-test.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 68 insertions(+)
> >> 
> >> diff --git a/tests/migration-test.c b/tests/migration-test.c
> >> index 841c89e28a..41dee78a9a 100644
> >> --- a/tests/migration-test.c
> >> +++ b/tests/migration-test.c
> >> @@ -410,6 +410,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 migrate_set_parameter(QTestState *who, const char *parameter,
> >>                                    const char *value)
> >>  {
> >> @@ -424,6 +438,10 @@ static void migrate_set_parameter(QTestState *who, const char *parameter,
> >>          deprecated_set_speed(who, "12345");
> >>      }
> >>  
> >> +    if (strcmp(parameter, "xbzrle-cache-size") == 0) {
> >> +        deprecated_set_cache_size(who, "4096");
> >> +    }
> >> +
> >>      cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
> >>                            "'arguments': { '%s': %s } }",
> >>                            parameter, value);
> >> @@ -673,6 +691,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.
> >> +     */
> >> +    /* 100 ms */
> >> +    migrate_set_parameter(from, "downtime-limit", "1");
> >
> > ? That's 1 ms not 100ms as the comment says?
> 
> Changed comment.
> 
> >
> >> +    /* 1MB/s slow*/
> >> +    migrate_set_parameter(from, "max-bandwidth", "1000000000");
> >
> > That's still 1GB/s isn't it?
> 
> Changed also.
> 
> > Don't you need to reduce that if xbzrle is actually working otherwise
> > it'll complete?
> 
> It don't complete on my setup (dunno with a faster machine).
> 
> >> +    migrate_set_parameter(from, "xbzrle-cache-size", "33554432");
> >
> > That's 32M - why?
> 
> I need to check that the command is running, no clue about _which_ one
> is better here.
> 
> > The test continually rewrites 100M of data; only changing one byte/page
> > - so actually the fun way to do the xbzrle test is to leave this as 32M
> > here.....
> 
> My idea here is that we somke test all the transports/commands.  Not
> that I fully test everything.

OK, but if you set the cache size to 150M instead of 32M then xbzrle
should actually kick in and be tested; at 32M very little is being
tested.

> See on my next series, It already takes around 15 seconds, just adding
> stuff will make things go longer really fast.
> 
> BTW, is there a trivial way to convince qtest/getestr whatevre to run
> all the migration tests in parallel?

Not that I know of.

Dave

> Later, Juan.
> 
> 
> 
> >> +    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 it should converge */
> >> +    migrate_set_parameter(from, "downtime-limit", "300");
> >
> > but first increase the xvzrle cache to 120M and give it a couple of
> > seconds delay; you might even find it completes without the downtime
> > limit schange; but you should still do that after a few seconds.
> > You can check the stats as well, you should get some xbzrle cache hits
> > as long as you raised the cache size.
> >
> > Dave
> >
> >> +    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";
> >> @@ -695,6 +762,7 @@ int main(int argc, char **argv)
> >>      qtest_add_func("/migration/precopy/unix", test_precopy_unix);
> >>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
> >>      qtest_add_func("/migration/postcopy/unix", test_postcopy);
> >> +    qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
> >>  
> >>      ret = g_test_run();
> >>  
> >> -- 
> >> 2.14.3
> >> 
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK