[Qemu-devel] [PATCH v2 0/8] WIP: Multifd compression support

[Qemu-devel] [PATCH v2 0/8] WIP: Multifd compression support

[Juan Quintela]

v2:
- improve the code left and right
- Split better the zlib code
- rename everything to v4.1
- Add tests for multifd-compress zlib
- Parameter is now an enum (soon will see sztd)

ToDo:
- Make operations for diferent methods:
  * multifd_prepare_send_none/zlib
  * multifd_send_none/zlib
  * multifd_recv_none/zlib
- Use the MULTIFD_FLAG_ZLIB (it is unused so far).

Please review and comment.

v1:

This series create compression code on top of multifd.  It is still
WIP, but it is already:
- faster that current compression code
- it does the minimum amount of copies possible
- we allow support for other compression codes
- it pass the multifd test sent in my previous series

Test for existing code didn't work because code is too slow, I need to
make downtime 10 times bigger to make it to converge on my test
machine.  This code works with same limits that multifd no-

ToDo:
- move printf's  to traces
- move code to a struct instead of if (zlib) inside the main threads.
- improve error handling.

Please, review and coment.

Juan Quintela (8):
  migration: Fix migrate_set_parameter
  migration: fix multifd_recv event typo
  migration-test: rename parameter to parameter_int
  tests: Add migration multifd test
  migration-test: introduce functions to handle string parameters
  migration: Add multifd-compress parameter
  multifd: Add zlib compression support
  multifd: rest of zlib compression (WIP)

 hmp.c                        |  23 +++++-
 hw/core/qdev-properties.c    |  11 +++
 include/hw/qdev-properties.h |   1 +
 migration/migration.c        |  25 ++++++
 migration/migration.h        |   1 +
 migration/ram.c              | 140 +++++++++++++++++++++++++++++++--
 migration/trace-events       |   2 +-
 qapi/common.json             |  15 ++++
 qapi/migration.json          |  19 ++++-
 tests/migration-test.c       | 147 +++++++++++++++++++++++++++++------
 10 files changed, 348 insertions(+), 36 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH v2 1/8] migration: Fix migrate_set_parameter

[Juan Quintela]

Otherwise we are setting err twice, what is wrong and causes an abort.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hmp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hmp.c b/hmp.c
index 92941142af..8eec768088 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1825,8 +1825,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
     case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
         p->has_xbzrle_cache_size = true;
         visit_type_size(v, param, &cache_size, &err);
-        if (err || cache_size > INT64_MAX
-            || (size_t)cache_size != cache_size) {
+        if (err) {
+            break;
+        }
+        if (cache_size > INT64_MAX || (size_t)cache_size != cache_size) {
             error_setg(&err, "Invalid size %s", valuestr);
             break;
         }
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 1/8] migration: Fix migrate_set_parameter

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Otherwise we are setting err twice, what is wrong and causes an abort.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

This patch should be broken out as a separate fix.

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

> ---
>  hmp.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 92941142af..8eec768088 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1825,8 +1825,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>      case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
>          p->has_xbzrle_cache_size = true;
>          visit_type_size(v, param, &cache_size, &err);
> -        if (err || cache_size > INT64_MAX
> -            || (size_t)cache_size != cache_size) {
> +        if (err) {
> +            break;
> +        }
> +        if (cache_size > INT64_MAX || (size_t)cache_size != cache_size) {
>              error_setg(&err, "Invalid size %s", valuestr);
>              break;
>          }
> -- 
> 2.20.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 1/8] migration: Fix migrate_set_parameter

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Otherwise we are setting err twice, what is wrong and causes an abort.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Queued just this one.

> ---
>  hmp.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 92941142af..8eec768088 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1825,8 +1825,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>      case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
>          p->has_xbzrle_cache_size = true;
>          visit_type_size(v, param, &cache_size, &err);
> -        if (err || cache_size > INT64_MAX
> -            || (size_t)cache_size != cache_size) {
> +        if (err) {
> +            break;
> +        }
> +        if (cache_size > INT64_MAX || (size_t)cache_size != cache_size) {
>              error_setg(&err, "Invalid size %s", valuestr);
>              break;
>          }
> -- 
> 2.20.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 1/8] migration: Fix migrate_set_parameter

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Otherwise we are setting err twice, what is wrong and causes an abort.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Queued just this one.

> ---
>  hmp.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 92941142af..8eec768088 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1825,8 +1825,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>      case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
>          p->has_xbzrle_cache_size = true;
>          visit_type_size(v, param, &cache_size, &err);
> -        if (err || cache_size > INT64_MAX
> -            || (size_t)cache_size != cache_size) {
> +        if (err) {
> +            break;
> +        }
> +        if (cache_size > INT64_MAX || (size_t)cache_size != cache_size) {
>              error_setg(&err, "Invalid size %s", valuestr);
>              break;
>          }
> -- 
> 2.20.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH v2 2/8] migration: fix multifd_recv event typo

[Juan Quintela]

It uses num in multifd_send().  Make it coherent.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/trace-events | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/trace-events b/migration/trace-events
index de2e136e57..cd50a1e659 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -80,7 +80,7 @@ get_queued_page_not_dirty(const char *block_name, uint64_t tmp_offset, unsigned
 migration_bitmap_sync_start(void) ""
 migration_bitmap_sync_end(uint64_t dirty_pages) "dirty_pages %" PRIu64
 migration_throttle(void) ""
-multifd_recv(uint8_t id, uint64_t packet_num, uint32_t used, uint32_t flags, uint32_t next_packet_size) "channel %d packet number %" PRIu64 " pages %d flags 0x%x next packet size %d"
+multifd_recv(uint8_t id, uint64_t packet_num, uint32_t used, uint32_t flags, uint32_t next_packet_size) "channel %d packet_num %" PRIu64 " pages %d flags 0x%x next packet size %d"
 multifd_recv_sync_main(long packet_num) "packet num %ld"
 multifd_recv_sync_main_signal(uint8_t id) "channel %d"
 multifd_recv_sync_main_wait(uint8_t id) "channel %d"
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 2/8] migration: fix multifd_recv event typo

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> It uses num in multifd_send().  Make it coherent.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

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

> ---
>  migration/trace-events | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/migration/trace-events b/migration/trace-events
> index de2e136e57..cd50a1e659 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -80,7 +80,7 @@ get_queued_page_not_dirty(const char *block_name, uint64_t tmp_offset, unsigned
>  migration_bitmap_sync_start(void) ""
>  migration_bitmap_sync_end(uint64_t dirty_pages) "dirty_pages %" PRIu64
>  migration_throttle(void) ""
> -multifd_recv(uint8_t id, uint64_t packet_num, uint32_t used, uint32_t flags, uint32_t next_packet_size) "channel %d packet number %" PRIu64 " pages %d flags 0x%x next packet size %d"
> +multifd_recv(uint8_t id, uint64_t packet_num, uint32_t used, uint32_t flags, uint32_t next_packet_size) "channel %d packet_num %" PRIu64 " pages %d flags 0x%x next packet size %d"
>  multifd_recv_sync_main(long packet_num) "packet num %ld"
>  multifd_recv_sync_main_signal(uint8_t id) "channel %d"
>  multifd_recv_sync_main_wait(uint8_t id) "channel %d"
> -- 
> 2.20.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH v2 3/8] migration-test: rename parameter to parameter_int

[Juan Quintela]

We would need _str ones on the next patch.

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

diff --git a/tests/migration-test.c b/tests/migration-test.c
index bd3f5c3125..0b25aa3d6c 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -392,7 +392,8 @@ static char *migrate_get_socket_address(QTestState *who, const char *parameter)
     return result;
 }
 
-static long long migrate_get_parameter(QTestState *who, const char *parameter)
+static long long migrate_get_parameter_int(QTestState *who,
+                                           const char *parameter)
 {
     QDict *rsp;
     long long result;
@@ -403,17 +404,17 @@ static long long migrate_get_parameter(QTestState *who, const char *parameter)
     return result;
 }
 
-static void migrate_check_parameter(QTestState *who, const char *parameter,
-                                    long long value)
+static void migrate_check_parameter_int(QTestState *who, const char *parameter,
+                                        long long value)
 {
     long long result;
 
-    result = migrate_get_parameter(who, parameter);
+    result = migrate_get_parameter_int(who, parameter);
     g_assert_cmpint(result, ==, value);
 }
 
-static void migrate_set_parameter(QTestState *who, const char *parameter,
-                                  long long value)
+static void migrate_set_parameter_int(QTestState *who, const char *parameter,
+                                      long long value)
 {
     QDict *rsp;
 
@@ -423,7 +424,7 @@ static void migrate_set_parameter(QTestState *who, const char *parameter,
                     parameter, value);
     g_assert(qdict_haskey(rsp, "return"));
     qobject_unref(rsp);
-    migrate_check_parameter(who, parameter, value);
+    migrate_check_parameter_int(who, parameter, value);
 }
 
 static void migrate_pause(QTestState *who)
@@ -672,7 +673,7 @@ static void deprecated_set_downtime(QTestState *who, const double value)
                     " 'arguments': { 'value': %f } }", value);
     g_assert(qdict_haskey(rsp, "return"));
     qobject_unref(rsp);
-    migrate_check_parameter(who, "downtime-limit", value * 1000);
+    migrate_check_parameter_int(who, "downtime-limit", value * 1000);
 }
 
 static void deprecated_set_speed(QTestState *who, long long value)
@@ -683,7 +684,7 @@ static void deprecated_set_speed(QTestState *who, long long value)
                           "'arguments': { 'value': %lld } }", value);
     g_assert(qdict_haskey(rsp, "return"));
     qobject_unref(rsp);
-    migrate_check_parameter(who, "max-bandwidth", value);
+    migrate_check_parameter_int(who, "max-bandwidth", value);
 }
 
 static void deprecated_set_cache_size(QTestState *who, long long value)
@@ -694,7 +695,7 @@ static void deprecated_set_cache_size(QTestState *who, long long value)
                          "'arguments': { 'value': %lld } }", value);
     g_assert(qdict_haskey(rsp, "return"));
     qobject_unref(rsp);
-    migrate_check_parameter(who, "xbzrle-cache-size", value);
+    migrate_check_parameter_int(who, "xbzrle-cache-size", value);
 }
 
 static void test_deprecated(void)
@@ -729,8 +730,8 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
      * quickly, but that it doesn't complete precopy even on a slow
      * machine, so also set the downtime.
      */
-    migrate_set_parameter(from, "max-bandwidth", 100000000);
-    migrate_set_parameter(from, "downtime-limit", 1);
+    migrate_set_parameter_int(from, "max-bandwidth", 100000000);
+    migrate_set_parameter_int(from, "downtime-limit", 1);
 
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
@@ -781,7 +782,7 @@ static void test_postcopy_recovery(void)
     }
 
     /* Turn postcopy speed down, 4K/s is slow enough on any machines */
-    migrate_set_parameter(from, "max-postcopy-bandwidth", 4096);
+    migrate_set_parameter_int(from, "max-postcopy-bandwidth", 4096);
 
     /* Now we start the postcopy */
     migrate_postcopy_start(from, to);
@@ -822,7 +823,7 @@ static void test_postcopy_recovery(void)
     g_free(uri);
 
     /* Restore the postcopy bandwidth to unlimited */
-    migrate_set_parameter(from, "max-postcopy-bandwidth", 0);
+    migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0);
 
     migrate_postcopy_complete(from, to);
 }
@@ -868,9 +869,9 @@ static void test_precopy_unix(void)
      * machine, so also set the downtime.
      */
     /* 1 ms should make it not converge*/
-    migrate_set_parameter(from, "downtime-limit", 1);
+    migrate_set_parameter_int(from, "downtime-limit", 1);
     /* 1GB/s */
-    migrate_set_parameter(from, "max-bandwidth", 1000000000);
+    migrate_set_parameter_int(from, "max-bandwidth", 1000000000);
 
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
@@ -880,7 +881,7 @@ static void test_precopy_unix(void)
     wait_for_migration_pass(from);
 
     /* 300 ms should converge */
-    migrate_set_parameter(from, "downtime-limit", 300);
+    migrate_set_parameter_int(from, "downtime-limit", 300);
 
     if (!got_stop) {
         qtest_qmp_eventwait(from, "STOP");
@@ -947,11 +948,11 @@ static void test_xbzrle(const char *uri)
      * machine, so also set the downtime.
      */
     /* 1 ms should make it not converge*/
-    migrate_set_parameter(from, "downtime-limit", 1);
+    migrate_set_parameter_int(from, "downtime-limit", 1);
     /* 1GB/s */
-    migrate_set_parameter(from, "max-bandwidth", 1000000000);
+    migrate_set_parameter_int(from, "max-bandwidth", 1000000000);
 
-    migrate_set_parameter(from, "xbzrle-cache-size", 33554432);
+    migrate_set_parameter_int(from, "xbzrle-cache-size", 33554432);
 
     migrate_set_capability(from, "xbzrle", "true");
     migrate_set_capability(to, "xbzrle", "true");
@@ -963,7 +964,7 @@ static void test_xbzrle(const char *uri)
     wait_for_migration_pass(from);
 
     /* 300ms should converge */
-    migrate_set_parameter(from, "downtime-limit", 300);
+    migrate_set_parameter_int(from, "downtime-limit", 300);
 
     if (!got_stop) {
         qtest_qmp_eventwait(from, "STOP");
@@ -999,9 +1000,9 @@ static void test_precopy_tcp(void)
      * machine, so also set the downtime.
      */
     /* 1 ms should make it not converge*/
-    migrate_set_parameter(from, "downtime-limit", 1);
+    migrate_set_parameter_int(from, "downtime-limit", 1);
     /* 1GB/s */
-    migrate_set_parameter(from, "max-bandwidth", 1000000000);
+    migrate_set_parameter_int(from, "max-bandwidth", 1000000000);
 
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
@@ -1013,7 +1014,7 @@ static void test_precopy_tcp(void)
     wait_for_migration_pass(from);
 
     /* 300ms should converge */
-    migrate_set_parameter(from, "downtime-limit", 300);
+    migrate_set_parameter_int(from, "downtime-limit", 300);
 
     if (!got_stop) {
         qtest_qmp_eventwait(from, "STOP");
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 3/8] migration-test: rename parameter to parameter_int

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> We would need _str ones on the next patch.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

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

> ---
>  tests/migration-test.c | 49 +++++++++++++++++++++---------------------
>  1 file changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index bd3f5c3125..0b25aa3d6c 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -392,7 +392,8 @@ static char *migrate_get_socket_address(QTestState *who, const char *parameter)
>      return result;
>  }
>  
> -static long long migrate_get_parameter(QTestState *who, const char *parameter)
> +static long long migrate_get_parameter_int(QTestState *who,
> +                                           const char *parameter)
>  {
>      QDict *rsp;
>      long long result;
> @@ -403,17 +404,17 @@ static long long migrate_get_parameter(QTestState *who, const char *parameter)
>      return result;
>  }
>  
> -static void migrate_check_parameter(QTestState *who, const char *parameter,
> -                                    long long value)
> +static void migrate_check_parameter_int(QTestState *who, const char *parameter,
> +                                        long long value)
>  {
>      long long result;
>  
> -    result = migrate_get_parameter(who, parameter);
> +    result = migrate_get_parameter_int(who, parameter);
>      g_assert_cmpint(result, ==, value);
>  }
>  
> -static void migrate_set_parameter(QTestState *who, const char *parameter,
> -                                  long long value)
> +static void migrate_set_parameter_int(QTestState *who, const char *parameter,
> +                                      long long value)
>  {
>      QDict *rsp;
>  
> @@ -423,7 +424,7 @@ static void migrate_set_parameter(QTestState *who, const char *parameter,
>                      parameter, value);
>      g_assert(qdict_haskey(rsp, "return"));
>      qobject_unref(rsp);
> -    migrate_check_parameter(who, parameter, value);
> +    migrate_check_parameter_int(who, parameter, value);
>  }
>  
>  static void migrate_pause(QTestState *who)
> @@ -672,7 +673,7 @@ static void deprecated_set_downtime(QTestState *who, const double value)
>                      " 'arguments': { 'value': %f } }", value);
>      g_assert(qdict_haskey(rsp, "return"));
>      qobject_unref(rsp);
> -    migrate_check_parameter(who, "downtime-limit", value * 1000);
> +    migrate_check_parameter_int(who, "downtime-limit", value * 1000);
>  }
>  
>  static void deprecated_set_speed(QTestState *who, long long value)
> @@ -683,7 +684,7 @@ static void deprecated_set_speed(QTestState *who, long long value)
>                            "'arguments': { 'value': %lld } }", value);
>      g_assert(qdict_haskey(rsp, "return"));
>      qobject_unref(rsp);
> -    migrate_check_parameter(who, "max-bandwidth", value);
> +    migrate_check_parameter_int(who, "max-bandwidth", value);
>  }
>  
>  static void deprecated_set_cache_size(QTestState *who, long long value)
> @@ -694,7 +695,7 @@ static void deprecated_set_cache_size(QTestState *who, long long value)
>                           "'arguments': { 'value': %lld } }", value);
>      g_assert(qdict_haskey(rsp, "return"));
>      qobject_unref(rsp);
> -    migrate_check_parameter(who, "xbzrle-cache-size", value);
> +    migrate_check_parameter_int(who, "xbzrle-cache-size", value);
>  }
>  
>  static void test_deprecated(void)
> @@ -729,8 +730,8 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
>       * quickly, but that it doesn't complete precopy even on a slow
>       * machine, so also set the downtime.
>       */
> -    migrate_set_parameter(from, "max-bandwidth", 100000000);
> -    migrate_set_parameter(from, "downtime-limit", 1);
> +    migrate_set_parameter_int(from, "max-bandwidth", 100000000);
> +    migrate_set_parameter_int(from, "downtime-limit", 1);
>  
>      /* Wait for the first serial output from the source */
>      wait_for_serial("src_serial");
> @@ -781,7 +782,7 @@ static void test_postcopy_recovery(void)
>      }
>  
>      /* Turn postcopy speed down, 4K/s is slow enough on any machines */
> -    migrate_set_parameter(from, "max-postcopy-bandwidth", 4096);
> +    migrate_set_parameter_int(from, "max-postcopy-bandwidth", 4096);
>  
>      /* Now we start the postcopy */
>      migrate_postcopy_start(from, to);
> @@ -822,7 +823,7 @@ static void test_postcopy_recovery(void)
>      g_free(uri);
>  
>      /* Restore the postcopy bandwidth to unlimited */
> -    migrate_set_parameter(from, "max-postcopy-bandwidth", 0);
> +    migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0);
>  
>      migrate_postcopy_complete(from, to);
>  }
> @@ -868,9 +869,9 @@ static void test_precopy_unix(void)
>       * machine, so also set the downtime.
>       */
>      /* 1 ms should make it not converge*/
> -    migrate_set_parameter(from, "downtime-limit", 1);
> +    migrate_set_parameter_int(from, "downtime-limit", 1);
>      /* 1GB/s */
> -    migrate_set_parameter(from, "max-bandwidth", 1000000000);
> +    migrate_set_parameter_int(from, "max-bandwidth", 1000000000);
>  
>      /* Wait for the first serial output from the source */
>      wait_for_serial("src_serial");
> @@ -880,7 +881,7 @@ static void test_precopy_unix(void)
>      wait_for_migration_pass(from);
>  
>      /* 300 ms should converge */
> -    migrate_set_parameter(from, "downtime-limit", 300);
> +    migrate_set_parameter_int(from, "downtime-limit", 300);
>  
>      if (!got_stop) {
>          qtest_qmp_eventwait(from, "STOP");
> @@ -947,11 +948,11 @@ static void test_xbzrle(const char *uri)
>       * machine, so also set the downtime.
>       */
>      /* 1 ms should make it not converge*/
> -    migrate_set_parameter(from, "downtime-limit", 1);
> +    migrate_set_parameter_int(from, "downtime-limit", 1);
>      /* 1GB/s */
> -    migrate_set_parameter(from, "max-bandwidth", 1000000000);
> +    migrate_set_parameter_int(from, "max-bandwidth", 1000000000);
>  
> -    migrate_set_parameter(from, "xbzrle-cache-size", 33554432);
> +    migrate_set_parameter_int(from, "xbzrle-cache-size", 33554432);
>  
>      migrate_set_capability(from, "xbzrle", "true");
>      migrate_set_capability(to, "xbzrle", "true");
> @@ -963,7 +964,7 @@ static void test_xbzrle(const char *uri)
>      wait_for_migration_pass(from);
>  
>      /* 300ms should converge */
> -    migrate_set_parameter(from, "downtime-limit", 300);
> +    migrate_set_parameter_int(from, "downtime-limit", 300);
>  
>      if (!got_stop) {
>          qtest_qmp_eventwait(from, "STOP");
> @@ -999,9 +1000,9 @@ static void test_precopy_tcp(void)
>       * machine, so also set the downtime.
>       */
>      /* 1 ms should make it not converge*/
> -    migrate_set_parameter(from, "downtime-limit", 1);
> +    migrate_set_parameter_int(from, "downtime-limit", 1);
>      /* 1GB/s */
> -    migrate_set_parameter(from, "max-bandwidth", 1000000000);
> +    migrate_set_parameter_int(from, "max-bandwidth", 1000000000);
>  
>      /* Wait for the first serial output from the source */
>      wait_for_serial("src_serial");
> @@ -1013,7 +1014,7 @@ static void test_precopy_tcp(void)
>      wait_for_migration_pass(from);
>  
>      /* 300ms should converge */
> -    migrate_set_parameter(from, "downtime-limit", 300);
> +    migrate_set_parameter_int(from, "downtime-limit", 300);
>  
>      if (!got_stop) {
>          qtest_qmp_eventwait(from, "STOP");
> -- 
> 2.20.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH v2 4/8] tests: Add migration multifd test

[Juan Quintela]

We set multifd-channels.

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

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 0b25aa3d6c..ff480e0682 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -1028,6 +1028,53 @@ static void test_precopy_tcp(void)
     g_free(uri);
 }
 
+static void test_multifd_tcp(void)
+{
+    char *uri;
+    QTestState *from, *to;
+
+    if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", false, false)) {
+        return;
+    }
+
+    /*
+     * 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_int(from, "downtime-limit", 1);
+    /* 1GB/s */
+    migrate_set_parameter_int(from, "max-bandwidth", 1000000000);
+
+    migrate_set_parameter_int(from, "multifd-channels", 2);
+    migrate_set_parameter_int(to, "multifd-channels", 2);
+
+    migrate_set_capability(from, "multifd", "true");
+    migrate_set_capability(to, "multifd", "true");
+    /* Wait for the first serial output from the source */
+    wait_for_serial("src_serial");
+
+    uri = migrate_get_socket_address(to, "socket-address");
+
+    migrate(from, uri, "{}");
+
+    wait_for_migration_pass(from);
+
+    /* 300ms it should converge */
+    migrate_set_parameter_int(from, "downtime-limit", 600);
+
+    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, true);
+}
+
 int main(int argc, char **argv)
 {
     char template[] = "/tmp/migration-test-XXXXXX";
@@ -1082,6 +1129,7 @@ int main(int argc, char **argv)
     qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
     /* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
     qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
+    qtest_add_func("/migration/multifd/tcp", test_multifd_tcp);
 
     ret = g_test_run();
 
-- 
2.20.1

[Qemu-devel] [PATCH v2 5/8] migration-test: introduce functions to handle string parameters

[Juan Quintela]

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

diff --git a/tests/migration-test.c b/tests/migration-test.c
index ff480e0682..65d5e256a7 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -427,6 +427,43 @@ static void migrate_set_parameter_int(QTestState *who, const char *parameter,
     migrate_check_parameter_int(who, parameter, value);
 }
 
+static char *migrate_get_parameter_str(QTestState *who,
+                                       const char *parameter)
+{
+    QDict *rsp;
+    char *result;
+
+    rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }");
+    result = g_strdup(qdict_get_str(rsp, parameter));
+    qobject_unref(rsp);
+    return result;
+}
+
+static void migrate_check_parameter_str(QTestState *who, const char *parameter,
+                                        const char *value)
+{
+    char *result;
+
+    result = migrate_get_parameter_str(who, parameter);
+    g_assert_cmpstr(result, ==, value);
+    g_free(result);
+}
+
+__attribute__((unused))
+static void migrate_set_parameter_str(QTestState *who, const char *parameter,
+                                      const char *value)
+{
+    QDict *rsp;
+
+    rsp = qtest_qmp(who,
+                    "{ 'execute': 'migrate-set-parameters',"
+                    "'arguments': { %s: %s } }",
+                    parameter, value);
+    g_assert(qdict_haskey(rsp, "return"));
+    qobject_unref(rsp);
+    migrate_check_parameter_str(who, parameter, value);
+}
+
 static void migrate_pause(QTestState *who)
 {
     QDict *rsp;
-- 
2.20.1

[Qemu-devel] [PATCH v2 6/8] migration: Add multifd-compress parameter

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>

---
Rename it to NONE
---
 hmp.c                        | 17 +++++++++++++++++
 hw/core/qdev-properties.c    | 11 +++++++++++
 include/hw/qdev-properties.h |  1 +
 migration/migration.c        | 16 ++++++++++++++++
 qapi/common.json             | 13 +++++++++++++
 qapi/migration.json          | 19 +++++++++++++++----
 tests/migration-test.c       | 13 ++++++++++---
 7 files changed, 83 insertions(+), 7 deletions(-)

diff --git a/hmp.c b/hmp.c
index 8eec768088..02fbe27757 100644
--- a/hmp.c
+++ b/hmp.c
@@ -435,6 +435,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, "%s: %u\n",
             MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_CHANNELS),
             params->multifd_channels);
+        monitor_printf(mon, "%s: %s\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESS),
+            MultifdCompress_str(params->multifd_compress));
         monitor_printf(mon, "%s: %" PRIu64 "\n",
             MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
             params->xbzrle_cache_size);
@@ -1737,6 +1740,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
     MigrateSetParameters *p = g_new0(MigrateSetParameters, 1);
     uint64_t valuebw = 0;
     uint64_t cache_size;
+    int compress_type;
     Error *err = NULL;
     int val, ret;
 
@@ -1822,6 +1826,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
         p->has_multifd_channels = true;
         visit_type_int(v, param, &p->multifd_channels, &err);
         break;
+    case MIGRATION_PARAMETER_MULTIFD_COMPRESS:
+        p->has_multifd_compress = true;
+        visit_type_enum(v, param, &compress_type,
+                        &MultifdCompress_lookup, &err);
+        if (err) {
+            break;
+        }
+        if (compress_type < 0 || compress_type > MULTIFD_COMPRESS__MAX) {
+            error_setg(&err, "Invalid multifd_compress option %s", valuestr);
+            break;
+        }
+        p->multifd_compress = compress_type;
+        break;
     case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
         p->has_xbzrle_cache_size = true;
         visit_type_size(v, param, &cache_size, &err);
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 5da1439a8b..7c8e71532f 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -645,6 +645,17 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
     .set_default_value = set_default_value_enum,
 };
 
+/* --- MultifdCompress --- */
+
+const PropertyInfo qdev_prop_multifd_compress = {
+    .name = "MultifdCompress",
+    .description = "multifd_compress values",
+    .enum_table = &MultifdCompress_lookup,
+    .get = get_enum,
+    .set = set_enum,
+    .set_default_value = set_default_value_enum,
+};
+
 /* --- pci address --- */
 
 /*
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index b6758c852e..ac452d8f2c 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -23,6 +23,7 @@ extern const PropertyInfo qdev_prop_tpm;
 extern const PropertyInfo qdev_prop_ptr;
 extern const PropertyInfo qdev_prop_macaddr;
 extern const PropertyInfo qdev_prop_on_off_auto;
+extern const PropertyInfo qdev_prop_multifd_compress;
 extern const PropertyInfo qdev_prop_losttickpolicy;
 extern const PropertyInfo qdev_prop_blockdev_on_error;
 extern const PropertyInfo qdev_prop_bios_chs_trans;
diff --git a/migration/migration.c b/migration/migration.c
index 609e0df5d0..d6f8ef342a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -82,6 +82,7 @@
 /* The delay time (in ms) between two COLO checkpoints */
 #define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY (200 * 100)
 #define DEFAULT_MIGRATE_MULTIFD_CHANNELS 2
+#define DEFAULT_MIGRATE_MULTIFD_COMPRESS MULTIFD_COMPRESS_NONE
 
 /* Background transfer rate for postcopy, 0 means unlimited, note
  * that page requests can still exceed this limit.
@@ -769,6 +770,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     params->block_incremental = s->parameters.block_incremental;
     params->has_multifd_channels = true;
     params->multifd_channels = s->parameters.multifd_channels;
+    params->has_multifd_compress = true;
+    params->multifd_compress = s->parameters.multifd_compress;
     params->has_xbzrle_cache_size = true;
     params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
     params->has_max_postcopy_bandwidth = true;
@@ -1268,6 +1271,9 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
     if (params->has_multifd_channels) {
         dest->multifd_channels = params->multifd_channels;
     }
+    if (params->has_multifd_compress) {
+        dest->multifd_compress = params->multifd_compress;
+    }
     if (params->has_xbzrle_cache_size) {
         dest->xbzrle_cache_size = params->xbzrle_cache_size;
     }
@@ -1364,6 +1370,9 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
     if (params->has_multifd_channels) {
         s->parameters.multifd_channels = params->multifd_channels;
     }
+    if (params->has_multifd_compress) {
+        s->parameters.multifd_compress = params->multifd_compress;
+    }
     if (params->has_xbzrle_cache_size) {
         s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
         xbzrle_cache_resize(params->xbzrle_cache_size, errp);
@@ -3353,6 +3362,9 @@ void migration_global_dump(Monitor *mon)
 #define DEFINE_PROP_MIG_CAP(name, x)             \
     DEFINE_PROP_BOOL(name, MigrationState, enabled_capabilities[x], false)
 
+#define DEFINE_PROP_MULTIFD_COMPRESS(_n, _s, _f, _d) \
+    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_compress, MultifdCompress)
+
 static Property migration_properties[] = {
     DEFINE_PROP_BOOL("store-global-state", MigrationState,
                      store_global_state, true),
@@ -3392,6 +3404,9 @@ static Property migration_properties[] = {
     DEFINE_PROP_UINT8("multifd-channels", MigrationState,
                       parameters.multifd_channels,
                       DEFAULT_MIGRATE_MULTIFD_CHANNELS),
+    DEFINE_PROP_MULTIFD_COMPRESS("multifd-compress", MigrationState,
+                      parameters.multifd_compress,
+                      DEFAULT_MIGRATE_MULTIFD_COMPRESS),
     DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
                       parameters.xbzrle_cache_size,
                       DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
@@ -3481,6 +3496,7 @@ static void migration_instance_init(Object *obj)
     params->has_x_checkpoint_delay = true;
     params->has_block_incremental = true;
     params->has_multifd_channels = true;
+    params->has_multifd_compress = true;
     params->has_xbzrle_cache_size = true;
     params->has_max_postcopy_bandwidth = true;
     params->has_max_cpu_throttle = true;
diff --git a/qapi/common.json b/qapi/common.json
index 99d313ef3b..7248172792 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -193,3 +193,16 @@
              'ppc64', 'riscv32', 'riscv64', 's390x', 'sh4',
              'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
              'x86_64', 'xtensa', 'xtensaeb' ] }
+
+##
+# @MultifdCompress:
+#
+# An enumeration of multifd compression.
+#
+# @none: no compression.
+#
+# Since: 4.1
+#
+##
+{ 'enum': 'MultifdCompress',
+  'data': [ 'none' ] }
diff --git a/qapi/migration.json b/qapi/migration.json
index 9cfbaf8c6c..629795fd30 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -580,6 +580,9 @@
 # @max-cpu-throttle: maximum cpu throttle percentage.
 #                    Defaults to 99. (Since 3.1)
 #
+# @multifd-compress: What compression method to use.
+#                    Defaults to none. (Since 4.1)
+#
 # Since: 2.4
 ##
 { 'enum': 'MigrationParameter',
@@ -592,7 +595,7 @@
            'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
            'multifd-channels',
            'xbzrle-cache-size', 'max-postcopy-bandwidth',
-           'max-cpu-throttle' ] }
+           'max-cpu-throttle', 'multifd-compress' ] }
 
 ##
 # @MigrateSetParameters:
@@ -680,7 +683,10 @@
 #                     (Since 3.0)
 #
 # @max-cpu-throttle: maximum cpu throttle percentage.
-#                    The default value is 99. (Since 3.1)
+#                    The default value is 99. (Since 4.0)
+#
+# @multifd-compress: What compression method to use.
+#                    Defaults to none. (Since 4.1)
 #
 # Since: 2.4
 ##
@@ -707,7 +713,8 @@
             '*multifd-channels': 'int',
             '*xbzrle-cache-size': 'size',
             '*max-postcopy-bandwidth': 'size',
-	    '*max-cpu-throttle': 'int' } }
+	    '*max-cpu-throttle': 'int',
+            '*multifd-compress': 'MultifdCompress' } }
 
 ##
 # @migrate-set-parameters:
@@ -817,6 +824,9 @@
 #                    Defaults to 99.
 #                     (Since 3.1)
 #
+# @multifd-compress: What compression method to use.
+#                    Defaults to none. (Since 4.1)
+#
 # Since: 2.4
 ##
 { 'struct': 'MigrationParameters',
@@ -840,7 +850,8 @@
             '*multifd-channels': 'uint8',
             '*xbzrle-cache-size': 'size',
 	    '*max-postcopy-bandwidth': 'size',
-            '*max-cpu-throttle':'uint8'} }
+            '*max-cpu-throttle': 'uint8',
+            '*multifd-compress': 'MultifdCompress' } }
 
 ##
 # @query-migrate-parameters:
diff --git a/tests/migration-test.c b/tests/migration-test.c
index 65d5e256a7..8a1ccc2516 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -449,7 +449,6 @@ static void migrate_check_parameter_str(QTestState *who, const char *parameter,
     g_free(result);
 }
 
-__attribute__((unused))
 static void migrate_set_parameter_str(QTestState *who, const char *parameter,
                                       const char *value)
 {
@@ -1065,7 +1064,7 @@ static void test_precopy_tcp(void)
     g_free(uri);
 }
 
-static void test_multifd_tcp(void)
+static void test_multifd_tcp(const char *method)
 {
     char *uri;
     QTestState *from, *to;
@@ -1087,6 +1086,9 @@ static void test_multifd_tcp(void)
     migrate_set_parameter_int(from, "multifd-channels", 2);
     migrate_set_parameter_int(to, "multifd-channels", 2);
 
+    migrate_set_parameter_str(from, "multifd-compress", method);
+    migrate_set_parameter_str(to, "multifd-compress", method);
+
     migrate_set_capability(from, "multifd", "true");
     migrate_set_capability(to, "multifd", "true");
     /* Wait for the first serial output from the source */
@@ -1112,6 +1114,11 @@ static void test_multifd_tcp(void)
     test_migrate_end(from, to, true);
 }
 
+static void test_multifd_tcp_none(void)
+{
+    test_multifd_tcp("none");
+}
+
 int main(int argc, char **argv)
 {
     char template[] = "/tmp/migration-test-XXXXXX";
@@ -1166,7 +1173,7 @@ int main(int argc, char **argv)
     qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
     /* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
     qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
-    qtest_add_func("/migration/multifd/tcp", test_multifd_tcp);
+    qtest_add_func("/migration/multifd/tcp/none", test_multifd_tcp_none);
 
     ret = g_test_run();
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 6/8] migration: Add multifd-compress parameter

[Markus Armbruster]

Juan Quintela <quintela@redhat.com> writes:

> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> ---
> Rename it to NONE
> ---
>  hmp.c                        | 17 +++++++++++++++++
>  hw/core/qdev-properties.c    | 11 +++++++++++
>  include/hw/qdev-properties.h |  1 +
>  migration/migration.c        | 16 ++++++++++++++++
>  qapi/common.json             | 13 +++++++++++++
>  qapi/migration.json          | 19 +++++++++++++++----
>  tests/migration-test.c       | 13 ++++++++++---
>  7 files changed, 83 insertions(+), 7 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 8eec768088..02fbe27757 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -435,6 +435,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
>          monitor_printf(mon, "%s: %u\n",
>              MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_CHANNELS),
>              params->multifd_channels);
> +        monitor_printf(mon, "%s: %s\n",
> +            MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESS),
> +            MultifdCompress_str(params->multifd_compress));
>          monitor_printf(mon, "%s: %" PRIu64 "\n",
>              MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
>              params->xbzrle_cache_size);
> @@ -1737,6 +1740,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>      MigrateSetParameters *p = g_new0(MigrateSetParameters, 1);
>      uint64_t valuebw = 0;
>      uint64_t cache_size;
> +    int compress_type;
>      Error *err = NULL;
>      int val, ret;
>  
> @@ -1822,6 +1826,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>          p->has_multifd_channels = true;
>          visit_type_int(v, param, &p->multifd_channels, &err);
>          break;
> +    case MIGRATION_PARAMETER_MULTIFD_COMPRESS:
> +        p->has_multifd_compress = true;
> +        visit_type_enum(v, param, &compress_type,
> +                        &MultifdCompress_lookup, &err);

visit_type_MultifdCompress(), please.

> +        if (err) {
> +            break;
> +        }
> +        if (compress_type < 0 || compress_type > MULTIFD_COMPRESS__MAX) {
> +            error_setg(&err, "Invalid multifd_compress option %s", valuestr);
> +            break;
> +        }
> +        p->multifd_compress = compress_type;
> +        break;
>      case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
>          p->has_xbzrle_cache_size = true;
>          visit_type_size(v, param, &cache_size, &err);
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 5da1439a8b..7c8e71532f 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -645,6 +645,17 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
>      .set_default_value = set_default_value_enum,
>  };
>  
> +/* --- MultifdCompress --- */
> +
> +const PropertyInfo qdev_prop_multifd_compress = {
> +    .name = "MultifdCompress",
> +    .description = "multifd_compress values",

Similar property declarations list the valid values in .description.

> +    .enum_table = &MultifdCompress_lookup,
> +    .get = get_enum,
> +    .set = set_enum,
> +    .set_default_value = set_default_value_enum,
> +};
> +
>  /* --- pci address --- */
>  
>  /*
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index b6758c852e..ac452d8f2c 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -23,6 +23,7 @@ extern const PropertyInfo qdev_prop_tpm;
>  extern const PropertyInfo qdev_prop_ptr;
>  extern const PropertyInfo qdev_prop_macaddr;
>  extern const PropertyInfo qdev_prop_on_off_auto;
> +extern const PropertyInfo qdev_prop_multifd_compress;
>  extern const PropertyInfo qdev_prop_losttickpolicy;
>  extern const PropertyInfo qdev_prop_blockdev_on_error;
>  extern const PropertyInfo qdev_prop_bios_chs_trans;
> diff --git a/migration/migration.c b/migration/migration.c
> index 609e0df5d0..d6f8ef342a 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -82,6 +82,7 @@
>  /* The delay time (in ms) between two COLO checkpoints */
>  #define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY (200 * 100)
>  #define DEFAULT_MIGRATE_MULTIFD_CHANNELS 2
> +#define DEFAULT_MIGRATE_MULTIFD_COMPRESS MULTIFD_COMPRESS_NONE
>  
>  /* Background transfer rate for postcopy, 0 means unlimited, note
>   * that page requests can still exceed this limit.
> @@ -769,6 +770,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>      params->block_incremental = s->parameters.block_incremental;
>      params->has_multifd_channels = true;
>      params->multifd_channels = s->parameters.multifd_channels;
> +    params->has_multifd_compress = true;
> +    params->multifd_compress = s->parameters.multifd_compress;
>      params->has_xbzrle_cache_size = true;
>      params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
>      params->has_max_postcopy_bandwidth = true;
> @@ -1268,6 +1271,9 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>      if (params->has_multifd_channels) {
>          dest->multifd_channels = params->multifd_channels;
>      }
> +    if (params->has_multifd_compress) {
> +        dest->multifd_compress = params->multifd_compress;
> +    }
>      if (params->has_xbzrle_cache_size) {
>          dest->xbzrle_cache_size = params->xbzrle_cache_size;
>      }
> @@ -1364,6 +1370,9 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>      if (params->has_multifd_channels) {
>          s->parameters.multifd_channels = params->multifd_channels;
>      }
> +    if (params->has_multifd_compress) {
> +        s->parameters.multifd_compress = params->multifd_compress;
> +    }
>      if (params->has_xbzrle_cache_size) {
>          s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
>          xbzrle_cache_resize(params->xbzrle_cache_size, errp);
> @@ -3353,6 +3362,9 @@ void migration_global_dump(Monitor *mon)
>  #define DEFINE_PROP_MIG_CAP(name, x)             \
>      DEFINE_PROP_BOOL(name, MigrationState, enabled_capabilities[x], false)
>  
> +#define DEFINE_PROP_MULTIFD_COMPRESS(_n, _s, _f, _d) \
> +    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_compress, MultifdCompress)
> +

As long as qdev_prop_multifd_compress is exposed in qdev-properties.h,
hiding the macro using it doesn't make sense to me.

>  static Property migration_properties[] = {
>      DEFINE_PROP_BOOL("store-global-state", MigrationState,
>                       store_global_state, true),
> @@ -3392,6 +3404,9 @@ static Property migration_properties[] = {
>      DEFINE_PROP_UINT8("multifd-channels", MigrationState,
>                        parameters.multifd_channels,
>                        DEFAULT_MIGRATE_MULTIFD_CHANNELS),
> +    DEFINE_PROP_MULTIFD_COMPRESS("multifd-compress", MigrationState,
> +                      parameters.multifd_compress,
> +                      DEFAULT_MIGRATE_MULTIFD_COMPRESS),
>      DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
>                        parameters.xbzrle_cache_size,
>                        DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
> @@ -3481,6 +3496,7 @@ static void migration_instance_init(Object *obj)
>      params->has_x_checkpoint_delay = true;
>      params->has_block_incremental = true;
>      params->has_multifd_channels = true;
> +    params->has_multifd_compress = true;
>      params->has_xbzrle_cache_size = true;
>      params->has_max_postcopy_bandwidth = true;
>      params->has_max_cpu_throttle = true;
> diff --git a/qapi/common.json b/qapi/common.json
> index 99d313ef3b..7248172792 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -193,3 +193,16 @@
>               'ppc64', 'riscv32', 'riscv64', 's390x', 'sh4',
>               'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
>               'x86_64', 'xtensa', 'xtensaeb' ] }
> +
> +##
> +# @MultifdCompress:
> +#
> +# An enumeration of multifd compression.
> +#
> +# @none: no compression.
> +#
> +# Since: 4.1
> +#
> +##
> +{ 'enum': 'MultifdCompress',
> +  'data': [ 'none' ] }

Any particular reason for putting this in common.json?  As is, it looks
rather migration-specific to me...

> diff --git a/qapi/migration.json b/qapi/migration.json
> index 9cfbaf8c6c..629795fd30 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -580,6 +580,9 @@
>  # @max-cpu-throttle: maximum cpu throttle percentage.
>  #                    Defaults to 99. (Since 3.1)
>  #
> +# @multifd-compress: What compression method to use.
> +#                    Defaults to none. (Since 4.1)
> +#
>  # Since: 2.4
>  ##
>  { 'enum': 'MigrationParameter',
> @@ -592,7 +595,7 @@
>             'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
>             'multifd-channels',
>             'xbzrle-cache-size', 'max-postcopy-bandwidth',
> -           'max-cpu-throttle' ] }
> +           'max-cpu-throttle', 'multifd-compress' ] }
>  
>  ##
>  # @MigrateSetParameters:
> @@ -680,7 +683,10 @@
>  #                     (Since 3.0)
>  #
>  # @max-cpu-throttle: maximum cpu throttle percentage.
> -#                    The default value is 99. (Since 3.1)
> +#                    The default value is 99. (Since 4.0)
> +#
> +# @multifd-compress: What compression method to use.
> +#                    Defaults to none. (Since 4.1)
>  #
>  # Since: 2.4
>  ##
> @@ -707,7 +713,8 @@
>              '*multifd-channels': 'int',
>              '*xbzrle-cache-size': 'size',
>              '*max-postcopy-bandwidth': 'size',
> -	    '*max-cpu-throttle': 'int' } }
> +	    '*max-cpu-throttle': 'int',
> +            '*multifd-compress': 'MultifdCompress' } }
>  
>  ##
>  # @migrate-set-parameters:
> @@ -817,6 +824,9 @@
>  #                    Defaults to 99.
>  #                     (Since 3.1)
>  #
> +# @multifd-compress: What compression method to use.
> +#                    Defaults to none. (Since 4.1)
> +#
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
> @@ -840,7 +850,8 @@
>              '*multifd-channels': 'uint8',
>              '*xbzrle-cache-size': 'size',
>  	    '*max-postcopy-bandwidth': 'size',
> -            '*max-cpu-throttle':'uint8'} }
> +            '*max-cpu-throttle': 'uint8',
> +            '*multifd-compress': 'MultifdCompress' } }
>  
>  ##
>  # @query-migrate-parameters:
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 65d5e256a7..8a1ccc2516 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -449,7 +449,6 @@ static void migrate_check_parameter_str(QTestState *who, const char *parameter,
>      g_free(result);
>  }
>  
> -__attribute__((unused))
>  static void migrate_set_parameter_str(QTestState *who, const char *parameter,
>                                        const char *value)
>  {
> @@ -1065,7 +1064,7 @@ static void test_precopy_tcp(void)
>      g_free(uri);
>  }
>  
> -static void test_multifd_tcp(void)
> +static void test_multifd_tcp(const char *method)
>  {
>      char *uri;
>      QTestState *from, *to;
> @@ -1087,6 +1086,9 @@ static void test_multifd_tcp(void)
>      migrate_set_parameter_int(from, "multifd-channels", 2);
>      migrate_set_parameter_int(to, "multifd-channels", 2);
>  
> +    migrate_set_parameter_str(from, "multifd-compress", method);
> +    migrate_set_parameter_str(to, "multifd-compress", method);
> +
>      migrate_set_capability(from, "multifd", "true");
>      migrate_set_capability(to, "multifd", "true");
>      /* Wait for the first serial output from the source */
> @@ -1112,6 +1114,11 @@ static void test_multifd_tcp(void)
>      test_migrate_end(from, to, true);
>  }
>  
> +static void test_multifd_tcp_none(void)
> +{
> +    test_multifd_tcp("none");
> +}
> +
>  int main(int argc, char **argv)
>  {
>      char template[] = "/tmp/migration-test-XXXXXX";
> @@ -1166,7 +1173,7 @@ int main(int argc, char **argv)
>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
>      /* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
>      qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
> -    qtest_add_func("/migration/multifd/tcp", test_multifd_tcp);
> +    qtest_add_func("/migration/multifd/tcp/none", test_multifd_tcp_none);
>  
>      ret = g_test_run();

Re: [Qemu-devel] [PATCH v2 6/8] migration: Add multifd-compress parameter

[Markus Armbruster]

Juan Quintela <quintela@redhat.com> writes:

> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> ---
> Rename it to NONE
> ---
>  hmp.c                        | 17 +++++++++++++++++
>  hw/core/qdev-properties.c    | 11 +++++++++++
>  include/hw/qdev-properties.h |  1 +
>  migration/migration.c        | 16 ++++++++++++++++
>  qapi/common.json             | 13 +++++++++++++
>  qapi/migration.json          | 19 +++++++++++++++----
>  tests/migration-test.c       | 13 ++++++++++---
>  7 files changed, 83 insertions(+), 7 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 8eec768088..02fbe27757 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -435,6 +435,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
>          monitor_printf(mon, "%s: %u\n",
>              MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_CHANNELS),
>              params->multifd_channels);
> +        monitor_printf(mon, "%s: %s\n",
> +            MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESS),
> +            MultifdCompress_str(params->multifd_compress));
>          monitor_printf(mon, "%s: %" PRIu64 "\n",
>              MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
>              params->xbzrle_cache_size);
> @@ -1737,6 +1740,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>      MigrateSetParameters *p = g_new0(MigrateSetParameters, 1);
>      uint64_t valuebw = 0;
>      uint64_t cache_size;
> +    int compress_type;
>      Error *err = NULL;
>      int val, ret;
>  
> @@ -1822,6 +1826,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>          p->has_multifd_channels = true;
>          visit_type_int(v, param, &p->multifd_channels, &err);
>          break;
> +    case MIGRATION_PARAMETER_MULTIFD_COMPRESS:
> +        p->has_multifd_compress = true;
> +        visit_type_enum(v, param, &compress_type,
> +                        &MultifdCompress_lookup, &err);

visit_type_MultifdCompress(), please.

> +        if (err) {
> +            break;
> +        }
> +        if (compress_type < 0 || compress_type > MULTIFD_COMPRESS__MAX) {
> +            error_setg(&err, "Invalid multifd_compress option %s", valuestr);
> +            break;
> +        }
> +        p->multifd_compress = compress_type;
> +        break;
>      case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
>          p->has_xbzrle_cache_size = true;
>          visit_type_size(v, param, &cache_size, &err);
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 5da1439a8b..7c8e71532f 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -645,6 +645,17 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
>      .set_default_value = set_default_value_enum,
>  };
>  
> +/* --- MultifdCompress --- */
> +
> +const PropertyInfo qdev_prop_multifd_compress = {
> +    .name = "MultifdCompress",
> +    .description = "multifd_compress values",

Similar property declarations list the valid values in .description.

> +    .enum_table = &MultifdCompress_lookup,
> +    .get = get_enum,
> +    .set = set_enum,
> +    .set_default_value = set_default_value_enum,
> +};
> +
>  /* --- pci address --- */
>  
>  /*
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index b6758c852e..ac452d8f2c 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -23,6 +23,7 @@ extern const PropertyInfo qdev_prop_tpm;
>  extern const PropertyInfo qdev_prop_ptr;
>  extern const PropertyInfo qdev_prop_macaddr;
>  extern const PropertyInfo qdev_prop_on_off_auto;
> +extern const PropertyInfo qdev_prop_multifd_compress;
>  extern const PropertyInfo qdev_prop_losttickpolicy;
>  extern const PropertyInfo qdev_prop_blockdev_on_error;
>  extern const PropertyInfo qdev_prop_bios_chs_trans;
> diff --git a/migration/migration.c b/migration/migration.c
> index 609e0df5d0..d6f8ef342a 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -82,6 +82,7 @@
>  /* The delay time (in ms) between two COLO checkpoints */
>  #define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY (200 * 100)
>  #define DEFAULT_MIGRATE_MULTIFD_CHANNELS 2
> +#define DEFAULT_MIGRATE_MULTIFD_COMPRESS MULTIFD_COMPRESS_NONE
>  
>  /* Background transfer rate for postcopy, 0 means unlimited, note
>   * that page requests can still exceed this limit.
> @@ -769,6 +770,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>      params->block_incremental = s->parameters.block_incremental;
>      params->has_multifd_channels = true;
>      params->multifd_channels = s->parameters.multifd_channels;
> +    params->has_multifd_compress = true;
> +    params->multifd_compress = s->parameters.multifd_compress;
>      params->has_xbzrle_cache_size = true;
>      params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
>      params->has_max_postcopy_bandwidth = true;
> @@ -1268,6 +1271,9 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>      if (params->has_multifd_channels) {
>          dest->multifd_channels = params->multifd_channels;
>      }
> +    if (params->has_multifd_compress) {
> +        dest->multifd_compress = params->multifd_compress;
> +    }
>      if (params->has_xbzrle_cache_size) {
>          dest->xbzrle_cache_size = params->xbzrle_cache_size;
>      }
> @@ -1364,6 +1370,9 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>      if (params->has_multifd_channels) {
>          s->parameters.multifd_channels = params->multifd_channels;
>      }
> +    if (params->has_multifd_compress) {
> +        s->parameters.multifd_compress = params->multifd_compress;
> +    }
>      if (params->has_xbzrle_cache_size) {
>          s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
>          xbzrle_cache_resize(params->xbzrle_cache_size, errp);
> @@ -3353,6 +3362,9 @@ void migration_global_dump(Monitor *mon)
>  #define DEFINE_PROP_MIG_CAP(name, x)             \
>      DEFINE_PROP_BOOL(name, MigrationState, enabled_capabilities[x], false)
>  
> +#define DEFINE_PROP_MULTIFD_COMPRESS(_n, _s, _f, _d) \
> +    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_compress, MultifdCompress)
> +

As long as qdev_prop_multifd_compress is exposed in qdev-properties.h,
hiding the macro using it doesn't make sense to me.

>  static Property migration_properties[] = {
>      DEFINE_PROP_BOOL("store-global-state", MigrationState,
>                       store_global_state, true),
> @@ -3392,6 +3404,9 @@ static Property migration_properties[] = {
>      DEFINE_PROP_UINT8("multifd-channels", MigrationState,
>                        parameters.multifd_channels,
>                        DEFAULT_MIGRATE_MULTIFD_CHANNELS),
> +    DEFINE_PROP_MULTIFD_COMPRESS("multifd-compress", MigrationState,
> +                      parameters.multifd_compress,
> +                      DEFAULT_MIGRATE_MULTIFD_COMPRESS),
>      DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
>                        parameters.xbzrle_cache_size,
>                        DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
> @@ -3481,6 +3496,7 @@ static void migration_instance_init(Object *obj)
>      params->has_x_checkpoint_delay = true;
>      params->has_block_incremental = true;
>      params->has_multifd_channels = true;
> +    params->has_multifd_compress = true;
>      params->has_xbzrle_cache_size = true;
>      params->has_max_postcopy_bandwidth = true;
>      params->has_max_cpu_throttle = true;
> diff --git a/qapi/common.json b/qapi/common.json
> index 99d313ef3b..7248172792 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -193,3 +193,16 @@
>               'ppc64', 'riscv32', 'riscv64', 's390x', 'sh4',
>               'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
>               'x86_64', 'xtensa', 'xtensaeb' ] }
> +
> +##
> +# @MultifdCompress:
> +#
> +# An enumeration of multifd compression.
> +#
> +# @none: no compression.
> +#
> +# Since: 4.1
> +#
> +##
> +{ 'enum': 'MultifdCompress',
> +  'data': [ 'none' ] }

Any particular reason for putting this in common.json?  As is, it looks
rather migration-specific to me...

> diff --git a/qapi/migration.json b/qapi/migration.json
> index 9cfbaf8c6c..629795fd30 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -580,6 +580,9 @@
>  # @max-cpu-throttle: maximum cpu throttle percentage.
>  #                    Defaults to 99. (Since 3.1)
>  #
> +# @multifd-compress: What compression method to use.
> +#                    Defaults to none. (Since 4.1)
> +#
>  # Since: 2.4
>  ##
>  { 'enum': 'MigrationParameter',
> @@ -592,7 +595,7 @@
>             'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
>             'multifd-channels',
>             'xbzrle-cache-size', 'max-postcopy-bandwidth',
> -           'max-cpu-throttle' ] }
> +           'max-cpu-throttle', 'multifd-compress' ] }
>  
>  ##
>  # @MigrateSetParameters:
> @@ -680,7 +683,10 @@
>  #                     (Since 3.0)
>  #
>  # @max-cpu-throttle: maximum cpu throttle percentage.
> -#                    The default value is 99. (Since 3.1)
> +#                    The default value is 99. (Since 4.0)
> +#
> +# @multifd-compress: What compression method to use.
> +#                    Defaults to none. (Since 4.1)
>  #
>  # Since: 2.4
>  ##
> @@ -707,7 +713,8 @@
>              '*multifd-channels': 'int',
>              '*xbzrle-cache-size': 'size',
>              '*max-postcopy-bandwidth': 'size',
> -	    '*max-cpu-throttle': 'int' } }
> +	    '*max-cpu-throttle': 'int',
> +            '*multifd-compress': 'MultifdCompress' } }
>  
>  ##
>  # @migrate-set-parameters:
> @@ -817,6 +824,9 @@
>  #                    Defaults to 99.
>  #                     (Since 3.1)
>  #
> +# @multifd-compress: What compression method to use.
> +#                    Defaults to none. (Since 4.1)
> +#
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
> @@ -840,7 +850,8 @@
>              '*multifd-channels': 'uint8',
>              '*xbzrle-cache-size': 'size',
>  	    '*max-postcopy-bandwidth': 'size',
> -            '*max-cpu-throttle':'uint8'} }
> +            '*max-cpu-throttle': 'uint8',
> +            '*multifd-compress': 'MultifdCompress' } }
>  
>  ##
>  # @query-migrate-parameters:
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 65d5e256a7..8a1ccc2516 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -449,7 +449,6 @@ static void migrate_check_parameter_str(QTestState *who, const char *parameter,
>      g_free(result);
>  }
>  
> -__attribute__((unused))
>  static void migrate_set_parameter_str(QTestState *who, const char *parameter,
>                                        const char *value)
>  {
> @@ -1065,7 +1064,7 @@ static void test_precopy_tcp(void)
>      g_free(uri);
>  }
>  
> -static void test_multifd_tcp(void)
> +static void test_multifd_tcp(const char *method)
>  {
>      char *uri;
>      QTestState *from, *to;
> @@ -1087,6 +1086,9 @@ static void test_multifd_tcp(void)
>      migrate_set_parameter_int(from, "multifd-channels", 2);
>      migrate_set_parameter_int(to, "multifd-channels", 2);
>  
> +    migrate_set_parameter_str(from, "multifd-compress", method);
> +    migrate_set_parameter_str(to, "multifd-compress", method);
> +
>      migrate_set_capability(from, "multifd", "true");
>      migrate_set_capability(to, "multifd", "true");
>      /* Wait for the first serial output from the source */
> @@ -1112,6 +1114,11 @@ static void test_multifd_tcp(void)
>      test_migrate_end(from, to, true);
>  }
>  
> +static void test_multifd_tcp_none(void)
> +{
> +    test_multifd_tcp("none");
> +}
> +
>  int main(int argc, char **argv)
>  {
>      char template[] = "/tmp/migration-test-XXXXXX";
> @@ -1166,7 +1173,7 @@ int main(int argc, char **argv)
>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
>      /* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
>      qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
> -    qtest_add_func("/migration/multifd/tcp", test_multifd_tcp);
> +    qtest_add_func("/migration/multifd/tcp/none", test_multifd_tcp_none);
>  
>      ret = g_test_run();

Re: [Qemu-devel] [PATCH v2 6/8] migration: Add multifd-compress parameter

[Juan Quintela]

Markus Armbruster <armbru@redhat.com> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>
>> ---
>> Rename it to NONE

>> @@ -1822,6 +1826,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>>          p->has_multifd_channels = true;
>>          visit_type_int(v, param, &p->multifd_channels, &err);
>>          break;
>> +    case MIGRATION_PARAMETER_MULTIFD_COMPRESS:
>> +        p->has_multifd_compress = true;
>> +        visit_type_enum(v, param, &compress_type,
>> +                        &MultifdCompress_lookup, &err);
>
> visit_type_MultifdCompress(), please.

done.

Interesting that I can

#include "qapi/qapi-visit-common.h"

but not what I would expect/want:

#include "qapi/qapi-visit.h"

Perhaps we should remove

#include "qapi-visit-target.h"

from there?

Anyways, independent of this patch.

>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>> index 5da1439a8b..7c8e71532f 100644
>> --- a/hw/core/qdev-properties.c
>> +++ b/hw/core/qdev-properties.c
>> @@ -645,6 +645,17 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
>>      .set_default_value = set_default_value_enum,
>>  };
>>  
>> +/* --- MultifdCompress --- */
>> +
>> +const PropertyInfo qdev_prop_multifd_compress = {
>> +    .name = "MultifdCompress",
>> +    .description = "multifd_compress values",
>
> Similar property declarations list the valid values in .description.

Fixed, thanks.

>>  
>> +#define DEFINE_PROP_MULTIFD_COMPRESS(_n, _s, _f, _d) \
>> +    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_compress, MultifdCompress)
>> +
>
> As long as qdev_prop_multifd_compress is exposed in qdev-properties.h,
> hiding the macro using it doesn't make sense to me.

Where do you want it to be?  This should only be used here, but if you
want it anywhere else, told me where.


>> +##
>> +# @MultifdCompress:
>> +#
>> +# An enumeration of multifd compression.
>> +#
>> +# @none: no compression.
>> +#
>> +# Since: 4.1
>> +#
>> +##
>> +{ 'enum': 'MultifdCompress',
>> +  'data': [ 'none' ] }
>
> Any particular reason for putting this in common.json?  As is, it looks
> rather migration-specific to me...

Not sure if with new "qapi" compiler it works, it used to be that it
failed if you declared an enum anywhere else.  See how I have to put
property info into qdev-properties.c instead of any migration file.


Thanks, Juan.

Re: [Qemu-devel] [PATCH v2 6/8] migration: Add multifd-compress parameter

[Markus Armbruster]

Juan Quintela <quintela@redhat.com> writes:

> Markus Armbruster <armbru@redhat.com> wrote:
>> Juan Quintela <quintela@redhat.com> writes:
>>
>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>>
>>> ---
>>> Rename it to NONE
>
>>> @@ -1822,6 +1826,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>>>          p->has_multifd_channels = true;
>>>          visit_type_int(v, param, &p->multifd_channels, &err);
>>>          break;
>>> +    case MIGRATION_PARAMETER_MULTIFD_COMPRESS:
>>> +        p->has_multifd_compress = true;
>>> +        visit_type_enum(v, param, &compress_type,
>>> +                        &MultifdCompress_lookup, &err);
>>
>> visit_type_MultifdCompress(), please.
>
> done.
>
> Interesting that I can
>
> #include "qapi/qapi-visit-common.h"
>
> but not what I would expect/want:
>
> #include "qapi/qapi-visit.h"

I guess you tried to include it into target-independent code, which
doesn't work since we added target conditionals like

    { 'command': 'rtc-reset-reinjection',
      'if': 'defined(TARGET_I386)' }

Adding these (merge commit a0430dd8abb) was only possible because the
code generated from the QAPI schema mirrors the QAPI schema's modular
structure: just like qapi-schema.json includes its sub-modules such as
common.json, the generated qapi-visit.h includes the headers generated
for its sub-modules such as qapi-visit-common.h.  See commit 252dc3105f
"qapi: Generate separate .h, .c for each module".

The original motivation for this modularization was actually compile
time.  Before modularization, touching the QAPI schema typically
recompiled everything using QAPI, which nowadays means pretty much
everything.  Modularization let me replace "include all generated QAPI
stuff" by "include just the generated QAPI stuff I actually need", for
*massive* compile time improvements.

Back to target conditionals.  Right now, we confine them to sub-module
target.json.  This means the headers generated for this sub-module, such
as qapi-visit-target.h, can only be included into target-dependent code.
Since qapi-visit.h includes all sub-modules, it can also only be
included there.

> Perhaps we should remove
>
> #include "qapi-visit-target.h"
>
> from there?

No for two reasons:

1. The code generating qapi-visit.h has no idea which sub-modules are
target-specific.

2. You shouldn't include qapi-visit.h anyway, it's bad for compile time.

> Anyways, independent of this patch.

Yes.

>>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>>> index 5da1439a8b..7c8e71532f 100644
>>> --- a/hw/core/qdev-properties.c
>>> +++ b/hw/core/qdev-properties.c
>>> @@ -645,6 +645,17 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
>>>      .set_default_value = set_default_value_enum,
>>>  };
>>>  
>>> +/* --- MultifdCompress --- */
>>> +
>>> +const PropertyInfo qdev_prop_multifd_compress = {
>>> +    .name = "MultifdCompress",
>>> +    .description = "multifd_compress values",
>>
>> Similar property declarations list the valid values in .description.
>
> Fixed, thanks.
>
>>>  
>>> +#define DEFINE_PROP_MULTIFD_COMPRESS(_n, _s, _f, _d) \
>>> +    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_compress, MultifdCompress)
>>> +
>>
>> As long as qdev_prop_multifd_compress is exposed in qdev-properties.h,
>> hiding the macro using it doesn't make sense to me.
>
> Where do you want it to be?  This should only be used here, but if you
> want it anywhere else, told me where.

Put the declaration of qdev_prop_multifd_compress and the macro in the
same source file.  Simplest: qdev-properties.h.

>>> +##
>>> +# @MultifdCompress:
>>> +#
>>> +# An enumeration of multifd compression.
>>> +#
>>> +# @none: no compression.
>>> +#
>>> +# Since: 4.1
>>> +#
>>> +##
>>> +{ 'enum': 'MultifdCompress',
>>> +  'data': [ 'none' ] }
>>
>> Any particular reason for putting this in common.json?  As is, it looks
>> rather migration-specific to me...
>
> Not sure if with new "qapi" compiler it works, it used to be that it
> failed if you declared an enum anywhere else.  See how I have to put
> property info into qdev-properties.c instead of any migration file.

You can certainly declare enums anywhere, you just have to make sure to
include the right headers in the right places, just like for manually
written enums in C headers.

Rules for modular QAPI schema:

1. If module A uses an entity defined in module B, A must include B

   Example: since migration.json uses MultifdCompress defined in
   common.json, migration.json must include common.json (it does).

2. No circular includes, ever.

3. Try to avoid cross-module dependencies

   Example: migration.json uses MultifdCompress defined in common.json.
   This is a cross-module dependency.  Can we avoid it by moving
   MultifdCompress into migration.json?

   The QAPI schema would be just fine with such a move, but C code using
   the generated headers might be unhappy, because it now has to include
   qapi-types-migration.h instead of just qapi-types-common.h.

   My question was: is C code really unhappy?  Please find out and tell
   me.

Re: [Qemu-devel] [PATCH v2 6/8] migration: Add multifd-compress parameter

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> 
> ---
> Rename it to NONE
> ---
>  hmp.c                        | 17 +++++++++++++++++
>  hw/core/qdev-properties.c    | 11 +++++++++++
>  include/hw/qdev-properties.h |  1 +
>  migration/migration.c        | 16 ++++++++++++++++
>  qapi/common.json             | 13 +++++++++++++
>  qapi/migration.json          | 19 +++++++++++++++----
>  tests/migration-test.c       | 13 ++++++++++---
>  7 files changed, 83 insertions(+), 7 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 8eec768088..02fbe27757 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -435,6 +435,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
>          monitor_printf(mon, "%s: %u\n",
>              MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_CHANNELS),
>              params->multifd_channels);
> +        monitor_printf(mon, "%s: %s\n",
> +            MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESS),
> +            MultifdCompress_str(params->multifd_compress));
>          monitor_printf(mon, "%s: %" PRIu64 "\n",
>              MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
>              params->xbzrle_cache_size);
> @@ -1737,6 +1740,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>      MigrateSetParameters *p = g_new0(MigrateSetParameters, 1);
>      uint64_t valuebw = 0;
>      uint64_t cache_size;
> +    int compress_type;
>      Error *err = NULL;
>      int val, ret;
>  
> @@ -1822,6 +1826,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>          p->has_multifd_channels = true;
>          visit_type_int(v, param, &p->multifd_channels, &err);
>          break;
> +    case MIGRATION_PARAMETER_MULTIFD_COMPRESS:
> +        p->has_multifd_compress = true;
> +        visit_type_enum(v, param, &compress_type,
> +                        &MultifdCompress_lookup, &err);
> +        if (err) {
> +            break;
> +        }
> +        if (compress_type < 0 || compress_type > MULTIFD_COMPRESS__MAX) {

I still think that needs to be >= rather than > ?

Dave

> +            error_setg(&err, "Invalid multifd_compress option %s", valuestr);
> +            break;
> +        }
> +        p->multifd_compress = compress_type;
> +        break;
>      case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
>          p->has_xbzrle_cache_size = true;
>          visit_type_size(v, param, &cache_size, &err);
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 5da1439a8b..7c8e71532f 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -645,6 +645,17 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
>      .set_default_value = set_default_value_enum,
>  };
>  
> +/* --- MultifdCompress --- */
> +
> +const PropertyInfo qdev_prop_multifd_compress = {
> +    .name = "MultifdCompress",
> +    .description = "multifd_compress values",
> +    .enum_table = &MultifdCompress_lookup,
> +    .get = get_enum,
> +    .set = set_enum,
> +    .set_default_value = set_default_value_enum,
> +};
> +
>  /* --- pci address --- */
>  
>  /*
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index b6758c852e..ac452d8f2c 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -23,6 +23,7 @@ extern const PropertyInfo qdev_prop_tpm;
>  extern const PropertyInfo qdev_prop_ptr;
>  extern const PropertyInfo qdev_prop_macaddr;
>  extern const PropertyInfo qdev_prop_on_off_auto;
> +extern const PropertyInfo qdev_prop_multifd_compress;
>  extern const PropertyInfo qdev_prop_losttickpolicy;
>  extern const PropertyInfo qdev_prop_blockdev_on_error;
>  extern const PropertyInfo qdev_prop_bios_chs_trans;
> diff --git a/migration/migration.c b/migration/migration.c
> index 609e0df5d0..d6f8ef342a 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -82,6 +82,7 @@
>  /* The delay time (in ms) between two COLO checkpoints */
>  #define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY (200 * 100)
>  #define DEFAULT_MIGRATE_MULTIFD_CHANNELS 2
> +#define DEFAULT_MIGRATE_MULTIFD_COMPRESS MULTIFD_COMPRESS_NONE
>  
>  /* Background transfer rate for postcopy, 0 means unlimited, note
>   * that page requests can still exceed this limit.
> @@ -769,6 +770,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>      params->block_incremental = s->parameters.block_incremental;
>      params->has_multifd_channels = true;
>      params->multifd_channels = s->parameters.multifd_channels;
> +    params->has_multifd_compress = true;
> +    params->multifd_compress = s->parameters.multifd_compress;
>      params->has_xbzrle_cache_size = true;
>      params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
>      params->has_max_postcopy_bandwidth = true;
> @@ -1268,6 +1271,9 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>      if (params->has_multifd_channels) {
>          dest->multifd_channels = params->multifd_channels;
>      }
> +    if (params->has_multifd_compress) {
> +        dest->multifd_compress = params->multifd_compress;
> +    }
>      if (params->has_xbzrle_cache_size) {
>          dest->xbzrle_cache_size = params->xbzrle_cache_size;
>      }
> @@ -1364,6 +1370,9 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>      if (params->has_multifd_channels) {
>          s->parameters.multifd_channels = params->multifd_channels;
>      }
> +    if (params->has_multifd_compress) {
> +        s->parameters.multifd_compress = params->multifd_compress;
> +    }
>      if (params->has_xbzrle_cache_size) {
>          s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
>          xbzrle_cache_resize(params->xbzrle_cache_size, errp);
> @@ -3353,6 +3362,9 @@ void migration_global_dump(Monitor *mon)
>  #define DEFINE_PROP_MIG_CAP(name, x)             \
>      DEFINE_PROP_BOOL(name, MigrationState, enabled_capabilities[x], false)
>  
> +#define DEFINE_PROP_MULTIFD_COMPRESS(_n, _s, _f, _d) \
> +    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_compress, MultifdCompress)
> +
>  static Property migration_properties[] = {
>      DEFINE_PROP_BOOL("store-global-state", MigrationState,
>                       store_global_state, true),
> @@ -3392,6 +3404,9 @@ static Property migration_properties[] = {
>      DEFINE_PROP_UINT8("multifd-channels", MigrationState,
>                        parameters.multifd_channels,
>                        DEFAULT_MIGRATE_MULTIFD_CHANNELS),
> +    DEFINE_PROP_MULTIFD_COMPRESS("multifd-compress", MigrationState,
> +                      parameters.multifd_compress,
> +                      DEFAULT_MIGRATE_MULTIFD_COMPRESS),
>      DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
>                        parameters.xbzrle_cache_size,
>                        DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
> @@ -3481,6 +3496,7 @@ static void migration_instance_init(Object *obj)
>      params->has_x_checkpoint_delay = true;
>      params->has_block_incremental = true;
>      params->has_multifd_channels = true;
> +    params->has_multifd_compress = true;
>      params->has_xbzrle_cache_size = true;
>      params->has_max_postcopy_bandwidth = true;
>      params->has_max_cpu_throttle = true;
> diff --git a/qapi/common.json b/qapi/common.json
> index 99d313ef3b..7248172792 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -193,3 +193,16 @@
>               'ppc64', 'riscv32', 'riscv64', 's390x', 'sh4',
>               'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
>               'x86_64', 'xtensa', 'xtensaeb' ] }
> +
> +##
> +# @MultifdCompress:
> +#
> +# An enumeration of multifd compression.
> +#
> +# @none: no compression.
> +#
> +# Since: 4.1
> +#
> +##
> +{ 'enum': 'MultifdCompress',
> +  'data': [ 'none' ] }
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 9cfbaf8c6c..629795fd30 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -580,6 +580,9 @@
>  # @max-cpu-throttle: maximum cpu throttle percentage.
>  #                    Defaults to 99. (Since 3.1)
>  #
> +# @multifd-compress: What compression method to use.
> +#                    Defaults to none. (Since 4.1)
> +#
>  # Since: 2.4
>  ##
>  { 'enum': 'MigrationParameter',
> @@ -592,7 +595,7 @@
>             'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
>             'multifd-channels',
>             'xbzrle-cache-size', 'max-postcopy-bandwidth',
> -           'max-cpu-throttle' ] }
> +           'max-cpu-throttle', 'multifd-compress' ] }
>  
>  ##
>  # @MigrateSetParameters:
> @@ -680,7 +683,10 @@
>  #                     (Since 3.0)
>  #
>  # @max-cpu-throttle: maximum cpu throttle percentage.
> -#                    The default value is 99. (Since 3.1)
> +#                    The default value is 99. (Since 4.0)
> +#
> +# @multifd-compress: What compression method to use.
> +#                    Defaults to none. (Since 4.1)
>  #
>  # Since: 2.4
>  ##
> @@ -707,7 +713,8 @@
>              '*multifd-channels': 'int',
>              '*xbzrle-cache-size': 'size',
>              '*max-postcopy-bandwidth': 'size',
> -	    '*max-cpu-throttle': 'int' } }
> +	    '*max-cpu-throttle': 'int',
> +            '*multifd-compress': 'MultifdCompress' } }
>  
>  ##
>  # @migrate-set-parameters:
> @@ -817,6 +824,9 @@
>  #                    Defaults to 99.
>  #                     (Since 3.1)
>  #
> +# @multifd-compress: What compression method to use.
> +#                    Defaults to none. (Since 4.1)
> +#
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
> @@ -840,7 +850,8 @@
>              '*multifd-channels': 'uint8',
>              '*xbzrle-cache-size': 'size',
>  	    '*max-postcopy-bandwidth': 'size',
> -            '*max-cpu-throttle':'uint8'} }
> +            '*max-cpu-throttle': 'uint8',
> +            '*multifd-compress': 'MultifdCompress' } }
>  
>  ##
>  # @query-migrate-parameters:
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 65d5e256a7..8a1ccc2516 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -449,7 +449,6 @@ static void migrate_check_parameter_str(QTestState *who, const char *parameter,
>      g_free(result);
>  }
>  
> -__attribute__((unused))
>  static void migrate_set_parameter_str(QTestState *who, const char *parameter,
>                                        const char *value)
>  {
> @@ -1065,7 +1064,7 @@ static void test_precopy_tcp(void)
>      g_free(uri);
>  }
>  
> -static void test_multifd_tcp(void)
> +static void test_multifd_tcp(const char *method)
>  {
>      char *uri;
>      QTestState *from, *to;
> @@ -1087,6 +1086,9 @@ static void test_multifd_tcp(void)
>      migrate_set_parameter_int(from, "multifd-channels", 2);
>      migrate_set_parameter_int(to, "multifd-channels", 2);
>  
> +    migrate_set_parameter_str(from, "multifd-compress", method);
> +    migrate_set_parameter_str(to, "multifd-compress", method);
> +
>      migrate_set_capability(from, "multifd", "true");
>      migrate_set_capability(to, "multifd", "true");
>      /* Wait for the first serial output from the source */
> @@ -1112,6 +1114,11 @@ static void test_multifd_tcp(void)
>      test_migrate_end(from, to, true);
>  }
>  
> +static void test_multifd_tcp_none(void)
> +{
> +    test_multifd_tcp("none");
> +}
> +
>  int main(int argc, char **argv)
>  {
>      char template[] = "/tmp/migration-test-XXXXXX";
> @@ -1166,7 +1173,7 @@ int main(int argc, char **argv)
>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
>      /* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
>      qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
> -    qtest_add_func("/migration/multifd/tcp", test_multifd_tcp);
> +    qtest_add_func("/migration/multifd/tcp/none", test_multifd_tcp_none);
>  
>      ret = g_test_run();
>  
> -- 
> 2.20.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 6/8] migration: Add multifd-compress parameter

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> 
> ---
> Rename it to NONE
> ---
>  hmp.c                        | 17 +++++++++++++++++
>  hw/core/qdev-properties.c    | 11 +++++++++++
>  include/hw/qdev-properties.h |  1 +
>  migration/migration.c        | 16 ++++++++++++++++
>  qapi/common.json             | 13 +++++++++++++
>  qapi/migration.json          | 19 +++++++++++++++----
>  tests/migration-test.c       | 13 ++++++++++---
>  7 files changed, 83 insertions(+), 7 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 8eec768088..02fbe27757 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -435,6 +435,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
>          monitor_printf(mon, "%s: %u\n",
>              MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_CHANNELS),
>              params->multifd_channels);
> +        monitor_printf(mon, "%s: %s\n",
> +            MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESS),
> +            MultifdCompress_str(params->multifd_compress));
>          monitor_printf(mon, "%s: %" PRIu64 "\n",
>              MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
>              params->xbzrle_cache_size);
> @@ -1737,6 +1740,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>      MigrateSetParameters *p = g_new0(MigrateSetParameters, 1);
>      uint64_t valuebw = 0;
>      uint64_t cache_size;
> +    int compress_type;
>      Error *err = NULL;
>      int val, ret;
>  
> @@ -1822,6 +1826,19 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>          p->has_multifd_channels = true;
>          visit_type_int(v, param, &p->multifd_channels, &err);
>          break;
> +    case MIGRATION_PARAMETER_MULTIFD_COMPRESS:
> +        p->has_multifd_compress = true;
> +        visit_type_enum(v, param, &compress_type,
> +                        &MultifdCompress_lookup, &err);
> +        if (err) {
> +            break;
> +        }
> +        if (compress_type < 0 || compress_type > MULTIFD_COMPRESS__MAX) {

I still think that needs to be >= rather than > ?

Dave

> +            error_setg(&err, "Invalid multifd_compress option %s", valuestr);
> +            break;
> +        }
> +        p->multifd_compress = compress_type;
> +        break;
>      case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
>          p->has_xbzrle_cache_size = true;
>          visit_type_size(v, param, &cache_size, &err);
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 5da1439a8b..7c8e71532f 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -645,6 +645,17 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
>      .set_default_value = set_default_value_enum,
>  };
>  
> +/* --- MultifdCompress --- */
> +
> +const PropertyInfo qdev_prop_multifd_compress = {
> +    .name = "MultifdCompress",
> +    .description = "multifd_compress values",
> +    .enum_table = &MultifdCompress_lookup,
> +    .get = get_enum,
> +    .set = set_enum,
> +    .set_default_value = set_default_value_enum,
> +};
> +
>  /* --- pci address --- */
>  
>  /*
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index b6758c852e..ac452d8f2c 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -23,6 +23,7 @@ extern const PropertyInfo qdev_prop_tpm;
>  extern const PropertyInfo qdev_prop_ptr;
>  extern const PropertyInfo qdev_prop_macaddr;
>  extern const PropertyInfo qdev_prop_on_off_auto;
> +extern const PropertyInfo qdev_prop_multifd_compress;
>  extern const PropertyInfo qdev_prop_losttickpolicy;
>  extern const PropertyInfo qdev_prop_blockdev_on_error;
>  extern const PropertyInfo qdev_prop_bios_chs_trans;
> diff --git a/migration/migration.c b/migration/migration.c
> index 609e0df5d0..d6f8ef342a 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -82,6 +82,7 @@
>  /* The delay time (in ms) between two COLO checkpoints */
>  #define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY (200 * 100)
>  #define DEFAULT_MIGRATE_MULTIFD_CHANNELS 2
> +#define DEFAULT_MIGRATE_MULTIFD_COMPRESS MULTIFD_COMPRESS_NONE
>  
>  /* Background transfer rate for postcopy, 0 means unlimited, note
>   * that page requests can still exceed this limit.
> @@ -769,6 +770,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>      params->block_incremental = s->parameters.block_incremental;
>      params->has_multifd_channels = true;
>      params->multifd_channels = s->parameters.multifd_channels;
> +    params->has_multifd_compress = true;
> +    params->multifd_compress = s->parameters.multifd_compress;
>      params->has_xbzrle_cache_size = true;
>      params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
>      params->has_max_postcopy_bandwidth = true;
> @@ -1268,6 +1271,9 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>      if (params->has_multifd_channels) {
>          dest->multifd_channels = params->multifd_channels;
>      }
> +    if (params->has_multifd_compress) {
> +        dest->multifd_compress = params->multifd_compress;
> +    }
>      if (params->has_xbzrle_cache_size) {
>          dest->xbzrle_cache_size = params->xbzrle_cache_size;
>      }
> @@ -1364,6 +1370,9 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>      if (params->has_multifd_channels) {
>          s->parameters.multifd_channels = params->multifd_channels;
>      }
> +    if (params->has_multifd_compress) {
> +        s->parameters.multifd_compress = params->multifd_compress;
> +    }
>      if (params->has_xbzrle_cache_size) {
>          s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
>          xbzrle_cache_resize(params->xbzrle_cache_size, errp);
> @@ -3353,6 +3362,9 @@ void migration_global_dump(Monitor *mon)
>  #define DEFINE_PROP_MIG_CAP(name, x)             \
>      DEFINE_PROP_BOOL(name, MigrationState, enabled_capabilities[x], false)
>  
> +#define DEFINE_PROP_MULTIFD_COMPRESS(_n, _s, _f, _d) \
> +    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_compress, MultifdCompress)
> +
>  static Property migration_properties[] = {
>      DEFINE_PROP_BOOL("store-global-state", MigrationState,
>                       store_global_state, true),
> @@ -3392,6 +3404,9 @@ static Property migration_properties[] = {
>      DEFINE_PROP_UINT8("multifd-channels", MigrationState,
>                        parameters.multifd_channels,
>                        DEFAULT_MIGRATE_MULTIFD_CHANNELS),
> +    DEFINE_PROP_MULTIFD_COMPRESS("multifd-compress", MigrationState,
> +                      parameters.multifd_compress,
> +                      DEFAULT_MIGRATE_MULTIFD_COMPRESS),
>      DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
>                        parameters.xbzrle_cache_size,
>                        DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
> @@ -3481,6 +3496,7 @@ static void migration_instance_init(Object *obj)
>      params->has_x_checkpoint_delay = true;
>      params->has_block_incremental = true;
>      params->has_multifd_channels = true;
> +    params->has_multifd_compress = true;
>      params->has_xbzrle_cache_size = true;
>      params->has_max_postcopy_bandwidth = true;
>      params->has_max_cpu_throttle = true;
> diff --git a/qapi/common.json b/qapi/common.json
> index 99d313ef3b..7248172792 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -193,3 +193,16 @@
>               'ppc64', 'riscv32', 'riscv64', 's390x', 'sh4',
>               'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
>               'x86_64', 'xtensa', 'xtensaeb' ] }
> +
> +##
> +# @MultifdCompress:
> +#
> +# An enumeration of multifd compression.
> +#
> +# @none: no compression.
> +#
> +# Since: 4.1
> +#
> +##
> +{ 'enum': 'MultifdCompress',
> +  'data': [ 'none' ] }
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 9cfbaf8c6c..629795fd30 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -580,6 +580,9 @@
>  # @max-cpu-throttle: maximum cpu throttle percentage.
>  #                    Defaults to 99. (Since 3.1)
>  #
> +# @multifd-compress: What compression method to use.
> +#                    Defaults to none. (Since 4.1)
> +#
>  # Since: 2.4
>  ##
>  { 'enum': 'MigrationParameter',
> @@ -592,7 +595,7 @@
>             'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
>             'multifd-channels',
>             'xbzrle-cache-size', 'max-postcopy-bandwidth',
> -           'max-cpu-throttle' ] }
> +           'max-cpu-throttle', 'multifd-compress' ] }
>  
>  ##
>  # @MigrateSetParameters:
> @@ -680,7 +683,10 @@
>  #                     (Since 3.0)
>  #
>  # @max-cpu-throttle: maximum cpu throttle percentage.
> -#                    The default value is 99. (Since 3.1)
> +#                    The default value is 99. (Since 4.0)
> +#
> +# @multifd-compress: What compression method to use.
> +#                    Defaults to none. (Since 4.1)
>  #
>  # Since: 2.4
>  ##
> @@ -707,7 +713,8 @@
>              '*multifd-channels': 'int',
>              '*xbzrle-cache-size': 'size',
>              '*max-postcopy-bandwidth': 'size',
> -	    '*max-cpu-throttle': 'int' } }
> +	    '*max-cpu-throttle': 'int',
> +            '*multifd-compress': 'MultifdCompress' } }
>  
>  ##
>  # @migrate-set-parameters:
> @@ -817,6 +824,9 @@
>  #                    Defaults to 99.
>  #                     (Since 3.1)
>  #
> +# @multifd-compress: What compression method to use.
> +#                    Defaults to none. (Since 4.1)
> +#
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
> @@ -840,7 +850,8 @@
>              '*multifd-channels': 'uint8',
>              '*xbzrle-cache-size': 'size',
>  	    '*max-postcopy-bandwidth': 'size',
> -            '*max-cpu-throttle':'uint8'} }
> +            '*max-cpu-throttle': 'uint8',
> +            '*multifd-compress': 'MultifdCompress' } }
>  
>  ##
>  # @query-migrate-parameters:
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 65d5e256a7..8a1ccc2516 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -449,7 +449,6 @@ static void migrate_check_parameter_str(QTestState *who, const char *parameter,
>      g_free(result);
>  }
>  
> -__attribute__((unused))
>  static void migrate_set_parameter_str(QTestState *who, const char *parameter,
>                                        const char *value)
>  {
> @@ -1065,7 +1064,7 @@ static void test_precopy_tcp(void)
>      g_free(uri);
>  }
>  
> -static void test_multifd_tcp(void)
> +static void test_multifd_tcp(const char *method)
>  {
>      char *uri;
>      QTestState *from, *to;
> @@ -1087,6 +1086,9 @@ static void test_multifd_tcp(void)
>      migrate_set_parameter_int(from, "multifd-channels", 2);
>      migrate_set_parameter_int(to, "multifd-channels", 2);
>  
> +    migrate_set_parameter_str(from, "multifd-compress", method);
> +    migrate_set_parameter_str(to, "multifd-compress", method);
> +
>      migrate_set_capability(from, "multifd", "true");
>      migrate_set_capability(to, "multifd", "true");
>      /* Wait for the first serial output from the source */
> @@ -1112,6 +1114,11 @@ static void test_multifd_tcp(void)
>      test_migrate_end(from, to, true);
>  }
>  
> +static void test_multifd_tcp_none(void)
> +{
> +    test_multifd_tcp("none");
> +}
> +
>  int main(int argc, char **argv)
>  {
>      char template[] = "/tmp/migration-test-XXXXXX";
> @@ -1166,7 +1173,7 @@ int main(int argc, char **argv)
>      qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
>      /* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
>      qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
> -    qtest_add_func("/migration/multifd/tcp", test_multifd_tcp);
> +    qtest_add_func("/migration/multifd/tcp/none", test_multifd_tcp_none);
>  
>      ret = g_test_run();
>  
> -- 
> 2.20.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH v2 7/8] multifd: Add zlib compression support

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.c  |  9 ++++++++
 migration/migration.h  |  1 +
 migration/ram.c        | 47 ++++++++++++++++++++++++++++++++++++++++++
 qapi/common.json       |  4 +++-
 tests/migration-test.c |  6 ++++++
 5 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index d6f8ef342a..69d85cbe5e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2141,6 +2141,15 @@ bool migrate_use_multifd(void)
     return s->enabled_capabilities[MIGRATION_CAPABILITY_MULTIFD];
 }
 
+bool migrate_use_multifd_zlib(void)
+{
+    MigrationState *s;
+
+    s = migrate_get_current();
+
+    return s->parameters.multifd_compress == MULTIFD_COMPRESS_ZLIB;
+}
+
 bool migrate_pause_before_switchover(void)
 {
     MigrationState *s;
diff --git a/migration/migration.h b/migration/migration.h
index 438f17edad..fc4fb841d4 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -269,6 +269,7 @@ bool migrate_ignore_shared(void);
 
 bool migrate_auto_converge(void);
 bool migrate_use_multifd(void);
+bool migrate_use_multifd_zlib(void);
 bool migrate_pause_before_switchover(void);
 int migrate_multifd_channels(void);
 
diff --git a/migration/ram.c b/migration/ram.c
index d7f8fe45a8..06b25ac66d 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -582,6 +582,7 @@ exit:
 #define MULTIFD_VERSION 1
 
 #define MULTIFD_FLAG_SYNC (1 << 0)
+#define MULTIFD_FLAG_ZLIB (1 << 1)
 
 /* This value needs to be a multiple of qemu_target_page_size() */
 #define MULTIFD_PACKET_SIZE (512 * 1024)
@@ -663,6 +664,12 @@ typedef struct {
     uint64_t num_pages;
     /* syncs main thread and channels */
     QemuSemaphore sem_sync;
+    /* stream for compression */
+    z_stream zs;
+    /* compressed buffer */
+    uint8_t *zbuff;
+    /* size of compressed buffer */
+    uint32_t zbuff_len;
 }  MultiFDSendParams;
 
 typedef struct {
@@ -698,6 +705,12 @@ typedef struct {
     uint64_t num_pages;
     /* syncs main thread and channels */
     QemuSemaphore sem_sync;
+    /* stream for compression */
+    z_stream zs;
+    /* compressed buffer */
+    uint8_t *zbuff;
+    /* size of compressed buffer */
+    uint32_t zbuff_len;
 } MultiFDRecvParams;
 
 static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
@@ -1035,6 +1048,9 @@ void multifd_save_cleanup(void)
         p->packet_len = 0;
         g_free(p->packet);
         p->packet = NULL;
+        deflateEnd(&p->zs);
+        g_free(p->zbuff);
+        p->zbuff = NULL;
     }
     qemu_sem_destroy(&multifd_send_state->channels_ready);
     qemu_sem_destroy(&multifd_send_state->sem_sync);
@@ -1198,6 +1214,7 @@ int multifd_save_setup(void)
 
     for (i = 0; i < thread_count; i++) {
         MultiFDSendParams *p = &multifd_send_state->params[i];
+        z_stream *zs = &p->zs;
 
         qemu_mutex_init(&p->mutex);
         qemu_sem_init(&p->sem, 0);
@@ -1211,6 +1228,17 @@ int multifd_save_setup(void)
         p->packet = g_malloc0(p->packet_len);
         p->name = g_strdup_printf("multifdsend_%d", i);
         socket_send_channel_create(multifd_new_send_channel_async, p);
+        zs->zalloc = Z_NULL;
+        zs->zfree = Z_NULL;
+        zs->opaque = Z_NULL;
+        if (deflateInit(zs, migrate_compress_level()) != Z_OK) {
+            printf("deflate init failed\n");
+            return -1;
+        }
+        /* We will never have more than page_count pages */
+        p->zbuff_len = page_count * qemu_target_page_size();
+        p->zbuff_len *= 2;
+        p->zbuff = g_malloc0(p->zbuff_len);
     }
     return 0;
 }
@@ -1278,6 +1306,9 @@ int multifd_load_cleanup(Error **errp)
         p->packet_len = 0;
         g_free(p->packet);
         p->packet = NULL;
+        inflateEnd(&p->zs);
+        g_free(p->zbuff);
+        p->zbuff = NULL;
     }
     qemu_sem_destroy(&multifd_recv_state->sem_sync);
     g_free(multifd_recv_state->params);
@@ -1396,6 +1427,7 @@ int multifd_load_setup(void)
 
     for (i = 0; i < thread_count; i++) {
         MultiFDRecvParams *p = &multifd_recv_state->params[i];
+        z_stream *zs = &p->zs;
 
         qemu_mutex_init(&p->mutex);
         qemu_sem_init(&p->sem_sync, 0);
@@ -1405,6 +1437,21 @@ int multifd_load_setup(void)
                       + sizeof(ram_addr_t) * page_count;
         p->packet = g_malloc0(p->packet_len);
         p->name = g_strdup_printf("multifdrecv_%d", i);
+
+        zs->zalloc = Z_NULL;
+        zs->zfree = Z_NULL;
+        zs->opaque = Z_NULL;
+        zs->avail_in = 0;
+        zs->next_in = Z_NULL;
+        if (inflateInit(zs) != Z_OK) {
+            printf("inflate init failed\n");
+            return -1;
+        }
+        /* We will never have more than page_count pages */
+        p->zbuff_len = page_count * qemu_target_page_size();
+        /* We know compression "could" use more space */
+        p->zbuff_len *= 2;
+        p->zbuff = g_malloc0(p->zbuff_len);
     }
     return 0;
 }
diff --git a/qapi/common.json b/qapi/common.json
index 7248172792..89df6854cb 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -201,8 +201,10 @@
 #
 # @none: no compression.
 #
+# @zlib: Compress using zlib.
+#
 # Since: 4.1
 #
 ##
 { 'enum': 'MultifdCompress',
-  'data': [ 'none' ] }
+  'data': [ 'none', 'zlib' ] }
diff --git a/tests/migration-test.c b/tests/migration-test.c
index 8a1ccc2516..2dd4d4c5b4 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -1119,6 +1119,11 @@ static void test_multifd_tcp_none(void)
     test_multifd_tcp("none");
 }
 
+static void test_multifd_tcp_zlib(void)
+{
+    test_multifd_tcp("zlib");
+}
+
 int main(int argc, char **argv)
 {
     char template[] = "/tmp/migration-test-XXXXXX";
@@ -1174,6 +1179,7 @@ int main(int argc, char **argv)
     /* qtest_add_func("/migration/ignore_shared", test_ignore_shared); */
     qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
     qtest_add_func("/migration/multifd/tcp/none", test_multifd_tcp_none);
+    qtest_add_func("/migration/multifd/tcp/zlib", test_multifd_tcp_zlib);
 
     ret = g_test_run();
 
-- 
2.20.1

[Qemu-devel] [PATCH v2 8/8] multifd: rest of zlib compression (WIP)

[Juan Quintela]

This is still a work in progress, but get everything sent as expected
and it is faster than the code that is already there.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 88 insertions(+), 5 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 06b25ac66d..1b3b88d711 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1118,7 +1118,41 @@ static void *multifd_send_thread(void *opaque)
             uint64_t packet_num = p->packet_num;
             uint32_t flags = p->flags;
 
-            p->next_packet_size = used * qemu_target_page_size();
+            if (used) {
+                if (migrate_use_multifd_zlib()) {
+                    struct iovec *iov = p->pages->iov;
+                    z_stream *zs = &p->zs;
+                    uint32_t out_size = 0;
+                    int i;
+
+                    for (i = 0; i < used; i++ ) {
+                        uint32_t available = p->zbuff_len - out_size;
+                        int flush = Z_NO_FLUSH;
+
+                        if (i == used  - 1) {
+                            flush = Z_SYNC_FLUSH;
+                        }
+
+                        zs->avail_in = iov[i].iov_len;
+                        zs->next_in = iov[i].iov_base;
+
+                        zs->avail_out = available;
+                        zs->next_out = p->zbuff + out_size;
+
+                        ret = deflate(zs, flush);
+                        if (ret != Z_OK) {
+                            printf("problem with deflate? %d\n", ret);
+                            qemu_mutex_unlock(&p->mutex);
+                            break;
+                        }
+                        out_size += available - zs->avail_out;
+                    }
+                    p->next_packet_size = out_size;
+                } else {
+                    p->next_packet_size = used * qemu_target_page_size();
+                }
+            }
+
             multifd_send_fill_packet(p);
             p->flags = 0;
             p->num_packets++;
@@ -1136,8 +1170,13 @@ static void *multifd_send_thread(void *opaque)
             }
 
             if (used) {
-                ret = qio_channel_writev_all(p->c, p->pages->iov,
-                                             used, &local_err);
+                if (migrate_use_multifd_zlib()) {
+                    ret = qio_channel_write_all(p->c, (void *)p->zbuff,
+                                               p->next_packet_size, &local_err);
+                } else {
+                    ret = qio_channel_writev_all(p->c, p->pages->iov,
+                                                 used, &local_err);
+                }
                 if (ret != 0) {
                     break;
                 }
@@ -1384,8 +1423,52 @@ static void *multifd_recv_thread(void *opaque)
         qemu_mutex_unlock(&p->mutex);
 
         if (used) {
-            ret = qio_channel_readv_all(p->c, p->pages->iov,
-                                        used, &local_err);
+            uint32_t in_size = p->next_packet_size;
+            uint32_t out_size = 0;
+            uint32_t expected_size = used * qemu_target_page_size();
+            int i;
+
+            if (migrate_use_multifd_zlib()) {
+                z_stream *zs = &p->zs;
+
+                ret = qio_channel_read_all(p->c, (void *)p->zbuff,
+                                           in_size, &local_err);
+
+                if (ret != 0) {
+                    break;
+                }
+
+                zs->avail_in = in_size;
+                zs->next_in = p->zbuff;
+
+                for (i = 0; i < used; i++ ) {
+                    struct iovec *iov = &p->pages->iov[i];
+                    int flush = Z_NO_FLUSH;
+
+                    if (i == used  - 1) {
+                        flush = Z_SYNC_FLUSH;
+                    }
+
+                    zs->avail_out = iov->iov_len;
+                    zs->next_out = iov->iov_base;
+
+                    ret = inflate(zs, flush);
+                    if (ret != Z_OK) {
+                        printf("%d: problem with inflate? %d\n", p->id, ret);
+                        qemu_mutex_unlock(&p->mutex);
+                        break;
+                    }
+                    out_size += iov->iov_len;
+                }
+                if (out_size != expected_size) {
+                    printf("out size %d expected size %d\n",
+                           out_size, expected_size);
+                    break;
+                }
+            } else {
+                ret = qio_channel_readv_all(p->c, p->pages->iov,
+                                            used, &local_err);
+            }
             if (ret != 0) {
                 break;
             }
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 0/8] WIP: Multifd compression support

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190403114958.3705-1-quintela@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20190403114958.3705-1-quintela@redhat.com
Subject: [Qemu-devel] [PATCH v2 0/8] WIP: Multifd compression support
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20190403114958.3705-1-quintela@redhat.com -> patchew/20190403114958.3705-1-quintela@redhat.com
Switched to a new branch 'test'
2e0702511c multifd: rest of zlib compression (WIP)
1f67f96a63 multifd: Add zlib compression support
f5c3ad30bc migration: Add multifd-compress parameter
c5c77f97be migration-test: introduce functions to handle string parameters
94ab8bb4f3 tests: Add migration multifd test
d73c9d31bb migration-test: rename parameter to parameter_int
5c0dbda2f7 migration: fix multifd_recv event typo
227dcab34a migration: Fix migrate_set_parameter

=== OUTPUT BEGIN ===
1/8 Checking commit 227dcab34a61 (migration: Fix migrate_set_parameter)
2/8 Checking commit 5c0dbda2f7b4 (migration: fix multifd_recv event typo)
3/8 Checking commit d73c9d31bb62 (migration-test: rename parameter to parameter_int)
4/8 Checking commit 94ab8bb4f3c6 (tests: Add migration multifd test)
5/8 Checking commit c5c77f97be9e (migration-test: introduce functions to handle string parameters)
6/8 Checking commit f5c3ad30bc62 (migration: Add multifd-compress parameter)
WARNING: line over 80 characters
#133: FILE: migration/migration.c:3366:
+    DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_compress, MultifdCompress)

total: 0 errors, 1 warnings, 231 lines checked

Patch 6/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/8 Checking commit 1f67f96a636a (multifd: Add zlib compression support)
8/8 Checking commit 2e0702511c82 (multifd: rest of zlib compression (WIP))
ERROR: space prohibited before that close parenthesis ')'
#29: FILE: migration/ram.c:1128:
+                    for (i = 0; i < used; i++ ) {

ERROR: space prohibited before that close parenthesis ')'
#100: FILE: migration/ram.c:1444:
+                for (i = 0; i < used; i++ ) {

total: 2 errors, 0 warnings, 111 lines checked

Patch 8/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190403114958.3705-1-quintela@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com