diff mbox series

[5/6] tests: Add migration compress threads tests

Message ID 20171004103933.7898-6-quintela@redhat.com
State New
Headers show
Series Add make check tests for Migration | expand

Commit Message

Juan Quintela Oct. 4, 2017, 10:39 a.m. UTC
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(+)

Comments

Peter Xu Oct. 9, 2017, 8:28 a.m. UTC | #1
On Wed, Oct 04, 2017 at 12:39:32PM +0200, Juan Quintela wrote:

[...]

>  int main(int argc, char **argv)
>  {
> @@ -641,6 +689,9 @@ int main(int argc, char **argv)
>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
>      qtest_add_func("/migration/deprecated/unix", test_deprecated_unix);
>      qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);

Nits: Not sure whether we can have better naming for the tests?  Say:

  /migration/precopy/unix
  /migration/precopy/tcp
  /migration/deprecated-cmds
  /migration/xbzrle
  /migration/compression

I'm fine with existing naming as well.

For the deprecated commands test, not sure whether we can just send
those commands and query using "query-migrate-parameters" to make sure
they are setup and valid.  I assume it can be faster than real
migrations.

> +    if (0) {

Is this intended? :)

> +        qtest_add_func("/migration/compress/unix", test_compress_unix);
> +    }
>  
>      ret = g_test_run();
>  
> -- 
> 2.13.5
> 

Thanks,
Dr. David Alan Gilbert Oct. 16, 2017, 4:29 p.m. UTC | #2
* Juan Quintela (quintela@redhat.com) wrote:
> 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 f0734e4ba0..a8abc3d007 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -621,6 +621,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", "100");
> +    /* 1MB/s slow*/
> +    migrate_set_parameter(from, "max-bandwidth", "100000000");
> +
> +    migrate_set_parameter(from, "compress-threads", "4");
> +    migrate_set_parameter(from, "decompress-threads", "3"); 

Should that be 'to' ?

I worry about having this many threads in a loaded test environment.

Dave

> +    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);
> +
> +    /* 1GB/s now it should converge */
> +    migrate_set_parameter(from, "max-bandwidth", "1000000000");
> +
> +    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)
>  {
> @@ -641,6 +689,9 @@ int main(int argc, char **argv)
>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
>      qtest_add_func("/migration/deprecated/unix", test_deprecated_unix);
>      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.13.5
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Juan Quintela Oct. 18, 2017, 11:57 a.m. UTC | #3
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> 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 f0734e4ba0..a8abc3d007 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -621,6 +621,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", "100");
>> +    /* 1MB/s slow*/
>> +    migrate_set_parameter(from, "max-bandwidth", "100000000");
>> +
>> +    migrate_set_parameter(from, "compress-threads", "4");
>> +    migrate_set_parameter(from, "decompress-threads", "3"); 
>
> Should that be 'to' ?

Yes.

> I worry about having this many threads in a loaded test environment.

Suggestion is?

I am not getting they working reliabely without having it loaded.

Later, Juan.
Juan Quintela Oct. 18, 2017, 11:59 a.m. UTC | #4
Peter Xu <peterx@redhat.com> wrote:
> On Wed, Oct 04, 2017 at 12:39:32PM +0200, Juan Quintela wrote:
>
> [...]
>
>>  int main(int argc, char **argv)
>>  {
>> @@ -641,6 +689,9 @@ int main(int argc, char **argv)
>>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
>>      qtest_add_func("/migration/deprecated/unix", test_deprecated_unix);
>>      qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
>
> Nits: Not sure whether we can have better naming for the tests?  Say:
>
>   /migration/precopy/unix
>   /migration/precopy/tcp
>   /migration/deprecated-cmds
>   /migration/xbzrle
>   /migration/compression
>
> I'm fine with existing naming as well.
>
> For the deprecated commands test, not sure whether we can just send
> those commands and query using "query-migrate-parameters" to make sure
> they are setup and valid.  I assume it can be faster than real
> migrations.

Yeap, having done that because I wanted people to say what they thought
before I invested too much time.

>> +    if (0) {
>
> Is this intended? :)

I think I put that on the cover letter.  It is not working reliabely to
me, and as this was more an RFC than anything else ....

Thanks, Juan.
diff mbox series

Patch

diff --git a/tests/migration-test.c b/tests/migration-test.c
index f0734e4ba0..a8abc3d007 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -621,6 +621,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", "100");
+    /* 1MB/s slow*/
+    migrate_set_parameter(from, "max-bandwidth", "100000000");
+
+    migrate_set_parameter(from, "compress-threads", "4");
+    migrate_set_parameter(from, "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);
+
+    /* 1GB/s now it should converge */
+    migrate_set_parameter(from, "max-bandwidth", "1000000000");
+
+    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)
 {
@@ -641,6 +689,9 @@  int main(int argc, char **argv)
     qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
     qtest_add_func("/migration/deprecated/unix", test_deprecated_unix);
     qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
+    if (0) {
+        qtest_add_func("/migration/compress/unix", test_compress_unix);
+    }
 
     ret = g_test_run();