[Qemu-devel] [PULL 00/15] Migration

[Qemu-devel] [PULL 00/15] Migration

[Juan Quintela]

From: Juan Quintela <quintela@trasno.org>

The following changes since commit bbc4c3f4f3c624e2de64fdcb79f4dd8c1a508e9d:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2016-10-04 14:25:08 +0100)

are available in the git repository at:

  git://github.com/juanquintela/qemu.git tags/migration/20161005-1

for you to fetch changes up to c62da143b6792cc32dbd5db15b936d7f58cc36ee:

  docs/xbzrle: correction (2016-10-05 14:28:41 +0200)

----------------------------------------------------------------
migration/next for 20161005

Hi

This get:
- documentation fix: Cao
- improve max-bandwidth and downtime-limit (Ashijeet)
- move migration commands to "boxed" (Eric)
- rdma fixes (David)
- postcopy fixes (David)
- better errors (David)

Please apply

Thanks, Juan
----------------------------------------------------------------
Ashijeet Acharya (2):
      migrate: Fix bounds check for migration parameters in migration.c
      migrate: move max-bandwidth and downtime-limit to migrate_set_parameter

Cao jin (1):
      docs/xbzrle: correction

Dr. David Alan Gilbert (9):
      migration: report an error giving the failed field
      migration: Report values for comparisons
      migration/rdma: Pass qemu_file errors across link
      migration: Make failed migration load set file error
      migration/rdma: Don't flag an error when we've been told about one
      Postcopy vs xbzrle: Don't send xbzrle pages once in postcopy [for 2.8]
      RAMBlocks: Store page size
      migration/postcopy: Explicitly disallow huge pages
      migration: Fix seg with missing port

Eric Blake (3):
      migrate: Fix cpu-throttle-increment regression in HMP
      migrate: Share common MigrationParameters struct
      migrate: Use boxed qapi for migrate-set-parameters

 docs/qmp-commands.txt         |  13 +++-
 docs/xbzrle.txt               |   4 +-
 exec.c                        |  19 +++--
 hmp.c                         |  77 ++++++++++++++-------
 include/exec/cpu-common.h     |   1 +
 include/exec/ram_addr.h       |   1 +
 include/migration/migration.h |   1 -
 migration/migration.c         | 157 +++++++++++++++++++++++-------------------
 migration/postcopy-ram.c      |  24 +++++++
 migration/ram.c               |   4 +-
 migration/rdma.c              |   9 ++-
 migration/savevm.c            |  19 +++--
 migration/socket.c            |  16 +++--
 migration/vmstate.c           |  10 +++
 qapi-schema.json              | 109 +++++++++++++----------------
 15 files changed, 282 insertions(+), 182 deletions(-)

[Qemu-devel] [PULL 01/15] migration: report an error giving the failed field

[Juan Quintela]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

When a field fails to load (typically due to a limit
check, or a call to a get/put) report the device and field
to give an indication of the cause.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/vmstate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/migration/vmstate.c b/migration/vmstate.c
index fc29acf..1d637b2 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -130,6 +130,8 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                 }
                 if (ret < 0) {
                     qemu_file_set_error(f, ret);
+                    error_report("Failed to load %s:%s", vmsd->name,
+                                 field->name);
                     trace_vmstate_load_field_error(field->name, ret);
                     return ret;
                 }
-- 
2.7.4

[Qemu-devel] [PULL 02/15] migration: Report values for comparisons

[Juan Quintela]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Report the values when a comparison fails; together with
the previous patch that prints the device and field names
this should give a good idea of why loading the migration failed.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/vmstate.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/migration/vmstate.c b/migration/vmstate.c
index 1d637b2..0bc9f35 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -557,6 +557,7 @@ static int get_int32_equal(QEMUFile *f, void *pv, size_t size)
     if (*v == v2) {
         return 0;
     }
+    error_report("%" PRIx32 " != %" PRIx32, *v, v2);
     return -EINVAL;
 }

@@ -580,6 +581,9 @@ static int get_int32_le(QEMUFile *f, void *pv, size_t size)
         *cur = loaded;
         return 0;
     }
+    error_report("Invalid value %" PRId32
+                 " expecting positive value <= %" PRId32,
+                 loaded, *cur);
     return -EINVAL;
 }

@@ -685,6 +689,7 @@ static int get_uint32_equal(QEMUFile *f, void *pv, size_t size)
     if (*v == v2) {
         return 0;
     }
+    error_report("%" PRIx32 " != %" PRIx32, *v, v2);
     return -EINVAL;
 }

@@ -727,6 +732,7 @@ static int get_uint64_equal(QEMUFile *f, void *pv, size_t size)
     if (*v == v2) {
         return 0;
     }
+    error_report("%" PRIx64 " != %" PRIx64, *v, v2);
     return -EINVAL;
 }

@@ -748,6 +754,7 @@ static int get_uint8_equal(QEMUFile *f, void *pv, size_t size)
     if (*v == v2) {
         return 0;
     }
+    error_report("%x != %x", *v, v2);
     return -EINVAL;
 }

@@ -769,6 +776,7 @@ static int get_uint16_equal(QEMUFile *f, void *pv, size_t size)
     if (*v == v2) {
         return 0;
     }
+    error_report("%x != %x", *v, v2);
     return -EINVAL;
 }

-- 
2.7.4

[Qemu-devel] [PULL 03/15] migration/rdma: Pass qemu_file errors across link

[Juan Quintela]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

If we fail for some reason (e.g. a mismatched RAMBlock)
and it's set the qemu_file error flag, pass that error back to the
peer so it can clean up rather than waiting for some higher level
progress.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Michael R. Hines <michael@hinespot.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/rdma.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/migration/rdma.c b/migration/rdma.c
index 88bdb64..7271292 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2804,6 +2804,9 @@ static int qio_channel_rdma_close(QIOChannel *ioc,
     QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
     trace_qemu_rdma_close();
     if (rioc->rdma) {
+        if (!rioc->rdma->error_state) {
+            rioc->rdma->error_state = qemu_file_get_error(rioc->file);
+        }
         qemu_rdma_cleanup(rioc->rdma);
         g_free(rioc->rdma);
         rioc->rdma = NULL;
-- 
2.7.4

[Qemu-devel] [PULL 04/15] migration: Make failed migration load set file error

[Juan Quintela]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

If an error occurs in a section load, set the file error flag
so that the transport can get notified to do a cleanup.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Michael R. Hines <michael@hinespot.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/savevm.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 33a2911..a831ec2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1828,40 +1828,45 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
 static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
 {
     uint8_t section_type;
-    int ret;
+    int ret = 0;

     while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
-
+        ret = 0;
         trace_qemu_loadvm_state_section(section_type);
         switch (section_type) {
         case QEMU_VM_SECTION_START:
         case QEMU_VM_SECTION_FULL:
             ret = qemu_loadvm_section_start_full(f, mis);
             if (ret < 0) {
-                return ret;
+                goto out;
             }
             break;
         case QEMU_VM_SECTION_PART:
         case QEMU_VM_SECTION_END:
             ret = qemu_loadvm_section_part_end(f, mis);
             if (ret < 0) {
-                return ret;
+                goto out;
             }
             break;
         case QEMU_VM_COMMAND:
             ret = loadvm_process_command(f);
             trace_qemu_loadvm_state_section_command(ret);
             if ((ret < 0) || (ret & LOADVM_QUIT)) {
-                return ret;
+                goto out;
             }
             break;
         default:
             error_report("Unknown savevm section type %d", section_type);
-            return -EINVAL;
+            ret = -EINVAL;
+            goto out;
         }
     }

-    return 0;
+out:
+    if (ret < 0) {
+        qemu_file_set_error(f, ret);
+    }
+    return ret;
 }

 int qemu_loadvm_state(QEMUFile *f)
-- 
2.7.4

[Qemu-devel] [PULL 05/15] migration/rdma: Don't flag an error when we've been told about one

[Juan Quintela]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

If the other side tells us there's been an error and we fail
the migration, we don't need to signal that failure to the other
side because it already knew.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Michael R. Hines <michael@hinespot.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/rdma.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 7271292..674ccab 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -350,6 +350,7 @@ typedef struct RDMAContext {
      */
     int error_state;
     int error_reported;
+    int received_error;

     /*
      * Description of ram blocks used throughout the code.
@@ -1676,6 +1677,9 @@ static int qemu_rdma_exchange_get_response(RDMAContext *rdma,
                 ", but got: %s (%d), length: %d",
                 control_desc[expecting], expecting,
                 control_desc[head->type], head->type, head->len);
+        if (head->type == RDMA_CONTROL_ERROR) {
+            rdma->received_error = true;
+        }
         return -EIO;
     }
     if (head->len > RDMA_CONTROL_MAX_BUFFER - sizeof(*head)) {
@@ -2202,7 +2206,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
     int ret, idx;

     if (rdma->cm_id && rdma->connected) {
-        if (rdma->error_state) {
+        if (rdma->error_state && !rdma->received_error) {
             RDMAControlHeader head = { .len = 0,
                                        .type = RDMA_CONTROL_ERROR,
                                        .repeat = 1,
-- 
2.7.4

[Qemu-devel] [PULL 06/15] migrate: Fix cpu-throttle-increment regression in HMP

[Juan Quintela]

From: Eric Blake <eblake@redhat.com>

Commit 69ef1f3 accidentally broke migrate_set_parameter's ability
to set the cpu-throttle-increment to anything other than the
default, because it forgot to parse the user's string into an
integer.

CC: qemu-stable@nongnu.org
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hmp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hmp.c b/hmp.c
index 336e7bf..9b31a97 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1286,6 +1286,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
                 break;
             case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT:
                 has_cpu_throttle_increment = true;
+                use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_TLS_CREDS:
                 has_tls_creds = true;
-- 
2.7.4

[Qemu-devel] [PULL 07/15] migrate: Share common MigrationParameters struct

[Juan Quintela]

From: Eric Blake <eblake@redhat.com>

It is rather verbose, and slightly error-prone, to repeat
the same set of parameters for input (migrate-set-parameters)
as for output (query-migrate-parameters), where the only
difference is whether the members are optional.  We can just
document that the optional members will always be present
on output, and then share a common struct between both
commands.  The next patch can then reduce the amount of
code needed on input.

Also, we made a mistake in qemu 2.7 of returning an empty
string during 'query-migrate-parameters' when there is no
TLS, rather than omitting TLS details entirely.  Technically,
this change risks breaking any 2.7 client that is hard-coded
to expect the parameter's existence; on the other hand, clients
that are portable to 2.6 already must be prepared for those
members to not be present.

And this gets rid of yet one more place where the QMP output
visitor is silently converting a NULL string into "" (which
is a hack I ultimately want to kill off).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hmp.c                 |   9 +++-
 migration/migration.c |   7 +++
 qapi-schema.json      | 116 +++++++++++++++++++-------------------------------
 3 files changed, 57 insertions(+), 75 deletions(-)

diff --git a/hmp.c b/hmp.c
index 9b31a97..ce741a5 100644
--- a/hmp.c
+++ b/hmp.c
@@ -283,27 +283,32 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)

     if (params) {
         monitor_printf(mon, "parameters:");
+        assert(params->has_compress_level);
         monitor_printf(mon, " %s: %" PRId64,
             MigrationParameter_lookup[MIGRATION_PARAMETER_COMPRESS_LEVEL],
             params->compress_level);
+        assert(params->has_compress_threads);
         monitor_printf(mon, " %s: %" PRId64,
             MigrationParameter_lookup[MIGRATION_PARAMETER_COMPRESS_THREADS],
             params->compress_threads);
+        assert(params->has_decompress_threads);
         monitor_printf(mon, " %s: %" PRId64,
             MigrationParameter_lookup[MIGRATION_PARAMETER_DECOMPRESS_THREADS],
             params->decompress_threads);
+        assert(params->has_cpu_throttle_initial);
         monitor_printf(mon, " %s: %" PRId64,
             MigrationParameter_lookup[MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL],
             params->cpu_throttle_initial);
+        assert(params->has_cpu_throttle_increment);
         monitor_printf(mon, " %s: %" PRId64,
             MigrationParameter_lookup[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT],
             params->cpu_throttle_increment);
         monitor_printf(mon, " %s: '%s'",
             MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_CREDS],
-            params->tls_creds ? : "");
+            params->has_tls_creds ? params->tls_creds : "");
         monitor_printf(mon, " %s: '%s'",
             MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_HOSTNAME],
-            params->tls_hostname ? : "");
+            params->has_tls_hostname ? params->tls_hostname : "");
         monitor_printf(mon, "\n");
     }

diff --git a/migration/migration.c b/migration/migration.c
index 955d5ee..1a8f26b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -559,12 +559,19 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     MigrationState *s = migrate_get_current();

     params = g_malloc0(sizeof(*params));
+    params->has_compress_level = true;
     params->compress_level = s->parameters.compress_level;
+    params->has_compress_threads = true;
     params->compress_threads = s->parameters.compress_threads;
+    params->has_decompress_threads = true;
     params->decompress_threads = s->parameters.decompress_threads;
+    params->has_cpu_throttle_initial = true;
     params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
+    params->has_cpu_throttle_increment = true;
     params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
+    params->has_tls_creds = !!s->parameters.tls_creds;
     params->tls_creds = g_strdup(s->parameters.tls_creds);
+    params->has_tls_hostname = !!s->parameters.tls_hostname;
     params->tls_hostname = g_strdup(s->parameters.tls_hostname);

     return params;
diff --git a/qapi-schema.json b/qapi-schema.json
index c3dcf11..1bc0f13 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -668,40 +668,53 @@
 #
 # @migrate-set-parameters
 #
-# Set the following migration parameters
-#
-# @compress-level: compression level
-#
-# @compress-threads: compression thread count
-#
-# @decompress-threads: decompression thread count
-#
-# @cpu-throttle-initial: Initial percentage of time guest cpus are throttled
-#                        when migration auto-converge is activated. The
-#                        default value is 20. (Since 2.7)
-#
-# @cpu-throttle-increment: throttle percentage increase each time
-#                          auto-converge detects that migration is not making
-#                          progress. The default value is 10. (Since 2.7)
-#
-# @tls-creds: ID of the 'tls-creds' object that provides credentials for
-#             establishing a TLS connection over the migration data channel.
-#             On the outgoing side of the migration, the credentials must
-#             be for a 'client' endpoint, while for the incoming side the
-#             credentials must be for a 'server' endpoint. Setting this
-#             will enable TLS for all migrations. The default is unset,
-#             resulting in unsecured migration at the QEMU level. (Since 2.7)
-#
-# @tls-hostname: hostname of the target host for the migration. This is
-#                required when using x509 based TLS credentials and the
-#                migration URI does not already include a hostname. For
-#                example if using fd: or exec: based migration, the
-#                hostname must be provided so that the server's x509
-#                certificate identity can be validated. (Since 2.7)
+# Set various migration parameters.  See MigrationParameters for details.
 #
 # Since: 2.4
 ##
 { 'command': 'migrate-set-parameters',
+  'data': 'MigrationParameters' }
+
+#
+# @MigrationParameters
+#
+# Optional members can be omitted on input ('migrate-set-parameters')
+# but most members will always be present on output
+# ('query-migrate-parameters'), with the exception of tls-creds and
+# tls-hostname.
+#
+# @compress-level: #optional compression level
+#
+# @compress-threads: #optional compression thread count
+#
+# @decompress-threads: #optional decompression thread count
+#
+# @cpu-throttle-initial: #optional Initial percentage of time guest cpus are
+#                        throttledwhen migration auto-converge is activated.
+#                        The default value is 20. (Since 2.7)
+#
+# @cpu-throttle-increment: #optional throttle percentage increase each time
+#                          auto-converge detects that migration is not making
+#                          progress. The default value is 10. (Since 2.7)
+#
+# @tls-creds: #optional ID of the 'tls-creds' object that provides credentials
+#             for establishing a TLS connection over the migration data
+#             channel. On the outgoing side of the migration, the credentials
+#             must be for a 'client' endpoint, while for the incoming side the
+#             credentials must be for a 'server' endpoint. Setting this
+#             will enable TLS for all migrations. The default is unset,
+#             resulting in unsecured migration at the QEMU level. (Since 2.7)
+#
+# @tls-hostname: #optional hostname of the target host for the migration. This
+#                is required when using x509 based TLS credentials and the
+#                migration URI does not already include a hostname. For
+#                example if using fd: or exec: based migration, the
+#                hostname must be provided so that the server's x509
+#                certificate identity can be validated. (Since 2.7)
+#
+# Since: 2.4
+##
+{ 'struct': 'MigrationParameters',
   'data': { '*compress-level': 'int',
             '*compress-threads': 'int',
             '*decompress-threads': 'int',
@@ -709,49 +722,6 @@
             '*cpu-throttle-increment': 'int',
             '*tls-creds': 'str',
             '*tls-hostname': 'str'} }
-
-#
-# @MigrationParameters
-#
-# @compress-level: compression level
-#
-# @compress-threads: compression thread count
-#
-# @decompress-threads: decompression thread count
-#
-# @cpu-throttle-initial: Initial percentage of time guest cpus are throttled
-#                        when migration auto-converge is activated. The
-#                        default value is 20. (Since 2.7)
-#
-# @cpu-throttle-increment: throttle percentage increase each time
-#                          auto-converge detects that migration is not making
-#                          progress. The default value is 10. (Since 2.7)
-#
-# @tls-creds: ID of the 'tls-creds' object that provides credentials for
-#             establishing a TLS connection over the migration data channel.
-#             On the outgoing side of the migration, the credentials must
-#             be for a 'client' endpoint, while for the incoming side the
-#             credentials must be for a 'server' endpoint. Setting this
-#             will enable TLS for all migrations. The default is unset,
-#             resulting in unsecured migration at the QEMU level. (Since 2.7)
-#
-# @tls-hostname: hostname of the target host for the migration. This is
-#                required when using x509 based TLS credentials and the
-#                migration URI does not already include a hostname. For
-#                example if using fd: or exec: based migration, the
-#                hostname must be provided so that the server's x509
-#                certificate identity can be validated. (Since 2.7)
-#
-# Since: 2.4
-##
-{ 'struct': 'MigrationParameters',
-  'data': { 'compress-level': 'int',
-            'compress-threads': 'int',
-            'decompress-threads': 'int',
-            'cpu-throttle-initial': 'int',
-            'cpu-throttle-increment': 'int',
-            'tls-creds': 'str',
-            'tls-hostname': 'str'} }
 ##
 # @query-migrate-parameters
 #
-- 
2.7.4

[Qemu-devel] [PULL 08/15] migrate: Use boxed qapi for migrate-set-parameters

[Juan Quintela]

From: Eric Blake <eblake@redhat.com>

Now that QAPI makes it easy to pass a struct around, we don't
have to declare as many parameters or local variables.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hmp.c                 | 40 ++++++++++++++-----------------
 migration/migration.c | 65 +++++++++++++++++++++------------------------------
 qapi-schema.json      |  2 +-
 3 files changed, 46 insertions(+), 61 deletions(-)

diff --git a/hmp.c b/hmp.c
index ce741a5..5f52ee2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1260,44 +1260,40 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
     const char *valuestr = qdict_get_str(qdict, "value");
     long valueint = 0;
     Error *err = NULL;
-    bool has_compress_level = false;
-    bool has_compress_threads = false;
-    bool has_decompress_threads = false;
-    bool has_cpu_throttle_initial = false;
-    bool has_cpu_throttle_increment = false;
-    bool has_tls_creds = false;
-    bool has_tls_hostname = false;
     bool use_int_value = false;
     int i;

     for (i = 0; i < MIGRATION_PARAMETER__MAX; i++) {
         if (strcmp(param, MigrationParameter_lookup[i]) == 0) {
+            MigrationParameters p = { 0 };
             switch (i) {
             case MIGRATION_PARAMETER_COMPRESS_LEVEL:
-                has_compress_level = true;
+                p.has_compress_level = true;
                 use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_COMPRESS_THREADS:
-                has_compress_threads = true;
+                p.has_compress_threads = true;
                 use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_DECOMPRESS_THREADS:
-                has_decompress_threads = true;
+                p.has_decompress_threads = true;
                 use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL:
-                has_cpu_throttle_initial = true;
+                p.has_cpu_throttle_initial = true;
                 use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT:
-                has_cpu_throttle_increment = true;
+                p.has_cpu_throttle_increment = true;
                 use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_TLS_CREDS:
-                has_tls_creds = true;
+                p.has_tls_creds = true;
+                p.tls_creds = (char *) valuestr;
                 break;
             case MIGRATION_PARAMETER_TLS_HOSTNAME:
-                has_tls_hostname = true;
+                p.has_tls_hostname = true;
+                p.tls_hostname = (char *) valuestr;
                 break;
             }

@@ -1307,16 +1303,16 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
                                valuestr);
                     goto cleanup;
                 }
+                /* Set all integers; only one has_FOO will be set, and
+                 * the code ignores the remaining values */
+                p.compress_level = valueint;
+                p.compress_threads = valueint;
+                p.decompress_threads = valueint;
+                p.cpu_throttle_initial = valueint;
+                p.cpu_throttle_increment = valueint;
             }

-            qmp_migrate_set_parameters(has_compress_level, valueint,
-                                       has_compress_threads, valueint,
-                                       has_decompress_threads, valueint,
-                                       has_cpu_throttle_initial, valueint,
-                                       has_cpu_throttle_increment, valueint,
-                                       has_tls_creds, valuestr,
-                                       has_tls_hostname, valuestr,
-                                       &err);
+            qmp_migrate_set_parameters(&p, &err);
             break;
         }
     }
diff --git a/migration/migration.c b/migration/migration.c
index 1a8f26b..42336e3 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -766,78 +766,67 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
     }
 }

-void qmp_migrate_set_parameters(bool has_compress_level,
-                                int64_t compress_level,
-                                bool has_compress_threads,
-                                int64_t compress_threads,
-                                bool has_decompress_threads,
-                                int64_t decompress_threads,
-                                bool has_cpu_throttle_initial,
-                                int64_t cpu_throttle_initial,
-                                bool has_cpu_throttle_increment,
-                                int64_t cpu_throttle_increment,
-                                bool has_tls_creds,
-                                const char *tls_creds,
-                                bool has_tls_hostname,
-                                const char *tls_hostname,
-                                Error **errp)
+void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
 {
     MigrationState *s = migrate_get_current();

-    if (has_compress_level && (compress_level < 0 || compress_level > 9)) {
+    if (params->has_compress_level &&
+        (params->compress_level < 0 || params->compress_level > 9)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "compress_level",
                    "is invalid, it should be in the range of 0 to 9");
         return;
     }
-    if (has_compress_threads &&
-            (compress_threads < 1 || compress_threads > 255)) {
+    if (params->has_compress_threads &&
+        (params->compress_threads < 1 || params->compress_threads > 255)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                    "compress_threads",
                    "is invalid, it should be in the range of 1 to 255");
         return;
     }
-    if (has_decompress_threads &&
-            (decompress_threads < 1 || decompress_threads > 255)) {
+    if (params->has_decompress_threads &&
+        (params->decompress_threads < 1 || params->decompress_threads > 255)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                    "decompress_threads",
                    "is invalid, it should be in the range of 1 to 255");
         return;
     }
-    if (has_cpu_throttle_initial &&
-            (cpu_throttle_initial < 1 || cpu_throttle_initial > 99)) {
+    if (params->has_cpu_throttle_initial &&
+        (params->cpu_throttle_initial < 1 ||
+         params->cpu_throttle_initial > 99)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                    "cpu_throttle_initial",
                    "an integer in the range of 1 to 99");
     }
-    if (has_cpu_throttle_increment &&
-            (cpu_throttle_increment < 1 || cpu_throttle_increment > 99)) {
+    if (params->has_cpu_throttle_increment &&
+        (params->cpu_throttle_increment < 1 ||
+         params->cpu_throttle_increment > 99)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                    "cpu_throttle_increment",
                    "an integer in the range of 1 to 99");
     }

-    if (has_compress_level) {
-        s->parameters.compress_level = compress_level;
+    if (params->has_compress_level) {
+        s->parameters.compress_level = params->compress_level;
     }
-    if (has_compress_threads) {
-        s->parameters.compress_threads = compress_threads;
+    if (params->has_compress_threads) {
+        s->parameters.compress_threads = params->compress_threads;
     }
-    if (has_decompress_threads) {
-        s->parameters.decompress_threads = decompress_threads;
+    if (params->has_decompress_threads) {
+        s->parameters.decompress_threads = params->decompress_threads;
     }
-    if (has_cpu_throttle_initial) {
-        s->parameters.cpu_throttle_initial = cpu_throttle_initial;
+    if (params->has_cpu_throttle_initial) {
+        s->parameters.cpu_throttle_initial = params->cpu_throttle_initial;
     }
-    if (has_cpu_throttle_increment) {
-        s->parameters.cpu_throttle_increment = cpu_throttle_increment;
+    if (params->has_cpu_throttle_increment) {
+        s->parameters.cpu_throttle_increment = params->cpu_throttle_increment;
     }
-    if (has_tls_creds) {
+    if (params->has_tls_creds) {
         g_free(s->parameters.tls_creds);
-        s->parameters.tls_creds = g_strdup(tls_creds);
+        s->parameters.tls_creds = g_strdup(params->tls_creds);
     }
-    if (has_tls_hostname) {
+    if (params->has_tls_hostname) {
         g_free(s->parameters.tls_hostname);
-        s->parameters.tls_hostname = g_strdup(tls_hostname);
+        s->parameters.tls_hostname = g_strdup(params->tls_hostname);
     }
 }

diff --git a/qapi-schema.json b/qapi-schema.json
index 1bc0f13..49bdec0 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -672,7 +672,7 @@
 #
 # Since: 2.4
 ##
-{ 'command': 'migrate-set-parameters',
+{ 'command': 'migrate-set-parameters', 'boxed': true,
   'data': 'MigrationParameters' }

 #
-- 
2.7.4

[Qemu-devel] [PULL 09/15] migrate: Fix bounds check for migration parameters in migration.c

[Juan Quintela]

From: Ashijeet Acharya <ashijeetacharya@gmail.com>

This patch fixes the out-of-bounds check of migration parameters in
qmp_migrate_set_parameters() for cpu-throttle-initial and
cpu-throttle-increment by adding a return statement for both as they
were broken since their introduction in 2.5 via commit 1626fee.
Due to the missing return statements, parameters were getting set to
out-of-bounds values despite the error.

Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index 42336e3..04b706f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -796,6 +796,7 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                    "cpu_throttle_initial",
                    "an integer in the range of 1 to 99");
+        return;
     }
     if (params->has_cpu_throttle_increment &&
         (params->cpu_throttle_increment < 1 ||
@@ -803,6 +804,7 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                    "cpu_throttle_increment",
                    "an integer in the range of 1 to 99");
+        return;
     }

     if (params->has_compress_level) {
-- 
2.7.4

[Qemu-devel] [PULL 10/15] Postcopy vs xbzrle: Don't send xbzrle pages once in postcopy [for 2.8]

[Juan Quintela]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

xbzrle relies on reading pages that have already been sent
to the destination and then applying the modifications; we can't
do that in postcopy because the destination may well have
modified the page already or the page has been discarded.

I already didn't allow reception of xbzrle pages, but I
forgot to add the test to stop them being sent.

Enabling both xbzrle and postcopy can make some sense;
if you think that your migration might finish if you
have xbzrle, then when it doesn't complete you flick
over to postcopy and stop xbzrle'ing.

This corresponds to RH bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1368422

Symptom is:

Unknown combination of migration flags: 0x60 (postcopy mode)
(either 0x60 or 0x40)

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

diff --git a/migration/ram.c b/migration/ram.c
index c8ec9f2..bc6154f 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -771,7 +771,9 @@ static int ram_save_page(QEMUFile *f, PageSearchStatus *pss,
              * page would be stale
              */
             xbzrle_cache_zero_page(current_addr);
-        } else if (!ram_bulk_stage && migrate_use_xbzrle()) {
+        } else if (!ram_bulk_stage &&
+                   !migration_in_postcopy(migrate_get_current()) &&
+                   migrate_use_xbzrle()) {
             pages = save_xbzrle_page(f, &p, current_addr, block,
                                      offset, last_stage, bytes_transferred);
             if (!last_stage) {
-- 
2.7.4

[Qemu-devel] [PULL 11/15] RAMBlocks: Store page size

[Juan Quintela]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Store the page size in each RAMBlock, we need it later.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 exec.c                    | 19 ++++++++++++-------
 include/exec/cpu-common.h |  1 +
 include/exec/ram_addr.h   |  1 +
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/exec.c b/exec.c
index c8389f9..9c457bc 100644
--- a/exec.c
+++ b/exec.c
@@ -1201,7 +1201,6 @@ static void *file_ram_alloc(RAMBlock *block,
     char *c;
     void *area = MAP_FAILED;
     int fd = -1;
-    int64_t page_size;

     if (kvm_enabled() && !kvm_has_sync_mmu()) {
         error_setg(errp,
@@ -1256,17 +1255,17 @@ static void *file_ram_alloc(RAMBlock *block,
          */
     }

-    page_size = qemu_fd_getpagesize(fd);
-    block->mr->align = MAX(page_size, QEMU_VMALLOC_ALIGN);
+    block->page_size = qemu_fd_getpagesize(fd);
+    block->mr->align = MAX(block->page_size, QEMU_VMALLOC_ALIGN);

-    if (memory < page_size) {
+    if (memory < block->page_size) {
         error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
-                   "or larger than page size 0x%" PRIx64,
-                   memory, page_size);
+                   "or larger than page size 0x%zx",
+                   memory, block->page_size);
         goto error;
     }

-    memory = ROUND_UP(memory, page_size);
+    memory = ROUND_UP(memory, block->page_size);

     /*
      * ftruncate is not supported by hugetlbfs in older
@@ -1421,6 +1420,11 @@ void qemu_ram_unset_idstr(RAMBlock *block)
     }
 }

+size_t qemu_ram_pagesize(RAMBlock *rb)
+{
+    return rb->page_size;
+}
+
 static int memory_try_enable_merging(void *addr, size_t len)
 {
     if (!machine_mem_merge(current_machine)) {
@@ -1660,6 +1664,7 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
     new_block->max_length = max_size;
     assert(max_size >= size);
     new_block->fd = -1;
+    new_block->page_size = getpagesize();
     new_block->host = host;
     if (host) {
         new_block->flags |= RAM_PREALLOC;
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 869ba41..cffdc13 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -63,6 +63,7 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
 void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev);
 void qemu_ram_unset_idstr(RAMBlock *block);
 const char *qemu_ram_get_idstr(RAMBlock *rb);
+size_t qemu_ram_pagesize(RAMBlock *block);

 void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
                             int len, int is_write);
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 2a9465d..54d7108 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -36,6 +36,7 @@ struct RAMBlock {
     /* RCU-enabled, writes protected by the ramlist lock */
     QLIST_ENTRY(RAMBlock) next;
     int fd;
+    size_t page_size;
 };

 static inline bool offset_in_ramblock(RAMBlock *b, ram_addr_t offset)
-- 
2.7.4

[Qemu-devel] [PULL 12/15] migration/postcopy: Explicitly disallow huge pages

[Juan Quintela]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

At the moment postcopy will fail as soon as qemu tries to register
userfault on the RAMBlock pages that are backed by hugepages.
However, the kernel is going to get userfault support for hugepage
at some point, and we've not got the rest of the QEMU code to support
it yet, so fail neatly with an error like:

Postcopy doesn't support hugetlbfs yet (/objects/mem1)

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/postcopy-ram.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 9b04778..a40dddb 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -85,6 +85,24 @@ static bool ufd_version_check(int ufd)
 }

 /*
+ * Check for things that postcopy won't support; returns 0 if the block
+ * is fine.
+ */
+static int check_range(const char *block_name, void *host_addr,
+                      ram_addr_t offset, ram_addr_t length, void *opaque)
+{
+    RAMBlock *rb = qemu_ram_block_by_name(block_name);
+
+    if (qemu_ram_pagesize(rb) > getpagesize()) {
+        error_report("Postcopy doesn't support large page sizes yet (%s)",
+                     block_name);
+        return -E2BIG;
+    }
+
+    return 0;
+}
+
+/*
  * Note: This has the side effect of munlock'ing all of RAM, that's
  * normally fine since if the postcopy succeeds it gets turned back on at the
  * end.
@@ -104,6 +122,12 @@ bool postcopy_ram_supported_by_host(void)
         goto out;
     }

+    /* Check for anything about the RAMBlocks we don't support */
+    if (qemu_ram_foreach_block(check_range, NULL)) {
+        /* check_range will have printed its own error */
+        goto out;
+    }
+
     ufd = syscall(__NR_userfaultfd, O_CLOEXEC);
     if (ufd == -1) {
         error_report("%s: userfaultfd not available: %s", __func__,
-- 
2.7.4

[Qemu-devel] [PULL 13/15] migration: Fix seg with missing port

[Juan Quintela]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The command :
   migrate tcp:localhost:

   currently segs; fix it so it now says:

   error parsing address 'localhost:'

and the same for -incoming.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/socket.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/migration/socket.c b/migration/socket.c
index 00de1fe..a21c0c5 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -112,8 +112,12 @@ void tcp_start_outgoing_migration(MigrationState *s,
                                   const char *host_port,
                                   Error **errp)
 {
-    SocketAddress *saddr = tcp_build_address(host_port, errp);
-    socket_start_outgoing_migration(s, saddr, errp);
+    Error *err = NULL;
+    SocketAddress *saddr = tcp_build_address(host_port, &err);
+    if (!err) {
+        socket_start_outgoing_migration(s, saddr, &err);
+    }
+    error_propagate(errp, err);
 }

 void unix_start_outgoing_migration(MigrationState *s,
@@ -174,8 +178,12 @@ static void socket_start_incoming_migration(SocketAddress *saddr,

 void tcp_start_incoming_migration(const char *host_port, Error **errp)
 {
-    SocketAddress *saddr = tcp_build_address(host_port, errp);
-    socket_start_incoming_migration(saddr, errp);
+    Error *err = NULL;
+    SocketAddress *saddr = tcp_build_address(host_port, &err);
+    if (!err) {
+        socket_start_incoming_migration(saddr, &err);
+    }
+    error_propagate(errp, err);
 }

 void unix_start_incoming_migration(const char *path, Error **errp)
-- 
2.7.4

[Qemu-devel] [PULL 14/15] migrate: move max-bandwidth and downtime-limit to migrate_set_parameter

[Juan Quintela]

From: Ashijeet Acharya <ashijeetacharya@gmail.com>

Mark the old commands 'migrate_set_speed' and 'migrate_set_downtime' as
deprecated.
Move max-bandwidth and downtime-limit into migrate-set-parameters for
setting maximum migration speed and expected downtime limit parameters
respectively.
Change downtime units to milliseconds (only for new-command) and set
its upper bound limit to 2000 seconds.
Update the query part in both hmp and qmp qemu control interfaces.

Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 docs/qmp-commands.txt         | 13 +++++--
 hmp.c                         | 27 ++++++++++++++
 include/migration/migration.h |  1 -
 migration/migration.c         | 83 ++++++++++++++++++++++++++-----------------
 qapi-schema.json              | 23 ++++++++++--
 5 files changed, 107 insertions(+), 40 deletions(-)

diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index e0adceb..4f4c156 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -2910,7 +2910,9 @@ Set migration parameters
                           throttled for auto-converge (json-int)
 - "cpu-throttle-increment": set throttle increasing percentage for
                             auto-converge (json-int)
-
+- "max-bandwidth": set maximum speed for migrations (in bytes/sec) (json-int)
+- "downtime-limit": set maximum tolerated downtime (in milliseconds) for
+                    migrations (json-int)
 Arguments:

 Example:
@@ -2931,7 +2933,10 @@ Query current migration parameters
                                     throttled (json-int)
          - "cpu-throttle-increment" : throttle increasing percentage for
                                       auto-converge (json-int)
-
+         - "max-bandwidth" : maximium migration speed in bytes per second
+                             (json-int)
+         - "downtime-limit" : maximum tolerated downtime of migration in
+                              milliseconds (json-int)
 Arguments:

 Example:
@@ -2943,7 +2948,9 @@ Example:
          "cpu-throttle-increment": 10,
          "compress-threads": 8,
          "compress-level": 1,
-         "cpu-throttle-initial": 20
+         "cpu-throttle-initial": 20,
+         "max-bandwidth": 33554432,
+         "downtime-limit": 300
       }
    }

diff --git a/hmp.c b/hmp.c
index 5f52ee2..147f439 100644
--- a/hmp.c
+++ b/hmp.c
@@ -309,6 +309,14 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, " %s: '%s'",
             MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_HOSTNAME],
             params->has_tls_hostname ? params->tls_hostname : "");
+        assert(params->has_max_bandwidth);
+        monitor_printf(mon, " %s: %" PRId64 " bytes/second",
+            MigrationParameter_lookup[MIGRATION_PARAMETER_MAX_BANDWIDTH],
+            params->max_bandwidth);
+        assert(params->has_downtime_limit);
+        monitor_printf(mon, " %s: %" PRId64 " milliseconds",
+            MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIMIT],
+            params->downtime_limit);
         monitor_printf(mon, "\n");
     }

@@ -1200,6 +1208,7 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, &err);
 }

+/* Kept for backwards compatibility */
 void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict)
 {
     double value = qdict_get_double(qdict, "value");
@@ -1218,6 +1227,7 @@ void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict)
     }
 }

+/* Kept for backwards compatibility */
 void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict)
 {
     int64_t value = qdict_get_int(qdict, "value");
@@ -1258,7 +1268,9 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
 {
     const char *param = qdict_get_str(qdict, "parameter");
     const char *valuestr = qdict_get_str(qdict, "value");
+    int64_t valuebw = 0;
     long valueint = 0;
+    char *endp;
     Error *err = NULL;
     bool use_int_value = false;
     int i;
@@ -1295,6 +1307,20 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
                 p.has_tls_hostname = true;
                 p.tls_hostname = (char *) valuestr;
                 break;
+            case MIGRATION_PARAMETER_MAX_BANDWIDTH:
+                p.has_max_bandwidth = true;
+                valuebw = qemu_strtosz(valuestr, &endp);
+                if (valuebw < 0 || (size_t)valuebw != valuebw
+                    || *endp != '\0') {
+                    error_setg(&err, "Invalid size %s", valuestr);
+                    goto cleanup;
+                }
+                p.max_bandwidth = valuebw;
+                break;
+            case MIGRATION_PARAMETER_DOWNTIME_LIMIT:
+                p.has_downtime_limit = true;
+                use_int_value = true;
+                break;
             }

             if (use_int_value) {
@@ -1310,6 +1336,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
                 p.decompress_threads = valueint;
                 p.cpu_throttle_initial = valueint;
                 p.cpu_throttle_increment = valueint;
+                p.downtime_limit = valueint;
             }

             qmp_migrate_set_parameters(&p, &err);
diff --git a/include/migration/migration.h b/include/migration/migration.h
index d4acc72..2791b90 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -129,7 +129,6 @@ struct MigrationSrcPageRequest {

 struct MigrationState
 {
-    int64_t bandwidth_limit;
     size_t bytes_xfer;
     size_t xfer_limit;
     QemuThread thread;
diff --git a/migration/migration.c b/migration/migration.c
index 04b706f..4d417b7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -44,6 +44,10 @@
 #define BUFFER_DELAY     100
 #define XFER_LIMIT_RATIO (1000 / BUFFER_DELAY)

+/* Time in milliseconds we are allowed to stop the source,
+ * for sending the last part */
+#define DEFAULT_MIGRATE_SET_DOWNTIME 300
+
 /* Default compression thread count */
 #define DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT 8
 /* Default decompression thread count, usually decompression is at
@@ -80,7 +84,6 @@ MigrationState *migrate_get_current(void)
     static bool once;
     static MigrationState current_migration = {
         .state = MIGRATION_STATUS_NONE,
-        .bandwidth_limit = MAX_THROTTLE,
         .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE,
         .mbps = -1,
         .parameters = {
@@ -89,6 +92,8 @@ MigrationState *migrate_get_current(void)
             .decompress_threads = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,
             .cpu_throttle_initial = DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL,
             .cpu_throttle_increment = DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT,
+            .max_bandwidth = MAX_THROTTLE,
+            .downtime_limit = DEFAULT_MIGRATE_SET_DOWNTIME,
         },
     };

@@ -517,17 +522,6 @@ void migrate_send_rp_pong(MigrationIncomingState *mis,
     migrate_send_rp_message(mis, MIG_RP_MSG_PONG, sizeof(buf), &buf);
 }

-/* amount of nanoseconds we are willing to wait for migration to be down.
- * the choice of nanoseconds is because it is the maximum resolution that
- * get_clock() can achieve. It is an internal measure. All user-visible
- * units must be in seconds */
-static uint64_t max_downtime = 300000000;
-
-uint64_t migrate_max_downtime(void)
-{
-    return max_downtime;
-}
-
 MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
 {
     MigrationCapabilityStatusList *head = NULL;
@@ -573,6 +567,10 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     params->tls_creds = g_strdup(s->parameters.tls_creds);
     params->has_tls_hostname = !!s->parameters.tls_hostname;
     params->tls_hostname = g_strdup(s->parameters.tls_hostname);
+    params->has_max_bandwidth = true;
+    params->max_bandwidth = s->parameters.max_bandwidth;
+    params->has_downtime_limit = true;
+    params->downtime_limit = s->parameters.downtime_limit;

     return params;
 }
@@ -806,6 +804,19 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
                    "an integer in the range of 1 to 99");
         return;
     }
+    if (params->has_max_bandwidth &&
+        (params->max_bandwidth < 0 || params->max_bandwidth > SIZE_MAX)) {
+        error_setg(errp, "Parameter 'max_bandwidth' expects an integer in the"
+                         " range of 0 to %zu bytes/second", SIZE_MAX);
+        return;
+    }
+    if (params->has_downtime_limit &&
+        (params->downtime_limit < 0 || params->downtime_limit > 2000000)) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   "downtime_limit",
+                   "an integer in the range of 0 to 2000000 milliseconds");
+        return;
+    }

     if (params->has_compress_level) {
         s->parameters.compress_level = params->compress_level;
@@ -830,6 +841,16 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
         g_free(s->parameters.tls_hostname);
         s->parameters.tls_hostname = g_strdup(params->tls_hostname);
     }
+    if (params->has_max_bandwidth) {
+        s->parameters.max_bandwidth = params->max_bandwidth;
+        if (s->to_dst_file) {
+            qemu_file_set_rate_limit(s->to_dst_file,
+                                s->parameters.max_bandwidth / XFER_LIMIT_RATIO);
+        }
+    }
+    if (params->has_downtime_limit) {
+        s->parameters.downtime_limit = params->downtime_limit;
+    }
 }


@@ -1163,28 +1184,25 @@ int64_t qmp_query_migrate_cache_size(Error **errp)

 void qmp_migrate_set_speed(int64_t value, Error **errp)
 {
-    MigrationState *s;
+    MigrationParameters p = {
+        .has_max_bandwidth = true,
+        .max_bandwidth = value,
+    };

-    if (value < 0) {
-        value = 0;
-    }
-    if (value > SIZE_MAX) {
-        value = SIZE_MAX;
-    }
-
-    s = migrate_get_current();
-    s->bandwidth_limit = value;
-    if (s->to_dst_file) {
-        qemu_file_set_rate_limit(s->to_dst_file,
-                                 s->bandwidth_limit / XFER_LIMIT_RATIO);
-    }
+    qmp_migrate_set_parameters(&p, errp);
 }

 void qmp_migrate_set_downtime(double value, Error **errp)
 {
-    value *= 1e9;
-    value = MAX(0, MIN(UINT64_MAX, value));
-    max_downtime = (uint64_t)value;
+    value *= 1000; /* Convert to milliseconds */
+    value = MAX(0, MIN(INT64_MAX, value));
+
+    MigrationParameters p = {
+        .has_downtime_limit = true,
+        .downtime_limit = value,
+    };
+
+    qmp_migrate_set_parameters(&p, errp);
 }

 bool migrate_postcopy_ram(void)
@@ -1791,7 +1809,7 @@ static void *migration_thread(void *opaque)
                                          initial_bytes;
             uint64_t time_spent = current_time - initial_time;
             double bandwidth = (double)transferred_bytes / time_spent;
-            max_size = bandwidth * migrate_max_downtime() / 1000000;
+            max_size = bandwidth * s->parameters.downtime_limit;

             s->mbps = (((double) transferred_bytes * 8.0) /
                     ((double) time_spent / 1000.0)) / 1000.0 / 1000.0;
@@ -1850,13 +1868,12 @@ static void *migration_thread(void *opaque)

 void migrate_fd_connect(MigrationState *s)
 {
-    /* This is a best 1st approximation. ns to ms */
-    s->expected_downtime = max_downtime/1000000;
+    s->expected_downtime = s->parameters.downtime_limit;
     s->cleanup_bh = qemu_bh_new(migrate_fd_cleanup, s);

     qemu_file_set_blocking(s->to_dst_file, true);
     qemu_file_set_rate_limit(s->to_dst_file,
-                             s->bandwidth_limit / XFER_LIMIT_RATIO);
+                             s->parameters.max_bandwidth / XFER_LIMIT_RATIO);

     /* Notify before starting migration thread */
     notifier_list_notify(&migration_state_notifiers, s);
diff --git a/qapi-schema.json b/qapi-schema.json
index 49bdec0..8d473db 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -658,12 +658,19 @@
 #                hostname must be provided so that the server's x509
 #                certificate identity can be validated. (Since 2.7)
 #
+# @max-bandwidth: to set maximum speed for migration. maximum speed in
+#                 bytes per second. (Since 2.8)
+#
+# @downtime-limit: set maximum tolerated downtime for migration. maximum
+#                  downtime in milliseconds (Since 2.8)
+#
 # Since: 2.4
 ##
 { 'enum': 'MigrationParameter',
   'data': ['compress-level', 'compress-threads', 'decompress-threads',
            'cpu-throttle-initial', 'cpu-throttle-increment',
-           'tls-creds', 'tls-hostname'] }
+           'tls-creds', 'tls-hostname', 'max-bandwidth',
+           'downtime-limit'] }

 #
 # @migrate-set-parameters
@@ -712,6 +719,12 @@
 #                hostname must be provided so that the server's x509
 #                certificate identity can be validated. (Since 2.7)
 #
+# @max-bandwidth: to set maximum speed for migration. maximum speed in
+#                 bytes per second. (Since 2.8)
+#
+# @downtime-limit: set maximum tolerated downtime for migration. maximum
+#                  downtime in milliseconds (Since 2.8)
+#
 # Since: 2.4
 ##
 { 'struct': 'MigrationParameters',
@@ -721,7 +734,9 @@
             '*cpu-throttle-initial': 'int',
             '*cpu-throttle-increment': 'int',
             '*tls-creds': 'str',
-            '*tls-hostname': 'str'} }
+            '*tls-hostname': 'str',
+            '*max-bandwidth': 'int',
+            '*downtime-limit': 'int'} }
 ##
 # @query-migrate-parameters
 #
@@ -1803,6 +1818,8 @@
 #
 # Returns: nothing on success
 #
+# Notes: This command is deprecated in favor of 'migrate-set-parameters'
+#
 # Since: 0.14.0
 ##
 { 'command': 'migrate_set_downtime', 'data': {'value': 'number'} }
@@ -1816,7 +1833,7 @@
 #
 # Returns: nothing on success
 #
-# Notes: A value lesser than zero will be automatically round up to zero.
+# Notes: This command is deprecated in favor of 'migrate-set-parameters'
 #
 # Since: 0.14.0
 ##
-- 
2.7.4

[Qemu-devel] [PULL 15/15] docs/xbzrle: correction

[Juan Quintela]

From: Cao jin <caoj.fnst@cn.fujitsu.com>

1. Default cache size is 64MB.
2. Semantics correction.

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 docs/xbzrle.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/xbzrle.txt b/docs/xbzrle.txt
index 52c8511..c0a7dfd 100644
--- a/docs/xbzrle.txt
+++ b/docs/xbzrle.txt
@@ -42,7 +42,7 @@ nzrun = length byte...
 length = uleb128 encoded integer

 On the sender side XBZRLE is used as a compact delta encoding of page updates,
-retrieving the old page content from the cache (default size of 512 MB). The
+retrieving the old page content from the cache (default size of 64MB). The
 receiving side uses the existing page's content and XBZRLE to decode the new
 page's content.

@@ -73,7 +73,7 @@ e9 07 0f 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 03 01 67 01 01 69

 Cache update strategy
 =====================
-Keeping the hot pages in the cache is effective for decreased cache
+Keeping the hot pages in the cache is effective for decreasing cache
 misses. XBZRLE uses a counter as the age of each page. The counter will
 increase after each ram dirty bitmap sync. When a cache conflict is
 detected, XBZRLE will only evict pages in the cache that are older than
-- 
2.7.4

Re: [Qemu-devel] [PULL 00/15] Migration

[Peter Maydell]

On 5 October 2016 at 14:44, Juan Quintela <quintela@redhat.com> wrote:
> From: Juan Quintela <quintela@trasno.org>
>
> The following changes since commit bbc4c3f4f3c624e2de64fdcb79f4dd8c1a508e9d:
>
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2016-10-04 14:25:08 +0100)
>
> are available in the git repository at:
>
>   git://github.com/juanquintela/qemu.git tags/migration/20161005-1
>
> for you to fetch changes up to c62da143b6792cc32dbd5db15b936d7f58cc36ee:
>
>   docs/xbzrle: correction (2016-10-05 14:28:41 +0200)
>
> ----------------------------------------------------------------
> migration/next for 20161005
>
> Hi
>
> This get:
> - documentation fix: Cao
> - improve max-bandwidth and downtime-limit (Ashijeet)
> - move migration commands to "boxed" (Eric)
> - rdma fixes (David)
> - postcopy fixes (David)
> - better errors (David)
>
> Please apply

This runs into the "OSX %zu vs SIZE_MAX" issue again:

/Users/pm215/src/qemu-for-merges/migration/migration.c:810:61:
warning: format specifies type 'size_t' (aka 'unsigned long') but the
argument has type 'unsigned long long' [-Wformat]
                         " range of 0 to %zu bytes/second", SIZE_MAX);
                                         ~~~                ^~~~~~~~
                                         %llu
/Users/pm215/src/qemu-for-merges/include/qapi/error.h:163:35: note:
expanded from macro 'error_setg'
                        (fmt), ## __VA_ARGS__)
                                  ^
/usr/include/stdint.h:153:20: note: expanded from macro 'SIZE_MAX'
#define SIZE_MAX          UINT64_MAX
                          ^~~~~~~~~~
/usr/include/stdint.h:87:27: note: expanded from macro 'UINT64_MAX'
#define UINT64_MAX        18446744073709551615ULL
                          ^~~~~~~~~~~~~~~~~~~~~~~

In the absence of a suitable configure test and override of SIZE_MAX,
we've been adding explicit (uint64_t) casts in the printf as the
simplest workaround.

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/15] Migration

[Juan Quintela]

Peter Maydell <peter.maydell@linaro.org> wrote:
> On 5 October 2016 at 14:44, Juan Quintela <quintela@redhat.com> wrote:
>> From: Juan Quintela <quintela@trasno.org>
>>
>> The following changes since commit bbc4c3f4f3c624e2de64fdcb79f4dd8c1a508e9d:
>>
>>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream'
>> into staging (2016-10-04 14:25:08 +0100)
>>
>> are available in the git repository at:
>>
>>   git://github.com/juanquintela/qemu.git tags/migration/20161005-1
>>
>> for you to fetch changes up to c62da143b6792cc32dbd5db15b936d7f58cc36ee:
>>
>>   docs/xbzrle: correction (2016-10-05 14:28:41 +0200)
>>
>> ----------------------------------------------------------------
>> migration/next for 20161005
>>
>> Hi
>>
>> This get:
>> - documentation fix: Cao
>> - improve max-bandwidth and downtime-limit (Ashijeet)
>> - move migration commands to "boxed" (Eric)
>> - rdma fixes (David)
>> - postcopy fixes (David)
>> - better errors (David)
>>
>> Please apply
>
> This runs into the "OSX %zu vs SIZE_MAX" issue again:

Grrrr.
I already have trouble with %PR vs %zu for i386 vs anything else.

Anyone has a linux crosscompiler for osx that I can use?  Or there is a
way to get darwin or whatever virtual machine to test compile this?

It gets frustrating, before submissing I do:
- make check
- avocado virt test
- full compile on linux x86_64 (64 bits)
- full compile on linux i386 (32 bits)
- full cross-compile for windows 64bit
- full cross-compile for windows 32bit

(ok, I only test linux 64 bit), and I still don't get all the errors.

> /Users/pm215/src/qemu-for-merges/migration/migration.c:810:61:
> warning: format specifies type 'size_t' (aka 'unsigned long') but the
> argument has type 'unsigned long long' [-Wformat]
>                          " range of 0 to %zu bytes/second", SIZE_MAX);
>                                          ~~~                ^~~~~~~~
>                                          %llu
> /Users/pm215/src/qemu-for-merges/include/qapi/error.h:163:35: note:
> expanded from macro 'error_setg'
>                         (fmt), ## __VA_ARGS__)
>                                   ^
> /usr/include/stdint.h:153:20: note: expanded from macro 'SIZE_MAX'
> #define SIZE_MAX          UINT64_MAX
>                           ^~~~~~~~~~
> /usr/include/stdint.h:87:27: note: expanded from macro 'UINT64_MAX'
> #define UINT64_MAX        18446744073709551615ULL
>                           ^~~~~~~~~~~~~~~~~~~~~~~
>
> In the absence of a suitable configure test and override of SIZE_MAX,
> we've been adding explicit (uint64_t) casts in the printf as the
> simplest workaround.

Will do that, but as I have no way to compile test this, it will
continue hitting me :-(

Thanks for telling me.

Later, Juan.

PD.  Sorry if it sounded like a rant, just frustration O:-)

Re: [Qemu-devel] [PULL 00/15] Migration

[Peter Maydell]

On 6 October 2016 at 17:09, Juan Quintela <quintela@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 5 October 2016 at 14:44, Juan Quintela <quintela@redhat.com> wrote:
>>> From: Juan Quintela <quintela@trasno.org>
>>>
>>> The following changes since commit bbc4c3f4f3c624e2de64fdcb79f4dd8c1a508e9d:
>>>
>>>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream'
>>> into staging (2016-10-04 14:25:08 +0100)
>>>
>>> are available in the git repository at:
>>>
>>>   git://github.com/juanquintela/qemu.git tags/migration/20161005-1
>>>
>>> for you to fetch changes up to c62da143b6792cc32dbd5db15b936d7f58cc36ee:
>>>
>>>   docs/xbzrle: correction (2016-10-05 14:28:41 +0200)
>>>
>>> ----------------------------------------------------------------
>>> migration/next for 20161005
>>>
>>> Hi
>>>
>>> This get:
>>> - documentation fix: Cao
>>> - improve max-bandwidth and downtime-limit (Ashijeet)
>>> - move migration commands to "boxed" (Eric)
>>> - rdma fixes (David)
>>> - postcopy fixes (David)
>>> - better errors (David)
>>>
>>> Please apply
>>
>> This runs into the "OSX %zu vs SIZE_MAX" issue again:
>
> Grrrr.
> I already have trouble with %PR vs %zu for i386 vs anything else.
>
> Anyone has a linux crosscompiler for osx that I can use?  Or there is a
> way to get darwin or whatever virtual machine to test compile this?

Travis builds include OSX, if you're prepared to wait around
for them to complete before submitting (they're usually about 10
hours wall-clock time to complete a build).

> It gets frustrating, before submissing I do:
> - make check
> - avocado virt test
> - full compile on linux x86_64 (64 bits)
> - full compile on linux i386 (32 bits)
> - full cross-compile for windows 64bit
> - full cross-compile for windows 32bit
>
> (ok, I only test linux 64 bit), and I still don't get all the errors.

This is why I do OSX builds in my merge tests, they do pick
up some stuff other builds don't.

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/15] Migration

[Juan Quintela]

Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 October 2016 at 17:09, Juan Quintela <quintela@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 5 October 2016 at 14:44, Juan Quintela <quintela@redhat.com> wrote:
>>>> From: Juan Quintela <quintela@trasno.org>
>>>>
>>>> The following changes since commit bbc4c3f4f3c624e2de64fdcb79f4dd8c1a508e9d:
>>>>
>>>>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream'
>>>> into staging (2016-10-04 14:25:08 +0100)
>>>>
>>>> are available in the git repository at:
>>>>
>>>>   git://github.com/juanquintela/qemu.git tags/migration/20161005-1
>>>>
>>>> for you to fetch changes up to c62da143b6792cc32dbd5db15b936d7f58cc36ee:
>>>>
>>>>   docs/xbzrle: correction (2016-10-05 14:28:41 +0200)
>>>>
>>>> ----------------------------------------------------------------
>>>> migration/next for 20161005
>>>>
>>>> Hi
>>>>
>>>> This get:
>>>> - documentation fix: Cao
>>>> - improve max-bandwidth and downtime-limit (Ashijeet)
>>>> - move migration commands to "boxed" (Eric)
>>>> - rdma fixes (David)
>>>> - postcopy fixes (David)
>>>> - better errors (David)
>>>>
>>>> Please apply
>>>
>>> This runs into the "OSX %zu vs SIZE_MAX" issue again:
>>
>> Grrrr.
>> I already have trouble with %PR vs %zu for i386 vs anything else.
>>
>> Anyone has a linux crosscompiler for osx that I can use?  Or there is a
>> way to get darwin or whatever virtual machine to test compile this?
>
> Travis builds include OSX, if you're prepared to wait around
> for them to complete before submitting (they're usually about 10
> hours wall-clock time to complete a build).

And I was complaining that each of my full builds after a rebase can
take around 45mins ..... O:-)  They are all local.

>
>> It gets frustrating, before submissing I do:
>> - make check
>> - avocado virt test
>> - full compile on linux x86_64 (64 bits)
>> - full compile on linux i386 (32 bits)
>> - full cross-compile for windows 64bit
>> - full cross-compile for windows 32bit
>>
>> (ok, I only test linux 64 bit), and I still don't get all the errors.
>
> This is why I do OSX builds in my merge tests, they do pick
> up some stuff other builds don't.
>
> thanks
> -- PMM

Later, Juan.

Re: [Qemu-devel] [PULL 00/15] Migration

[Peter Maydell]

On 6 October 2016 at 17:23, Juan Quintela <quintela@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> Travis builds include OSX, if you're prepared to wait around
>> for them to complete before submitting (they're usually about 10
>> hours wall-clock time to complete a build).
>
> And I was complaining that each of my full builds after a rebase can
> take around 45mins ..... O:-)  They are all local.

None of the individual travis jobs take very long, I think it
just throttles our access to infrastructure and refuses to
parallelize us very much.

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/15] Migration

[Peter Maydell]

On 6 October 2016 at 11:59, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 5 October 2016 at 14:44, Juan Quintela <quintela@redhat.com> wrote:
>> From: Juan Quintela <quintela@trasno.org>
>>
>> The following changes since commit bbc4c3f4f3c624e2de64fdcb79f4dd8c1a508e9d:
>>
>>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2016-10-04 14:25:08 +0100)
>>
>> are available in the git repository at:
>>
>>   git://github.com/juanquintela/qemu.git tags/migration/20161005-1
>>
>> for you to fetch changes up to c62da143b6792cc32dbd5db15b936d7f58cc36ee:
>>
>>   docs/xbzrle: correction (2016-10-05 14:28:41 +0200)
>>
>> ----------------------------------------------------------------
>> migration/next for 20161005
>>
>> Hi
>>
>> This get:
>> - documentation fix: Cao
>> - improve max-bandwidth and downtime-limit (Ashijeet)
>> - move migration commands to "boxed" (Eric)
>> - rdma fixes (David)
>> - postcopy fixes (David)
>> - better errors (David)
>>
>> Please apply
>
> This runs into the "OSX %zu vs SIZE_MAX" issue again

Since Eric has now kindly provided a proper configure test and
generic workaround for the SIZE_MAX bug and it is in master,
I tried just re-applying this pull request, but unfortunately
it has a merge conflict in qapi-schema.json that I don't
understand well enough to resolve. So if you could produce
a new pull request that would be best, but you don't need
to worry about the OSX failure any more.

thanks
-- PMM

[Qemu-devel] [PULL 08/15] migrate: Use boxed qapi for migrate-set-parameters

[Juan Quintela]

From: Eric Blake <eblake@redhat.com>

Now that QAPI makes it easy to pass a struct around, we don't
have to declare as many parameters or local variables.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hmp.c                 | 40 ++++++++++++++-----------------
 migration/migration.c | 65 +++++++++++++++++++++------------------------------
 qapi-schema.json      |  2 +-
 3 files changed, 46 insertions(+), 61 deletions(-)

diff --git a/hmp.c b/hmp.c
index c405d3e..4c0f600 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1325,44 +1325,40 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
     const char *valuestr = qdict_get_str(qdict, "value");
     long valueint = 0;
     Error *err = NULL;
-    bool has_compress_level = false;
-    bool has_compress_threads = false;
-    bool has_decompress_threads = false;
-    bool has_cpu_throttle_initial = false;
-    bool has_cpu_throttle_increment = false;
-    bool has_tls_creds = false;
-    bool has_tls_hostname = false;
     bool use_int_value = false;
     int i;

     for (i = 0; i < MIGRATION_PARAMETER__MAX; i++) {
         if (strcmp(param, MigrationParameter_lookup[i]) == 0) {
+            MigrationParameters p = { 0 };
             switch (i) {
             case MIGRATION_PARAMETER_COMPRESS_LEVEL:
-                has_compress_level = true;
+                p.has_compress_level = true;
                 use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_COMPRESS_THREADS:
-                has_compress_threads = true;
+                p.has_compress_threads = true;
                 use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_DECOMPRESS_THREADS:
-                has_decompress_threads = true;
+                p.has_decompress_threads = true;
                 use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL:
-                has_cpu_throttle_initial = true;
+                p.has_cpu_throttle_initial = true;
                 use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT:
-                has_cpu_throttle_increment = true;
+                p.has_cpu_throttle_increment = true;
                 use_int_value = true;
                 break;
             case MIGRATION_PARAMETER_TLS_CREDS:
-                has_tls_creds = true;
+                p.has_tls_creds = true;
+                p.tls_creds = (char *) valuestr;
                 break;
             case MIGRATION_PARAMETER_TLS_HOSTNAME:
-                has_tls_hostname = true;
+                p.has_tls_hostname = true;
+                p.tls_hostname = (char *) valuestr;
                 break;
             }

@@ -1372,16 +1368,16 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
                                valuestr);
                     goto cleanup;
                 }
+                /* Set all integers; only one has_FOO will be set, and
+                 * the code ignores the remaining values */
+                p.compress_level = valueint;
+                p.compress_threads = valueint;
+                p.decompress_threads = valueint;
+                p.cpu_throttle_initial = valueint;
+                p.cpu_throttle_increment = valueint;
             }

-            qmp_migrate_set_parameters(has_compress_level, valueint,
-                                       has_compress_threads, valueint,
-                                       has_decompress_threads, valueint,
-                                       has_cpu_throttle_initial, valueint,
-                                       has_cpu_throttle_increment, valueint,
-                                       has_tls_creds, valuestr,
-                                       has_tls_hostname, valuestr,
-                                       &err);
+            qmp_migrate_set_parameters(&p, &err);
             break;
         }
     }
diff --git a/migration/migration.c b/migration/migration.c
index 1a8f26b..42336e3 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -766,78 +766,67 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
     }
 }

-void qmp_migrate_set_parameters(bool has_compress_level,
-                                int64_t compress_level,
-                                bool has_compress_threads,
-                                int64_t compress_threads,
-                                bool has_decompress_threads,
-                                int64_t decompress_threads,
-                                bool has_cpu_throttle_initial,
-                                int64_t cpu_throttle_initial,
-                                bool has_cpu_throttle_increment,
-                                int64_t cpu_throttle_increment,
-                                bool has_tls_creds,
-                                const char *tls_creds,
-                                bool has_tls_hostname,
-                                const char *tls_hostname,
-                                Error **errp)
+void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
 {
     MigrationState *s = migrate_get_current();

-    if (has_compress_level && (compress_level < 0 || compress_level > 9)) {
+    if (params->has_compress_level &&
+        (params->compress_level < 0 || params->compress_level > 9)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "compress_level",
                    "is invalid, it should be in the range of 0 to 9");
         return;
     }
-    if (has_compress_threads &&
-            (compress_threads < 1 || compress_threads > 255)) {
+    if (params->has_compress_threads &&
+        (params->compress_threads < 1 || params->compress_threads > 255)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                    "compress_threads",
                    "is invalid, it should be in the range of 1 to 255");
         return;
     }
-    if (has_decompress_threads &&
-            (decompress_threads < 1 || decompress_threads > 255)) {
+    if (params->has_decompress_threads &&
+        (params->decompress_threads < 1 || params->decompress_threads > 255)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                    "decompress_threads",
                    "is invalid, it should be in the range of 1 to 255");
         return;
     }
-    if (has_cpu_throttle_initial &&
-            (cpu_throttle_initial < 1 || cpu_throttle_initial > 99)) {
+    if (params->has_cpu_throttle_initial &&
+        (params->cpu_throttle_initial < 1 ||
+         params->cpu_throttle_initial > 99)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                    "cpu_throttle_initial",
                    "an integer in the range of 1 to 99");
     }
-    if (has_cpu_throttle_increment &&
-            (cpu_throttle_increment < 1 || cpu_throttle_increment > 99)) {
+    if (params->has_cpu_throttle_increment &&
+        (params->cpu_throttle_increment < 1 ||
+         params->cpu_throttle_increment > 99)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                    "cpu_throttle_increment",
                    "an integer in the range of 1 to 99");
     }

-    if (has_compress_level) {
-        s->parameters.compress_level = compress_level;
+    if (params->has_compress_level) {
+        s->parameters.compress_level = params->compress_level;
     }
-    if (has_compress_threads) {
-        s->parameters.compress_threads = compress_threads;
+    if (params->has_compress_threads) {
+        s->parameters.compress_threads = params->compress_threads;
     }
-    if (has_decompress_threads) {
-        s->parameters.decompress_threads = decompress_threads;
+    if (params->has_decompress_threads) {
+        s->parameters.decompress_threads = params->decompress_threads;
     }
-    if (has_cpu_throttle_initial) {
-        s->parameters.cpu_throttle_initial = cpu_throttle_initial;
+    if (params->has_cpu_throttle_initial) {
+        s->parameters.cpu_throttle_initial = params->cpu_throttle_initial;
     }
-    if (has_cpu_throttle_increment) {
-        s->parameters.cpu_throttle_increment = cpu_throttle_increment;
+    if (params->has_cpu_throttle_increment) {
+        s->parameters.cpu_throttle_increment = params->cpu_throttle_increment;
     }
-    if (has_tls_creds) {
+    if (params->has_tls_creds) {
         g_free(s->parameters.tls_creds);
-        s->parameters.tls_creds = g_strdup(tls_creds);
+        s->parameters.tls_creds = g_strdup(params->tls_creds);
     }
-    if (has_tls_hostname) {
+    if (params->has_tls_hostname) {
         g_free(s->parameters.tls_hostname);
-        s->parameters.tls_hostname = g_strdup(tls_hostname);
+        s->parameters.tls_hostname = g_strdup(params->tls_hostname);
     }
 }

diff --git a/qapi-schema.json b/qapi-schema.json
index e16e889..58bed71 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -672,7 +672,7 @@
 #
 # Since: 2.4
 ##
-{ 'command': 'migrate-set-parameters',
+{ 'command': 'migrate-set-parameters', 'boxed': true,
   'data': 'MigrationParameters' }

 #
-- 
2.7.4