[Qemu-devel] [PATCH v2 0/4] several migrations related patches

[Qemu-devel] [PATCH v2 0/4] several migrations related patches

[Peter Xu]

v2:
- fix comment for threshold_size [Dave]
- drop patch 5
- add Dave's r-b for each patch

Mostly small touch-ups, the last one looks like a cpu throttle bug on
time slice calculation (or I missed anything?). Anyway, please review,
thanks.

Peter Xu (4):
  migration: set current_active_state once
  migration: rename max_size to threshold_size
  hmp: info migrate_capability format tunes
  hmp: info migrate_parameters format tunes

 hmp.c                       | 26 +++++++++++---------------
 include/migration/vmstate.h |  3 ++-
 migration/migration.c       | 22 +++++++++++++---------
 migration/savevm.c          |  4 ++--
 4 files changed, 28 insertions(+), 27 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v2 1/4] migration: set current_active_state once

[Peter Xu]

We set it right above this one. No need to set it twice.

CC: Juan Quintela <quintela@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 54060f7..f9f4d98 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1938,7 +1938,6 @@ static void *migration_thread(void *opaque)
     qemu_savevm_state_begin(s->to_dst_file, &s->params);
 
     s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
-    current_active_state = MIGRATION_STATUS_ACTIVE;
     migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
                       MIGRATION_STATUS_ACTIVE);
 
-- 
2.7.4

[Qemu-devel] [PATCH v2 2/4] migration: rename max_size to threshold_size

[Peter Xu]

In migration codes (especially in migration_thread()), max_size is used
in many place for the threshold value that we will start to do the final
flush and jump to the next stage to dump the whole rest things to
destination. However its name is confusing to first readers. Let's
rename it to "threshold_size" when proper and add a comment for it. No
functional change is made.

CC: Juan Quintela <quintela@redhat.com>
CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Reviewed-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/migration/vmstate.h |  3 ++-
 migration/migration.c       | 21 +++++++++++++--------
 migration/savevm.c          |  4 ++--
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index f2dbf84..dad3984 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -56,7 +56,8 @@ typedef struct SaveVMHandlers {
 
     /* This runs outside the iothread lock!  */
     int (*save_live_setup)(QEMUFile *f, void *opaque);
-    void (*save_live_pending)(QEMUFile *f, void *opaque, uint64_t max_size,
+    void (*save_live_pending)(QEMUFile *f, void *opaque,
+                              uint64_t threshold_size,
                               uint64_t *non_postcopiable_pending,
                               uint64_t *postcopiable_pending);
     LoadStateHandler *load_state;
diff --git a/migration/migration.c b/migration/migration.c
index f9f4d98..2785c6c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1907,7 +1907,12 @@ static void *migration_thread(void *opaque)
     int64_t initial_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
     int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
     int64_t initial_bytes = 0;
-    int64_t max_size = 0;
+    /*
+     * The final stage happens when the remaining data is smaller than
+     * this threshold; it's calculated from the requested downtime and
+     * measured bandwidth
+     */
+    int64_t threshold_size = 0;
     int64_t start_time = initial_time;
     int64_t end_time;
     bool old_vm_running = false;
@@ -1951,17 +1956,17 @@ static void *migration_thread(void *opaque)
         if (!qemu_file_rate_limit(s->to_dst_file)) {
             uint64_t pend_post, pend_nonpost;
 
-            qemu_savevm_state_pending(s->to_dst_file, max_size, &pend_nonpost,
-                                      &pend_post);
+            qemu_savevm_state_pending(s->to_dst_file, threshold_size,
+                                      &pend_nonpost, &pend_post);
             pending_size = pend_nonpost + pend_post;
-            trace_migrate_pending(pending_size, max_size,
+            trace_migrate_pending(pending_size, threshold_size,
                                   pend_post, pend_nonpost);
-            if (pending_size && pending_size >= max_size) {
+            if (pending_size && pending_size >= threshold_size) {
                 /* Still a significant amount to transfer */
 
                 if (migrate_postcopy_ram() &&
                     s->state != MIGRATION_STATUS_POSTCOPY_ACTIVE &&
-                    pend_nonpost <= max_size &&
+                    pend_nonpost <= threshold_size &&
                     atomic_read(&s->start_postcopy)) {
 
                     if (!postcopy_start(s, &old_vm_running)) {
@@ -1993,13 +1998,13 @@ 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 * s->parameters.downtime_limit;
+            threshold_size = bandwidth * s->parameters.downtime_limit;
 
             s->mbps = (((double) transferred_bytes * 8.0) /
                     ((double) time_spent / 1000.0)) / 1000.0 / 1000.0;
 
             trace_migrate_transferred(transferred_bytes, time_spent,
-                                      bandwidth, max_size);
+                                      bandwidth, threshold_size);
             /* if we haven't sent anything, we don't want to recalculate
                10000 is a small enough number for our purposes */
             if (s->dirty_bytes_rate && transferred_bytes > 10000) {
diff --git a/migration/savevm.c b/migration/savevm.c
index 3b19a4a..59c04eb 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1197,7 +1197,7 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
  * the result is split into the amount for units that can and
  * for units that can't do postcopy.
  */
-void qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size,
+void qemu_savevm_state_pending(QEMUFile *f, uint64_t threshold_size,
                                uint64_t *res_non_postcopiable,
                                uint64_t *res_postcopiable)
 {
@@ -1216,7 +1216,7 @@ void qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size,
                 continue;
             }
         }
-        se->ops->save_live_pending(f, se->opaque, max_size,
+        se->ops->save_live_pending(f, se->opaque, threshold_size,
                                    res_non_postcopiable, res_postcopiable);
     }
 }
-- 
2.7.4

[Qemu-devel] [PATCH v2 3/4] hmp: info migrate_capability format tunes

[Peter Xu]

Dump the info in a single line is hard to read. Do it one per line.
Also, the first "capabilities:" didn't help much. Let's remove it.

CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Reviewed-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hmp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hmp.c b/hmp.c
index edb8970..95eef8c 100644
--- a/hmp.c
+++ b/hmp.c
@@ -265,13 +265,11 @@ void hmp_info_migrate_capabilities(Monitor *mon, const QDict *qdict)
     caps = qmp_query_migrate_capabilities(NULL);
 
     if (caps) {
-        monitor_printf(mon, "capabilities: ");
         for (cap = caps; cap; cap = cap->next) {
-            monitor_printf(mon, "%s: %s ",
+            monitor_printf(mon, "%s: %s\n",
                            MigrationCapability_lookup[cap->value->capability],
                            cap->value->state ? "on" : "off");
         }
-        monitor_printf(mon, "\n");
     }
 
     qapi_free_MigrationCapabilityStatusList(caps);
-- 
2.7.4

[Qemu-devel] [PATCH v2 4/4] hmp: info migrate_parameters format tunes

[Peter Xu]

Do the same (one per line) to the parameter list.

CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Reviewed-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hmp.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/hmp.c b/hmp.c
index 95eef8c..b33e39e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -282,46 +282,44 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
     params = qmp_query_migrate_parameters(NULL);
 
     if (params) {
-        monitor_printf(mon, "parameters:");
         assert(params->has_compress_level);
-        monitor_printf(mon, " %s: %" PRId64,
+        monitor_printf(mon, "%s: %" PRId64 "\n",
             MigrationParameter_lookup[MIGRATION_PARAMETER_COMPRESS_LEVEL],
             params->compress_level);
         assert(params->has_compress_threads);
-        monitor_printf(mon, " %s: %" PRId64,
+        monitor_printf(mon, "%s: %" PRId64 "\n",
             MigrationParameter_lookup[MIGRATION_PARAMETER_COMPRESS_THREADS],
             params->compress_threads);
         assert(params->has_decompress_threads);
-        monitor_printf(mon, " %s: %" PRId64,
+        monitor_printf(mon, "%s: %" PRId64 "\n",
             MigrationParameter_lookup[MIGRATION_PARAMETER_DECOMPRESS_THREADS],
             params->decompress_threads);
         assert(params->has_cpu_throttle_initial);
-        monitor_printf(mon, " %s: %" PRId64,
+        monitor_printf(mon, "%s: %" PRId64 "\n",
             MigrationParameter_lookup[MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL],
             params->cpu_throttle_initial);
         assert(params->has_cpu_throttle_increment);
-        monitor_printf(mon, " %s: %" PRId64,
+        monitor_printf(mon, "%s: %" PRId64 "\n",
             MigrationParameter_lookup[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT],
             params->cpu_throttle_increment);
-        monitor_printf(mon, " %s: '%s'",
+        monitor_printf(mon, "%s: '%s'\n",
             MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_CREDS],
             params->has_tls_creds ? params->tls_creds : "");
-        monitor_printf(mon, " %s: '%s'",
+        monitor_printf(mon, "%s: '%s'\n",
             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",
+        monitor_printf(mon, "%s: %" PRId64 " bytes/second\n",
             MigrationParameter_lookup[MIGRATION_PARAMETER_MAX_BANDWIDTH],
             params->max_bandwidth);
         assert(params->has_downtime_limit);
-        monitor_printf(mon, " %s: %" PRId64 " milliseconds",
+        monitor_printf(mon, "%s: %" PRId64 " milliseconds\n",
             MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIMIT],
             params->downtime_limit);
         assert(params->has_x_checkpoint_delay);
-        monitor_printf(mon, " %s: %" PRId64,
+        monitor_printf(mon, "%s: %" PRId64 "\n",
             MigrationParameter_lookup[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY],
             params->x_checkpoint_delay);
-        monitor_printf(mon, "\n");
     }
 
     qapi_free_MigrationParameters(params);
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 1/4] migration: set current_active_state once

[Philippe Mathieu-Daudé]

On 04/01/2017 05:18 AM, Peter Xu wrote:
> We set it right above this one. No need to set it twice.
>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  migration/migration.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 54060f7..f9f4d98 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1938,7 +1938,6 @@ static void *migration_thread(void *opaque)
>      qemu_savevm_state_begin(s->to_dst_file, &s->params);
>
>      s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
> -    current_active_state = MIGRATION_STATUS_ACTIVE;
>      migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
>                        MIGRATION_STATUS_ACTIVE);
>
>

Re: [Qemu-devel] [PATCH v2 2/4] migration: rename max_size to threshold_size

[Philippe Mathieu-Daudé]

On 04/01/2017 05:18 AM, Peter Xu wrote:
> In migration codes (especially in migration_thread()), max_size is used
> in many place for the threshold value that we will start to do the final
> flush and jump to the next stage to dump the whole rest things to
> destination. However its name is confusing to first readers. Let's
> rename it to "threshold_size" when proper and add a comment for it. No
> functional change is made.
>
> CC: Juan Quintela <quintela@redhat.com>
> CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Reviewed-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  include/migration/vmstate.h |  3 ++-
>  migration/migration.c       | 21 +++++++++++++--------
>  migration/savevm.c          |  4 ++--
>  3 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index f2dbf84..dad3984 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -56,7 +56,8 @@ typedef struct SaveVMHandlers {
>
>      /* This runs outside the iothread lock!  */
>      int (*save_live_setup)(QEMUFile *f, void *opaque);
> -    void (*save_live_pending)(QEMUFile *f, void *opaque, uint64_t max_size,
> +    void (*save_live_pending)(QEMUFile *f, void *opaque,
> +                              uint64_t threshold_size,
>                                uint64_t *non_postcopiable_pending,
>                                uint64_t *postcopiable_pending);
>      LoadStateHandler *load_state;
> diff --git a/migration/migration.c b/migration/migration.c
> index f9f4d98..2785c6c 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1907,7 +1907,12 @@ static void *migration_thread(void *opaque)
>      int64_t initial_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>      int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
>      int64_t initial_bytes = 0;
> -    int64_t max_size = 0;
> +    /*
> +     * The final stage happens when the remaining data is smaller than
> +     * this threshold; it's calculated from the requested downtime and
> +     * measured bandwidth
> +     */
> +    int64_t threshold_size = 0;
>      int64_t start_time = initial_time;
>      int64_t end_time;
>      bool old_vm_running = false;
> @@ -1951,17 +1956,17 @@ static void *migration_thread(void *opaque)
>          if (!qemu_file_rate_limit(s->to_dst_file)) {
>              uint64_t pend_post, pend_nonpost;
>
> -            qemu_savevm_state_pending(s->to_dst_file, max_size, &pend_nonpost,
> -                                      &pend_post);
> +            qemu_savevm_state_pending(s->to_dst_file, threshold_size,
> +                                      &pend_nonpost, &pend_post);
>              pending_size = pend_nonpost + pend_post;
> -            trace_migrate_pending(pending_size, max_size,
> +            trace_migrate_pending(pending_size, threshold_size,
>                                    pend_post, pend_nonpost);
> -            if (pending_size && pending_size >= max_size) {
> +            if (pending_size && pending_size >= threshold_size) {
>                  /* Still a significant amount to transfer */
>
>                  if (migrate_postcopy_ram() &&
>                      s->state != MIGRATION_STATUS_POSTCOPY_ACTIVE &&
> -                    pend_nonpost <= max_size &&
> +                    pend_nonpost <= threshold_size &&
>                      atomic_read(&s->start_postcopy)) {
>
>                      if (!postcopy_start(s, &old_vm_running)) {
> @@ -1993,13 +1998,13 @@ 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 * s->parameters.downtime_limit;
> +            threshold_size = bandwidth * s->parameters.downtime_limit;
>
>              s->mbps = (((double) transferred_bytes * 8.0) /
>                      ((double) time_spent / 1000.0)) / 1000.0 / 1000.0;
>
>              trace_migrate_transferred(transferred_bytes, time_spent,
> -                                      bandwidth, max_size);
> +                                      bandwidth, threshold_size);
>              /* if we haven't sent anything, we don't want to recalculate
>                 10000 is a small enough number for our purposes */
>              if (s->dirty_bytes_rate && transferred_bytes > 10000) {
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 3b19a4a..59c04eb 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1197,7 +1197,7 @@ void qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only)
>   * the result is split into the amount for units that can and
>   * for units that can't do postcopy.
>   */
> -void qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size,
> +void qemu_savevm_state_pending(QEMUFile *f, uint64_t threshold_size,
>                                 uint64_t *res_non_postcopiable,
>                                 uint64_t *res_postcopiable)
>  {
> @@ -1216,7 +1216,7 @@ void qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size,
>                  continue;
>              }
>          }
> -        se->ops->save_live_pending(f, se->opaque, max_size,
> +        se->ops->save_live_pending(f, se->opaque, threshold_size,
>                                     res_non_postcopiable, res_postcopiable);
>      }
>  }
>

Re: [Qemu-devel] [PATCH v2 3/4] hmp: info migrate_capability format tunes

[Philippe Mathieu-Daudé]

On 04/01/2017 05:18 AM, Peter Xu wrote:
> Dump the info in a single line is hard to read. Do it one per line.
> Also, the first "capabilities:" didn't help much. Let's remove it.
>
> CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Reviewed-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hmp.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index edb8970..95eef8c 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -265,13 +265,11 @@ void hmp_info_migrate_capabilities(Monitor *mon, const QDict *qdict)
>      caps = qmp_query_migrate_capabilities(NULL);
>
>      if (caps) {
> -        monitor_printf(mon, "capabilities: ");
>          for (cap = caps; cap; cap = cap->next) {
> -            monitor_printf(mon, "%s: %s ",
> +            monitor_printf(mon, "%s: %s\n",
>                             MigrationCapability_lookup[cap->value->capability],
>                             cap->value->state ? "on" : "off");
>          }
> -        monitor_printf(mon, "\n");
>      }
>
>      qapi_free_MigrationCapabilityStatusList(caps);
>

Re: [Qemu-devel] [PATCH v2 4/4] hmp: info migrate_parameters format tunes

[Philippe Mathieu-Daudé]

On 04/01/2017 05:18 AM, Peter Xu wrote:
> Do the same (one per line) to the parameter list.
>
> CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Reviewed-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hmp.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 95eef8c..b33e39e 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -282,46 +282,44 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
>      params = qmp_query_migrate_parameters(NULL);
>
>      if (params) {
> -        monitor_printf(mon, "parameters:");
>          assert(params->has_compress_level);
> -        monitor_printf(mon, " %s: %" PRId64,
> +        monitor_printf(mon, "%s: %" PRId64 "\n",
>              MigrationParameter_lookup[MIGRATION_PARAMETER_COMPRESS_LEVEL],
>              params->compress_level);
>          assert(params->has_compress_threads);
> -        monitor_printf(mon, " %s: %" PRId64,
> +        monitor_printf(mon, "%s: %" PRId64 "\n",
>              MigrationParameter_lookup[MIGRATION_PARAMETER_COMPRESS_THREADS],
>              params->compress_threads);
>          assert(params->has_decompress_threads);
> -        monitor_printf(mon, " %s: %" PRId64,
> +        monitor_printf(mon, "%s: %" PRId64 "\n",
>              MigrationParameter_lookup[MIGRATION_PARAMETER_DECOMPRESS_THREADS],
>              params->decompress_threads);
>          assert(params->has_cpu_throttle_initial);
> -        monitor_printf(mon, " %s: %" PRId64,
> +        monitor_printf(mon, "%s: %" PRId64 "\n",
>              MigrationParameter_lookup[MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL],
>              params->cpu_throttle_initial);
>          assert(params->has_cpu_throttle_increment);
> -        monitor_printf(mon, " %s: %" PRId64,
> +        monitor_printf(mon, "%s: %" PRId64 "\n",
>              MigrationParameter_lookup[MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT],
>              params->cpu_throttle_increment);
> -        monitor_printf(mon, " %s: '%s'",
> +        monitor_printf(mon, "%s: '%s'\n",
>              MigrationParameter_lookup[MIGRATION_PARAMETER_TLS_CREDS],
>              params->has_tls_creds ? params->tls_creds : "");
> -        monitor_printf(mon, " %s: '%s'",
> +        monitor_printf(mon, "%s: '%s'\n",
>              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",
> +        monitor_printf(mon, "%s: %" PRId64 " bytes/second\n",
>              MigrationParameter_lookup[MIGRATION_PARAMETER_MAX_BANDWIDTH],
>              params->max_bandwidth);
>          assert(params->has_downtime_limit);
> -        monitor_printf(mon, " %s: %" PRId64 " milliseconds",
> +        monitor_printf(mon, "%s: %" PRId64 " milliseconds\n",
>              MigrationParameter_lookup[MIGRATION_PARAMETER_DOWNTIME_LIMIT],
>              params->downtime_limit);
>          assert(params->has_x_checkpoint_delay);
> -        monitor_printf(mon, " %s: %" PRId64,
> +        monitor_printf(mon, "%s: %" PRId64 "\n",
>              MigrationParameter_lookup[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY],
>              params->x_checkpoint_delay);
> -        monitor_printf(mon, "\n");
>      }
>
>      qapi_free_MigrationParameters(params);
>

Re: [Qemu-devel] [PATCH v2 1/4] migration: set current_active_state once

[Juan Quintela]

Peter Xu <peterx@redhat.com> wrote:
> We set it right above this one. No need to set it twice.
>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

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

Re: [Qemu-devel] [PATCH v2 2/4] migration: rename max_size to threshold_size

[Juan Quintela]

Peter Xu <peterx@redhat.com> wrote:
> In migration codes (especially in migration_thread()), max_size is used
> in many place for the threshold value that we will start to do the final
> flush and jump to the next stage to dump the whole rest things to
> destination. However its name is confusing to first readers. Let's
> rename it to "threshold_size" when proper and add a comment for it. No
> functional change is made.
>
> CC: Juan Quintela <quintela@redhat.com>
> CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Reviewed-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

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

Re: [Qemu-devel] [PATCH v2 3/4] hmp: info migrate_capability format tunes

[Juan Quintela]

Peter Xu <peterx@redhat.com> wrote:
> Dump the info in a single line is hard to read. Do it one per line.
> Also, the first "capabilities:" didn't help much. Let's remove it.
>
> CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Reviewed-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

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

Re: [Qemu-devel] [PATCH v2 4/4] hmp: info migrate_parameters format tunes

[Juan Quintela]

Peter Xu <peterx@redhat.com> wrote:
> Do the same (one per line) to the parameter list.
>
> CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Reviewed-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

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