qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 00/12] migration queue
@ 2020-05-07 17:01 Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 01/12] migration: fix bad indentation in error_report() Dr. David Alan Gilbert (git)
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:01 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

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

The following changes since commit 3c7adbc67d9a5c3e992a4dd13b8704464daaad5b:

  Merge remote-tracking branch 'remotes/berrange/tags/qcrypto-next-pull-request' into staging (2020-05-07 14:30:12 +0100)

are available in the Git repository at:

  git://github.com/dagrh/qemu.git tags/pull-migration-20200507a

for you to fetch changes up to 13f2cb21e5fb33e9f8d7db8eee48edc1c67b812f:

  migration/multifd: Do error_free after migrate_set_error to avoid memleaks (2020-05-07 17:40:24 +0100)

----------------------------------------------------------------
Migration pull 2020-05-07

Mostly tidy-ups, but two new features:
  cpu-throttle-tailslow for making a gentler throttle
  xbzrle encoding rate measurement for getting a feal for xbzrle
performance.

----------------------------------------------------------------
David Hildenbrand (1):
      migration/ram: Consolidate variable reset after placement in ram_load_postcopy()

Keqian Zhu (1):
      migration/throttle: Add cpu-throttle-tailslow migration parameter

Mao Zhongyi (4):
      migration: fix bad indentation in error_report()
      migration/migration: improve error reporting for migrate parameters
      monitor/hmp-cmds: add hmp_handle_error() for hmp_migrate_set_speed()
      migration: move the units of migrate parameters from milliseconds to ms

Marc-André Lureau (1):
      docs/devel/migration: start a debugging section

Pan Nengyuan (3):
      migration/rdma: fix a memleak on error path in rdma_start_incoming_migration
      migration/multifd: fix memleaks in multifd_new_send_channel_async
      migration/multifd: Do error_free after migrate_set_error to avoid memleaks

Philippe Mathieu-Daudé (1):
      migration/colo: Add missing error-propagation code

Wei Wang (1):
      migration/xbzrle: add encoding rate

 docs/devel/migration.rst | 20 +++++++++++++
 migration/colo.c         |  3 ++
 migration/migration.c    | 44 +++++++++++++++++++---------
 migration/multifd.c      |  5 ++++
 migration/ram.c          | 74 ++++++++++++++++++++++++++++++++++++++++--------
 migration/rdma.c         |  1 +
 monitor/hmp-cmds.c       | 23 +++++++++++----
 qapi/migration.json      | 53 +++++++++++++++++++++++++++++++++-
 8 files changed, 192 insertions(+), 31 deletions(-)



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PULL 01/12] migration: fix bad indentation in error_report()
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
@ 2020-05-07 17:02 ` Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 02/12] migration/migration: improve error reporting for migrate parameters Dr. David Alan Gilbert (git)
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:02 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

From: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>

bad indentation conflicts with CODING_STYLE doc.

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Message-Id: <09f7529c665cac0c6a5e032ac6fdb6ca701f7e37.1585329482.git.maozhongyi@cmss.chinamobile.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 177cce9e95..8f27174ff6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2494,7 +2494,7 @@ retry:
         if (header_type >= MIG_RP_MSG_MAX ||
             header_type == MIG_RP_MSG_INVALID) {
             error_report("RP: Received invalid message 0x%04x length 0x%04x",
-                    header_type, header_len);
+                         header_type, header_len);
             mark_source_rp_bad(ms);
             goto out;
         }
@@ -2503,9 +2503,9 @@ retry:
             header_len != rp_cmd_args[header_type].len) ||
             header_len > sizeof(buf)) {
             error_report("RP: Received '%s' message (0x%04x) with"
-                    "incorrect length %d expecting %zu",
-                    rp_cmd_args[header_type].name, header_type, header_len,
-                    (size_t)rp_cmd_args[header_type].len);
+                         "incorrect length %d expecting %zu",
+                         rp_cmd_args[header_type].name, header_type, header_len,
+                         (size_t)rp_cmd_args[header_type].len);
             mark_source_rp_bad(ms);
             goto out;
         }
@@ -2560,7 +2560,7 @@ retry:
             }
             if (header_len != expected_len) {
                 error_report("RP: Req_Page_id with length %d expecting %zd",
-                        header_len, expected_len);
+                             header_len, expected_len);
                 mark_source_rp_bad(ms);
                 goto out;
             }
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 02/12] migration/migration: improve error reporting for migrate parameters
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 01/12] migration: fix bad indentation in error_report() Dr. David Alan Gilbert (git)
@ 2020-05-07 17:02 ` Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 03/12] monitor/hmp-cmds: add hmp_handle_error() for hmp_migrate_set_speed() Dr. David Alan Gilbert (git)
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:02 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

From: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>

use QERR_INVALID_PARAMETER_VALUE instead of
"Parameter '%s' expects" for consistency.

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Message-Id: <4ce71da4a5f98ad6ead0806ec71043473dcb4c07.1585641083.git.maozhongyi@cmss.chinamobile.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 8f27174ff6..6e079efdcc 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1202,16 +1202,19 @@ static bool migrate_params_check(MigrationParameters *params, Error **errp)
     }
 
     if (params->has_max_bandwidth && (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);
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   "max_bandwidth",
+                   "an integer in the range of 0 to "stringify(SIZE_MAX)
+                   " bytes/second");
         return false;
     }
 
     if (params->has_downtime_limit &&
         (params->downtime_limit > MAX_MIGRATE_DOWNTIME)) {
-        error_setg(errp, "Parameter 'downtime_limit' expects an integer in "
-                         "the range of 0 to %d milliseconds",
-                         MAX_MIGRATE_DOWNTIME);
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   "downtime_limit",
+                   "an integer in the range of 0 to "
+                    stringify(MAX_MIGRATE_DOWNTIME)" milliseconds");
         return false;
     }
 
@@ -2107,9 +2110,10 @@ void qmp_migrate_set_speed(int64_t value, Error **errp)
 void qmp_migrate_set_downtime(double value, Error **errp)
 {
     if (value < 0 || value > MAX_MIGRATE_DOWNTIME_SECONDS) {
-        error_setg(errp, "Parameter 'downtime_limit' expects an integer in "
-                         "the range of 0 to %d seconds",
-                         MAX_MIGRATE_DOWNTIME_SECONDS);
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   "downtime_limit",
+                   "an integer in the range of 0 to "
+                    stringify(MAX_MIGRATE_DOWNTIME_SECONDS)" seconds");
         return;
     }
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 03/12] monitor/hmp-cmds: add hmp_handle_error() for hmp_migrate_set_speed()
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 01/12] migration: fix bad indentation in error_report() Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 02/12] migration/migration: improve error reporting for migrate parameters Dr. David Alan Gilbert (git)
@ 2020-05-07 17:02 ` Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 04/12] migration: move the units of migrate parameters from milliseconds to ms Dr. David Alan Gilbert (git)
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:02 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

From: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <305323f835436023c53d759f5ab18af3ec874183.1585641083.git.maozhongyi@cmss.chinamobile.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 monitor/hmp-cmds.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 7f6e982dc8..9bb6946fbf 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1198,8 +1198,11 @@ 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)
 {
+    Error *err = NULL;
+
     int64_t value = qdict_get_int(qdict, "value");
-    qmp_migrate_set_speed(value, NULL);
+    qmp_migrate_set_speed(value, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 04/12] migration: move the units of migrate parameters from milliseconds to ms
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
                   ` (2 preceding siblings ...)
  2020-05-07 17:02 ` [PULL 03/12] monitor/hmp-cmds: add hmp_handle_error() for hmp_migrate_set_speed() Dr. David Alan Gilbert (git)
@ 2020-05-07 17:02 ` Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 05/12] docs/devel/migration: start a debugging section Dr. David Alan Gilbert (git)
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:02 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

From: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <474bb6cf67defb8be9de5035c11aee57a680557a.1585641083.git.maozhongyi@cmss.chinamobile.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 2 +-
 monitor/hmp-cmds.c    | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 6e079efdcc..79f16f6625 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1214,7 +1214,7 @@ static bool migrate_params_check(MigrationParameters *params, Error **errp)
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                    "downtime_limit",
                    "an integer in the range of 0 to "
-                    stringify(MAX_MIGRATE_DOWNTIME)" milliseconds");
+                    stringify(MAX_MIGRATE_DOWNTIME)" ms");
         return false;
     }
 
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 9bb6946fbf..1552dee489 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -231,18 +231,18 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
             monitor_printf(mon, "\n");
         }
 
-        monitor_printf(mon, "total time: %" PRIu64 " milliseconds\n",
+        monitor_printf(mon, "total time: %" PRIu64 " ms\n",
                        info->total_time);
         if (info->has_expected_downtime) {
-            monitor_printf(mon, "expected downtime: %" PRIu64 " milliseconds\n",
+            monitor_printf(mon, "expected downtime: %" PRIu64 " ms\n",
                            info->expected_downtime);
         }
         if (info->has_downtime) {
-            monitor_printf(mon, "downtime: %" PRIu64 " milliseconds\n",
+            monitor_printf(mon, "downtime: %" PRIu64 " ms\n",
                            info->downtime);
         }
         if (info->has_setup_time) {
-            monitor_printf(mon, "setup: %" PRIu64 " milliseconds\n",
+            monitor_printf(mon, "setup: %" PRIu64 " ms\n",
                            info->setup_time);
         }
     }
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 05/12] docs/devel/migration: start a debugging section
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
                   ` (3 preceding siblings ...)
  2020-05-07 17:02 ` [PULL 04/12] migration: move the units of migrate parameters from milliseconds to ms Dr. David Alan Gilbert (git)
@ 2020-05-07 17:02 ` Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 06/12] migration/colo: Add missing error-propagation code Dr. David Alan Gilbert (git)
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:02 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Explain how to use analyze-migration.py, this may help.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20200330174852.456148-1-marcandre.lureau@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 docs/devel/migration.rst | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
index e88918f763..2eb08624fc 100644
--- a/docs/devel/migration.rst
+++ b/docs/devel/migration.rst
@@ -50,6 +50,26 @@ All these migration protocols use the same infrastructure to
 save/restore state devices.  This infrastructure is shared with the
 savevm/loadvm functionality.
 
+Debugging
+=========
+
+The migration stream can be analyzed thanks to `scripts/analyze_migration.py`.
+
+Example usage:
+
+.. code-block:: shell
+
+  $ qemu-system-x86_64
+   (qemu) migrate "exec:cat > mig"
+  $ ./scripts/analyze_migration.py -f mig
+  {
+    "ram (3)": {
+        "section sizes": {
+            "pc.ram": "0x0000000008000000",
+  ...
+
+See also ``analyze_migration.py -h`` help for more options.
+
 Common infrastructure
 =====================
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 06/12] migration/colo: Add missing error-propagation code
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
                   ` (4 preceding siblings ...)
  2020-05-07 17:02 ` [PULL 05/12] docs/devel/migration: start a debugging section Dr. David Alan Gilbert (git)
@ 2020-05-07 17:02 ` Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 07/12] migration/throttle: Add cpu-throttle-tailslow migration parameter Dr. David Alan Gilbert (git)
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:02 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

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

Running the coccinelle script produced:

  $ spatch \
    --macro-file scripts/cocci-macro-file.h --include-headers \
    --sp-file scripts/coccinelle/find-missing-error_propagate.cocci \
    --keep-comments --smpl-spacing --dir .
  HANDLING: ./migration/colo.c
  [[manual check required: error_propagate() might be missing in migrate_set_block_enabled() ./migration/colo.c:439:4]]

Add the missing error_propagate() after review.

Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20200413205250.687-1-f4bug@amsat.org>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/colo.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/migration/colo.c b/migration/colo.c
index 1b3493729b..d015d4f84e 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -443,6 +443,9 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
 
     /* Disable block migration */
     migrate_set_block_enabled(false, &local_err);
+    if (local_err) {
+        goto out;
+    }
     qemu_mutex_lock_iothread();
 
 #ifdef CONFIG_REPLICATION
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 07/12] migration/throttle: Add cpu-throttle-tailslow migration parameter
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
                   ` (5 preceding siblings ...)
  2020-05-07 17:02 ` [PULL 06/12] migration/colo: Add missing error-propagation code Dr. David Alan Gilbert (git)
@ 2020-05-07 17:02 ` Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 08/12] migration/ram: Consolidate variable reset after placement in ram_load_postcopy() Dr. David Alan Gilbert (git)
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:02 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

From: Keqian Zhu <zhukeqian1@huawei.com>

At the tail stage of throttling, the Guest is very sensitive to
CPU percentage while the @cpu-throttle-increment is excessive
usually at tail stage.

If this parameter is true, we will compute the ideal CPU percentage
used by the Guest, which may exactly make the dirty rate match the
dirty rate threshold. Then we will choose a smaller throttle increment
between the one specified by @cpu-throttle-increment and the one
generated by ideal CPU percentage.

Therefore, it is compatible to traditional throttling, meanwhile
the throttle increment won't be excessive at tail stage. This may
make migration time longer, and is disabled by default.

Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
Message-Id: <20200413101508.54793-1-zhukeqian1@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 13 ++++++++++++
 migration/ram.c       | 25 +++++++++++++++++-----
 monitor/hmp-cmds.c    |  8 ++++++++
 qapi/migration.json   | 48 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 79f16f6625..f5dbffc442 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -785,6 +785,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     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_cpu_throttle_tailslow = true;
+    params->cpu_throttle_tailslow = s->parameters.cpu_throttle_tailslow;
     params->has_tls_creds = true;
     params->tls_creds = g_strdup(s->parameters.tls_creds);
     params->has_tls_hostname = true;
@@ -1327,6 +1329,10 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
         dest->cpu_throttle_increment = params->cpu_throttle_increment;
     }
 
+    if (params->has_cpu_throttle_tailslow) {
+        dest->cpu_throttle_tailslow = params->cpu_throttle_tailslow;
+    }
+
     if (params->has_tls_creds) {
         assert(params->tls_creds->type == QTYPE_QSTRING);
         dest->tls_creds = g_strdup(params->tls_creds->u.s);
@@ -1415,6 +1421,10 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
         s->parameters.cpu_throttle_increment = params->cpu_throttle_increment;
     }
 
+    if (params->has_cpu_throttle_tailslow) {
+        s->parameters.cpu_throttle_tailslow = params->cpu_throttle_tailslow;
+    }
+
     if (params->has_tls_creds) {
         g_free(s->parameters.tls_creds);
         assert(params->tls_creds->type == QTYPE_QSTRING);
@@ -3597,6 +3607,8 @@ static Property migration_properties[] = {
     DEFINE_PROP_UINT8("x-cpu-throttle-increment", MigrationState,
                       parameters.cpu_throttle_increment,
                       DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT),
+    DEFINE_PROP_BOOL("x-cpu-throttle-tailslow", MigrationState,
+                      parameters.cpu_throttle_tailslow, false),
     DEFINE_PROP_SIZE("x-max-bandwidth", MigrationState,
                       parameters.max_bandwidth, MAX_THROTTLE),
     DEFINE_PROP_UINT64("x-downtime-limit", MigrationState,
@@ -3703,6 +3715,7 @@ static void migration_instance_init(Object *obj)
     params->has_throttle_trigger_threshold = true;
     params->has_cpu_throttle_initial = true;
     params->has_cpu_throttle_increment = true;
+    params->has_cpu_throttle_tailslow = true;
     params->has_max_bandwidth = true;
     params->has_downtime_limit = true;
     params->has_x_checkpoint_delay = true;
diff --git a/migration/ram.c b/migration/ram.c
index 53166fc279..52fc032b83 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -616,20 +616,34 @@ static size_t save_page_header(RAMState *rs, QEMUFile *f,  RAMBlock *block,
  * able to complete migration. Some workloads dirty memory way too
  * fast and will not effectively converge, even with auto-converge.
  */
-static void mig_throttle_guest_down(void)
+static void mig_throttle_guest_down(uint64_t bytes_dirty_period,
+                                    uint64_t bytes_dirty_threshold)
 {
     MigrationState *s = migrate_get_current();
     uint64_t pct_initial = s->parameters.cpu_throttle_initial;
-    uint64_t pct_icrement = s->parameters.cpu_throttle_increment;
+    uint64_t pct_increment = s->parameters.cpu_throttle_increment;
+    bool pct_tailslow = s->parameters.cpu_throttle_tailslow;
     int pct_max = s->parameters.max_cpu_throttle;
 
+    uint64_t throttle_now = cpu_throttle_get_percentage();
+    uint64_t cpu_now, cpu_ideal, throttle_inc;
+
     /* We have not started throttling yet. Let's start it. */
     if (!cpu_throttle_active()) {
         cpu_throttle_set(pct_initial);
     } else {
         /* Throttling already on, just increase the rate */
-        cpu_throttle_set(MIN(cpu_throttle_get_percentage() + pct_icrement,
-                         pct_max));
+        if (!pct_tailslow) {
+            throttle_inc = pct_increment;
+        } else {
+            /* Compute the ideal CPU percentage used by Guest, which may
+             * make the dirty rate match the dirty rate threshold. */
+            cpu_now = 100 - throttle_now;
+            cpu_ideal = cpu_now * (bytes_dirty_threshold * 1.0 /
+                        bytes_dirty_period);
+            throttle_inc = MIN(cpu_now - cpu_ideal, pct_increment);
+        }
+        cpu_throttle_set(MIN(throttle_now + throttle_inc, pct_max));
     }
 }
 
@@ -919,7 +933,8 @@ static void migration_trigger_throttle(RAMState *rs)
             (++rs->dirty_rate_high_cnt >= 2)) {
             trace_migration_throttle();
             rs->dirty_rate_high_cnt = 0;
-            mig_throttle_guest_down();
+            mig_throttle_guest_down(bytes_dirty_period,
+                                    bytes_dirty_threshold);
         }
     }
 }
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 1552dee489..ade1f85e0c 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -420,6 +420,10 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, "%s: %u\n",
             MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT),
             params->cpu_throttle_increment);
+        assert(params->has_cpu_throttle_tailslow);
+        monitor_printf(mon, "%s: %s\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_TAILSLOW),
+            params->cpu_throttle_tailslow ? "on" : "off");
         assert(params->has_max_cpu_throttle);
         monitor_printf(mon, "%s: %u\n",
             MigrationParameter_str(MIGRATION_PARAMETER_MAX_CPU_THROTTLE),
@@ -1275,6 +1279,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
         p->has_cpu_throttle_increment = true;
         visit_type_int(v, param, &p->cpu_throttle_increment, &err);
         break;
+    case MIGRATION_PARAMETER_CPU_THROTTLE_TAILSLOW:
+        p->has_cpu_throttle_tailslow = true;
+        visit_type_bool(v, param, &p->cpu_throttle_tailslow, &err);
+        break;
     case MIGRATION_PARAMETER_MAX_CPU_THROTTLE:
         p->has_max_cpu_throttle = true;
         visit_type_int(v, param, &p->max_cpu_throttle, &err);
diff --git a/qapi/migration.json b/qapi/migration.json
index eca2981d0a..ee6c5a0cae 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -552,6 +552,21 @@
 #                          auto-converge detects that migration is not making
 #                          progress. The default value is 10. (Since 2.7)
 #
+# @cpu-throttle-tailslow: Make CPU throttling slower at tail stage
+#                         At the tail stage of throttling, the Guest is very
+#                         sensitive to CPU percentage while the @cpu-throttle
+#                         -increment is excessive usually at tail stage.
+#                         If this parameter is true, we will compute the ideal
+#                         CPU percentage used by the Guest, which may exactly make
+#                         the dirty rate match the dirty rate threshold. Then we
+#                         will choose a smaller throttle increment between the
+#                         one specified by @cpu-throttle-increment and the one
+#                         generated by ideal CPU percentage.
+#                         Therefore, it is compatible to traditional throttling,
+#                         meanwhile the throttle increment won't be excessive
+#                         at tail stage.
+#                         The default value is false. (Since 5.1)
+#
 # @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
@@ -631,6 +646,7 @@
            'compress-level', 'compress-threads', 'decompress-threads',
            'compress-wait-thread', 'throttle-trigger-threshold',
            'cpu-throttle-initial', 'cpu-throttle-increment',
+           'cpu-throttle-tailslow',
            'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth',
            'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
            'multifd-channels',
@@ -676,6 +692,21 @@
 #                          auto-converge detects that migration is not making
 #                          progress. The default value is 10. (Since 2.7)
 #
+# @cpu-throttle-tailslow: Make CPU throttling slower at tail stage
+#                         At the tail stage of throttling, the Guest is very
+#                         sensitive to CPU percentage while the @cpu-throttle
+#                         -increment is excessive usually at tail stage.
+#                         If this parameter is true, we will compute the ideal
+#                         CPU percentage used by the Guest, which may exactly make
+#                         the dirty rate match the dirty rate threshold. Then we
+#                         will choose a smaller throttle increment between the
+#                         one specified by @cpu-throttle-increment and the one
+#                         generated by ideal CPU percentage.
+#                         Therefore, it is compatible to traditional throttling,
+#                         meanwhile the throttle increment won't be excessive
+#                         at tail stage.
+#                         The default value is false. (Since 5.1)
+#
 # @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
@@ -763,6 +794,7 @@
             '*throttle-trigger-threshold': 'int',
             '*cpu-throttle-initial': 'int',
             '*cpu-throttle-increment': 'int',
+            '*cpu-throttle-tailslow': 'bool',
             '*tls-creds': 'StrOrNull',
             '*tls-hostname': 'StrOrNull',
             '*tls-authz': 'StrOrNull',
@@ -834,6 +866,21 @@
 #                          auto-converge detects that migration is not making
 #                          progress. (Since 2.7)
 #
+# @cpu-throttle-tailslow: Make CPU throttling slower at tail stage
+#                         At the tail stage of throttling, the Guest is very
+#                         sensitive to CPU percentage while the @cpu-throttle
+#                         -increment is excessive usually at tail stage.
+#                         If this parameter is true, we will compute the ideal
+#                         CPU percentage used by the Guest, which may exactly make
+#                         the dirty rate match the dirty rate threshold. Then we
+#                         will choose a smaller throttle increment between the
+#                         one specified by @cpu-throttle-increment and the one
+#                         generated by ideal CPU percentage.
+#                         Therefore, it is compatible to traditional throttling,
+#                         meanwhile the throttle increment won't be excessive
+#                         at tail stage.
+#                         The default value is false. (Since 5.1)
+#
 # @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
@@ -921,6 +968,7 @@
             '*throttle-trigger-threshold': 'uint8',
             '*cpu-throttle-initial': 'uint8',
             '*cpu-throttle-increment': 'uint8',
+            '*cpu-throttle-tailslow': 'bool',
             '*tls-creds': 'str',
             '*tls-hostname': 'str',
             '*tls-authz': 'str',
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 08/12] migration/ram: Consolidate variable reset after placement in ram_load_postcopy()
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
                   ` (6 preceding siblings ...)
  2020-05-07 17:02 ` [PULL 07/12] migration/throttle: Add cpu-throttle-tailslow migration parameter Dr. David Alan Gilbert (git)
@ 2020-05-07 17:02 ` Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 09/12] migration/rdma: fix a memleak on error path in rdma_start_incoming_migration Dr. David Alan Gilbert (git)
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:02 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

From: David Hildenbrand <david@redhat.com>

Let's consolidate resetting the variables.

Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20200421085300.7734-10-david@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
  Fixup for context conflicts with 91ba442
---
 migration/ram.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 52fc032b83..08eb382f53 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3147,7 +3147,7 @@ static int ram_load_postcopy(QEMUFile *f)
     /* Temporary page that is later 'placed' */
     void *postcopy_host_page = mis->postcopy_tmp_page;
     void *this_host = NULL;
-    bool all_zero = false;
+    bool all_zero = true;
     int target_pages = 0;
 
     while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
@@ -3174,7 +3174,6 @@ static int ram_load_postcopy(QEMUFile *f)
         addr &= TARGET_PAGE_MASK;
 
         trace_ram_load_postcopy_loop((uint64_t)addr, flags);
-        place_needed = false;
         if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
                      RAM_SAVE_FLAG_COMPRESS_PAGE)) {
             block = ram_block_from_stream(f, flags);
@@ -3199,9 +3198,7 @@ static int ram_load_postcopy(QEMUFile *f)
              */
             page_buffer = postcopy_host_page +
                           ((uintptr_t)host & (block->page_size - 1));
-            /* If all TP are zero then we can optimise the place */
             if (target_pages == 1) {
-                all_zero = true;
                 this_host = (void *)QEMU_ALIGN_DOWN((uintptr_t)host,
                                                     block->page_size);
             } else {
@@ -3221,7 +3218,6 @@ static int ram_load_postcopy(QEMUFile *f)
              */
             if (target_pages == (block->page_size / TARGET_PAGE_SIZE)) {
                 place_needed = true;
-                target_pages = 0;
             }
             place_source = postcopy_host_page;
         }
@@ -3303,6 +3299,10 @@ static int ram_load_postcopy(QEMUFile *f)
                 ret = postcopy_place_page(mis, place_dest,
                                           place_source, block);
             }
+            place_needed = false;
+            target_pages = 0;
+            /* Assume we have a zero page until we detect something different */
+            all_zero = true;
         }
     }
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 09/12] migration/rdma: fix a memleak on error path in rdma_start_incoming_migration
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
                   ` (7 preceding siblings ...)
  2020-05-07 17:02 ` [PULL 08/12] migration/ram: Consolidate variable reset after placement in ram_load_postcopy() Dr. David Alan Gilbert (git)
@ 2020-05-07 17:02 ` Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 10/12] migration/xbzrle: add encoding rate Dr. David Alan Gilbert (git)
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:02 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

From: Pan Nengyuan <pannengyuan@huawei.com>

'rdma->host' is malloced in qemu_rdma_data_init, but forgot to free on the error
path in rdma_start_incoming_migration(), this patch fix that.

The leak stack:
Direct leak of 2 byte(s) in 1 object(s) allocated from:
    #0 0x7fb7add18ae8 in __interceptor_malloc (/lib64/libasan.so.5+0xefae8)
    #1 0x7fb7ad0df1d5 in g_malloc (/lib64/libglib-2.0.so.0+0x531d5)
    #2 0x7fb7ad0f8b32 in g_strdup (/lib64/libglib-2.0.so.0+0x6cb32)
    #3 0x55a0464a0f6f in qemu_rdma_data_init /mnt/sdb/qemu/migration/rdma.c:2647
    #4 0x55a0464b0e76 in rdma_start_incoming_migration /mnt/sdb/qemu/migration/rdma.c:4020
    #5 0x55a0463f898a in qemu_start_incoming_migration /mnt/sdb/qemu/migration/migration.c:365
    #6 0x55a0458c75d3 in qemu_init /mnt/sdb/qemu/softmmu/vl.c:4438
    #7 0x55a046a3d811 in main /mnt/sdb/qemu/softmmu/main.c:48
    #8 0x7fb7a8417872 in __libc_start_main (/lib64/libc.so.6+0x23872)
    #9 0x55a04536b26d in _start (/mnt/sdb/qemu/build/x86_64-softmmu/qemu-system-x86_64+0x286926d)

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Message-Id: <20200420102727.17339-1-pannengyuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/rdma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration/rdma.c b/migration/rdma.c
index f61587891b..967fda5b0c 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -4056,6 +4056,7 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp)
     return;
 err:
     error_propagate(errp, local_err);
+    g_free(rdma->host);
     g_free(rdma);
     g_free(rdma_return_path);
 }
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 10/12] migration/xbzrle: add encoding rate
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
                   ` (8 preceding siblings ...)
  2020-05-07 17:02 ` [PULL 09/12] migration/rdma: fix a memleak on error path in rdma_start_incoming_migration Dr. David Alan Gilbert (git)
@ 2020-05-07 17:02 ` Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 11/12] migration/multifd: fix memleaks in multifd_new_send_channel_async Dr. David Alan Gilbert (git)
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:02 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

From: Wei Wang <wei.w.wang@intel.com>

Users may need to check the xbzrle encoding rate to know if the guest
memory is xbzrle encoding-friendly, and dynamically turn off the
encoding if the encoding rate is low.

Signed-off-by: Yi Sun <yi.y.sun@intel.com>
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Message-Id: <1588208375-19556-1-git-send-email-wei.w.wang@intel.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c |  1 +
 migration/ram.c       | 39 +++++++++++++++++++++++++++++++++++++--
 monitor/hmp-cmds.c    |  2 ++
 qapi/migration.json   |  5 ++++-
 4 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index f5dbffc442..0bb042a0f7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -932,6 +932,7 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s)
         info->xbzrle_cache->pages = xbzrle_counters.pages;
         info->xbzrle_cache->cache_miss = xbzrle_counters.cache_miss;
         info->xbzrle_cache->cache_miss_rate = xbzrle_counters.cache_miss_rate;
+        info->xbzrle_cache->encoding_rate = xbzrle_counters.encoding_rate;
         info->xbzrle_cache->overflow = xbzrle_counters.overflow;
     }
 
diff --git a/migration/ram.c b/migration/ram.c
index 08eb382f53..859f835f1a 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -327,6 +327,10 @@ struct RAMState {
     uint64_t num_dirty_pages_period;
     /* xbzrle misses since the beginning of the period */
     uint64_t xbzrle_cache_miss_prev;
+    /* Amount of xbzrle pages since the beginning of the period */
+    uint64_t xbzrle_pages_prev;
+    /* Amount of xbzrle encoded bytes since the beginning of the period */
+    uint64_t xbzrle_bytes_prev;
 
     /* compression statistics since the beginning of the period */
     /* amount of count that no free thread to compress data */
@@ -710,6 +714,18 @@ static int save_xbzrle_page(RAMState *rs, uint8_t **current_data,
         return -1;
     }
 
+    /*
+     * Reaching here means the page has hit the xbzrle cache, no matter what
+     * encoding result it is (normal encoding, overflow or skipping the page),
+     * count the page as encoded. This is used to caculate the encoding rate.
+     *
+     * Example: 2 pages (8KB) being encoded, first page encoding generates 2KB,
+     * 2nd page turns out to be skipped (i.e. no new bytes written to the
+     * page), the overall encoding rate will be 8KB / 2KB = 4, which has the
+     * skipped page included. In this way, the encoding rate can tell if the
+     * guest page is good for xbzrle encoding.
+     */
+    xbzrle_counters.pages++;
     prev_cached_page = get_cached_data(XBZRLE.cache, current_addr);
 
     /* save current buffer into memory */
@@ -740,6 +756,7 @@ static int save_xbzrle_page(RAMState *rs, uint8_t **current_data,
     } else if (encoded_len == -1) {
         trace_save_xbzrle_page_overflow();
         xbzrle_counters.overflow++;
+        xbzrle_counters.bytes += TARGET_PAGE_SIZE;
         return -1;
     }
 
@@ -750,8 +767,12 @@ static int save_xbzrle_page(RAMState *rs, uint8_t **current_data,
     qemu_put_be16(rs->f, encoded_len);
     qemu_put_buffer(rs->f, XBZRLE.encoded_buf, encoded_len);
     bytes_xbzrle += encoded_len + 1 + 2;
-    xbzrle_counters.pages++;
-    xbzrle_counters.bytes += bytes_xbzrle;
+    /*
+     * Like compressed_size (please see update_compress_thread_counts),
+     * the xbzrle encoded bytes don't count the 8 byte header with
+     * RAM_SAVE_FLAG_CONTINUE.
+     */
+    xbzrle_counters.bytes += bytes_xbzrle - 8;
     ram_counters.transferred += bytes_xbzrle;
 
     return 1;
@@ -884,9 +905,23 @@ static void migration_update_rates(RAMState *rs, int64_t end_time)
     }
 
     if (migrate_use_xbzrle()) {
+        double encoded_size, unencoded_size;
+
         xbzrle_counters.cache_miss_rate = (double)(xbzrle_counters.cache_miss -
             rs->xbzrle_cache_miss_prev) / page_count;
         rs->xbzrle_cache_miss_prev = xbzrle_counters.cache_miss;
+        unencoded_size = (xbzrle_counters.pages - rs->xbzrle_pages_prev) *
+                         TARGET_PAGE_SIZE;
+        encoded_size = xbzrle_counters.bytes - rs->xbzrle_bytes_prev;
+        if (xbzrle_counters.pages == rs->xbzrle_pages_prev) {
+            xbzrle_counters.encoding_rate = 0;
+        } else if (!encoded_size) {
+            xbzrle_counters.encoding_rate = UINT64_MAX;
+        } else {
+            xbzrle_counters.encoding_rate = unencoded_size / encoded_size;
+        }
+        rs->xbzrle_pages_prev = xbzrle_counters.pages;
+        rs->xbzrle_bytes_prev = xbzrle_counters.bytes;
     }
 
     if (migrate_use_compression()) {
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index ade1f85e0c..9c61e769ca 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -303,6 +303,8 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
                        info->xbzrle_cache->cache_miss);
         monitor_printf(mon, "xbzrle cache miss rate: %0.2f\n",
                        info->xbzrle_cache->cache_miss_rate);
+        monitor_printf(mon, "xbzrle encoding rate: %0.2f\n",
+                       info->xbzrle_cache->encoding_rate);
         monitor_printf(mon, "xbzrle overflow: %" PRIu64 "\n",
                        info->xbzrle_cache->overflow);
     }
diff --git a/qapi/migration.json b/qapi/migration.json
index ee6c5a0cae..d5000558c6 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -70,6 +70,8 @@
 #
 # @cache-miss-rate: rate of cache miss (since 2.1)
 #
+# @encoding-rate: rate of encoded bytes (since 5.1)
+#
 # @overflow: number of overflows
 #
 # Since: 1.2
@@ -77,7 +79,7 @@
 { 'struct': 'XBZRLECacheStats',
   'data': {'cache-size': 'int', 'bytes': 'int', 'pages': 'int',
            'cache-miss': 'int', 'cache-miss-rate': 'number',
-           'overflow': 'int' } }
+           'encoding-rate': 'number', 'overflow': 'int' } }
 
 ##
 # @CompressionStats:
@@ -337,6 +339,7 @@
 #             "pages":2444343,
 #             "cache-miss":2244,
 #             "cache-miss-rate":0.123,
+#             "encoding-rate":80.1,
 #             "overflow":34434
 #          }
 #       }
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 11/12] migration/multifd: fix memleaks in multifd_new_send_channel_async
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
                   ` (9 preceding siblings ...)
  2020-05-07 17:02 ` [PULL 10/12] migration/xbzrle: add encoding rate Dr. David Alan Gilbert (git)
@ 2020-05-07 17:02 ` Dr. David Alan Gilbert (git)
  2020-05-07 17:02 ` [PULL 12/12] migration/multifd: Do error_free after migrate_set_error to avoid memleaks Dr. David Alan Gilbert (git)
  2020-05-07 18:49 ` [PULL 00/12] migration queue Peter Maydell
  12 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:02 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

From: Pan Nengyuan <pannengyuan@huawei.com>

When error happen in multifd_new_send_channel_async, 'sioc' will not be used
to create the multifd_send_thread. Let's free it to avoid a memleak. And also
do error_free after migrate_set_error() to avoid another leak in the same place.

The leak stack:
Direct leak of 2880 byte(s) in 8 object(s) allocated from:
    #0 0x7f20b5118ae8 in __interceptor_malloc (/lib64/libasan.so.5+0xefae8)
    #1 0x7f20b44df1d5 in g_malloc (/lib64/libglib-2.0.so.0+0x531d5)
    #2 0x564133bce18b in object_new_with_type /mnt/sdb/backup/qemu/qom/object.c:683
    #3 0x564133eea950 in qio_channel_socket_new /mnt/sdb/backup/qemu/io/channel-socket.c:56
    #4 0x5641339cfe4f in socket_send_channel_create /mnt/sdb/backup/qemu/migration/socket.c:37
    #5 0x564133a10328 in multifd_save_setup /mnt/sdb/backup/qemu/migration/multifd.c:772
    #6 0x5641339cebed in migrate_fd_connect /mnt/sdb/backup/qemu/migration/migration.c:3530
    #7 0x5641339d15e4 in migration_channel_connect /mnt/sdb/backup/qemu/migration/channel.c:92
    #8 0x5641339cf5b7 in socket_outgoing_migration /mnt/sdb/backup/qemu/migration/socket.c:108

Direct leak of 384 byte(s) in 8 object(s) allocated from:
    #0 0x7f20b5118cf0 in calloc (/lib64/libasan.so.5+0xefcf0)
    #1 0x7f20b44df22d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5322d)
    #2 0x56413406fc17 in error_setv /mnt/sdb/backup/qemu/util/error.c:61
    #3 0x564134070464 in error_setg_errno_internal /mnt/sdb/backup/qemu/util/error.c:109
    #4 0x5641340851be in inet_connect_addr /mnt/sdb/backup/qemu/util/qemu-sockets.c:379
    #5 0x5641340851be in inet_connect_saddr /mnt/sdb/backup/qemu/util/qemu-sockets.c:458
    #6 0x5641340870ab in socket_connect /mnt/sdb/backup/qemu/util/qemu-sockets.c:1105
    #7 0x564133eeaabf in qio_channel_socket_connect_sync /mnt/sdb/backup/qemu/io/channel-socket.c:145
    #8 0x564133eeabf5 in qio_channel_socket_connect_worker /mnt/sdb/backup/qemu/io/channel-socket.c:168

Indirect leak of 360 byte(s) in 8 object(s) allocated from:
    #0 0x7f20b5118ae8 in __interceptor_malloc (/lib64/libasan.so.5+0xefae8)
    #1 0x7f20af901817 in __GI___vasprintf_chk (/lib64/libc.so.6+0x10d817)
    #2 0x7f20b451fa6c in g_vasprintf (/lib64/libglib-2.0.so.0+0x93a6c)
    #3 0x7f20b44f8cd0 in g_strdup_vprintf (/lib64/libglib-2.0.so.0+0x6ccd0)
    #4 0x7f20b44f8d8c in g_strdup_printf (/lib64/libglib-2.0.so.0+0x6cd8c)
    #5 0x56413406fc86 in error_setv /mnt/sdb/backup/qemu/util/error.c:65
    #6 0x564134070464 in error_setg_errno_internal /mnt/sdb/backup/qemu/util/error.c:109
    #7 0x5641340851be in inet_connect_addr /mnt/sdb/backup/qemu/util/qemu-sockets.c:379
    #8 0x5641340851be in inet_connect_saddr /mnt/sdb/backup/qemu/util/qemu-sockets.c:458
    #9 0x5641340870ab in socket_connect /mnt/sdb/backup/qemu/util/qemu-sockets.c:1105
    #10 0x564133eeaabf in qio_channel_socket_connect_sync /mnt/sdb/backup/qemu/io/channel-socket.c:145
    #11 0x564133eeabf5 in qio_channel_socket_connect_worker /mnt/sdb/backup/qemu/io/channel-socket.c:168

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Message-Id: <20200506095416.26099-2-pannengyuan@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/multifd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/migration/multifd.c b/migration/multifd.c
index 9123c111a3..e3efd33a0d 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -727,6 +727,8 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
          * its status.
          */
         p->quit = true;
+        object_unref(OBJECT(sioc));
+        error_free(local_err);
     } else {
         p->c = QIO_CHANNEL(sioc);
         qio_channel_set_delay(p->c, false);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PULL 12/12] migration/multifd: Do error_free after migrate_set_error to avoid memleaks
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
                   ` (10 preceding siblings ...)
  2020-05-07 17:02 ` [PULL 11/12] migration/multifd: fix memleaks in multifd_new_send_channel_async Dr. David Alan Gilbert (git)
@ 2020-05-07 17:02 ` Dr. David Alan Gilbert (git)
  2020-05-07 18:49 ` [PULL 00/12] migration queue Peter Maydell
  12 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2020-05-07 17:02 UTC (permalink / raw)
  To: qemu-devel, david, zhukeqian1, maozhongyi, marcandre.lureau,
	pannengyuan, f4bug, wei.w.wang, yi.y.sun, quintela

From: Pan Nengyuan <pannengyuan@huawei.com>

When error happen in multifd_send_thread, it use error_copy to set migrate error in
multifd_send_terminate_threads(). We should call error_free after it.

Similarly, fix another two places in multifd_recv_thread/multifd_save_cleanup.

The leak stack:
Direct leak of 48 byte(s) in 1 object(s) allocated from:
    #0 0x7f781af07cf0 in calloc (/lib64/libasan.so.5+0xefcf0)
    #1 0x7f781a2ce22d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5322d)
    #2 0x55ee1d075c17 in error_setv /mnt/sdb/backup/qemu/util/error.c:61
    #3 0x55ee1d076464 in error_setg_errno_internal /mnt/sdb/backup/qemu/util/error.c:109
    #4 0x55ee1cef066e in qio_channel_socket_writev /mnt/sdb/backup/qemu/io/channel-socket.c:569
    #5 0x55ee1cee806b in qio_channel_writev /mnt/sdb/backup/qemu/io/channel.c:207
    #6 0x55ee1cee806b in qio_channel_writev_all /mnt/sdb/backup/qemu/io/channel.c:171
    #7 0x55ee1cee8248 in qio_channel_write_all /mnt/sdb/backup/qemu/io/channel.c:257
    #8 0x55ee1ca12c9a in multifd_send_thread /mnt/sdb/backup/qemu/migration/multifd.c:657
    #9 0x55ee1d0607fc in qemu_thread_start /mnt/sdb/backup/qemu/util/qemu-thread-posix.c:519
    #10 0x7f78159ae2dd in start_thread (/lib64/libpthread.so.0+0x82dd)
    #11 0x7f78156df4b2 in __GI___clone (/lib64/libc.so.6+0xfc4b2)

Indirect leak of 52 byte(s) in 1 object(s) allocated from:
    #0 0x7f781af07f28 in __interceptor_realloc (/lib64/libasan.so.5+0xeff28)
    #1 0x7f78156f07d9 in __GI___vasprintf_chk (/lib64/libc.so.6+0x10d7d9)
    #2 0x7f781a30ea6c in g_vasprintf (/lib64/libglib-2.0.so.0+0x93a6c)
    #3 0x7f781a2e7cd0 in g_strdup_vprintf (/lib64/libglib-2.0.so.0+0x6ccd0)
    #4 0x7f781a2e7d8c in g_strdup_printf (/lib64/libglib-2.0.so.0+0x6cd8c)
    #5 0x55ee1d075c86 in error_setv /mnt/sdb/backup/qemu/util/error.c:65
    #6 0x55ee1d076464 in error_setg_errno_internal /mnt/sdb/backup/qemu/util/error.c:109
    #7 0x55ee1cef066e in qio_channel_socket_writev /mnt/sdb/backup/qemu/io/channel-socket.c:569
    #8 0x55ee1cee806b in qio_channel_writev /mnt/sdb/backup/qemu/io/channel.c:207
    #9 0x55ee1cee806b in qio_channel_writev_all /mnt/sdb/backup/qemu/io/channel.c:171
    #10 0x55ee1cee8248 in qio_channel_write_all /mnt/sdb/backup/qemu/io/channel.c:257
    #11 0x55ee1ca12c9a in multifd_send_thread /mnt/sdb/backup/qemu/migration/multifd.c:657
    #12 0x55ee1d0607fc in qemu_thread_start /mnt/sdb/backup/qemu/util/qemu-thread-posix.c:519
    #13 0x7f78159ae2dd in start_thread (/lib64/libpthread.so.0+0x82dd)
    #14 0x7f78156df4b2 in __GI___clone (/lib64/libc.so.6+0xfc4b2)

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Message-Id: <20200506095416.26099-3-pannengyuan@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/multifd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/migration/multifd.c b/migration/multifd.c
index e3efd33a0d..5a3e4d0d46 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -550,6 +550,7 @@ void multifd_save_cleanup(void)
         multifd_send_state->ops->send_cleanup(p, &local_err);
         if (local_err) {
             migrate_set_error(migrate_get_current(), local_err);
+            error_free(local_err);
         }
     }
     qemu_sem_destroy(&multifd_send_state->channels_ready);
@@ -688,6 +689,7 @@ out:
     if (local_err) {
         trace_multifd_send_error(p->id);
         multifd_send_terminate_threads(local_err);
+        error_free(local_err);
     }
 
     /*
@@ -965,6 +967,7 @@ static void *multifd_recv_thread(void *opaque)
 
     if (local_err) {
         multifd_recv_terminate_threads(local_err);
+        error_free(local_err);
     }
     qemu_mutex_lock(&p->mutex);
     p->running = false;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PULL 00/12] migration queue
  2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
                   ` (11 preceding siblings ...)
  2020-05-07 17:02 ` [PULL 12/12] migration/multifd: Do error_free after migrate_set_error to avoid memleaks Dr. David Alan Gilbert (git)
@ 2020-05-07 18:49 ` Peter Maydell
  12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2020-05-07 18:49 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git)
  Cc: Mao Zhongyi, David Hildenbrand, Juan Quintela, Pan Nengyuan,
	QEMU Developers, Philippe Mathieu-Daudé,
	wei.w.wang, Marc-André Lureau, Keqian Zhu, yi.y.sun

On Thu, 7 May 2020 at 18:04, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
>
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The following changes since commit 3c7adbc67d9a5c3e992a4dd13b8704464daaad5b:
>
>   Merge remote-tracking branch 'remotes/berrange/tags/qcrypto-next-pull-request' into staging (2020-05-07 14:30:12 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/dagrh/qemu.git tags/pull-migration-20200507a
>
> for you to fetch changes up to 13f2cb21e5fb33e9f8d7db8eee48edc1c67b812f:
>
>   migration/multifd: Do error_free after migrate_set_error to avoid memleaks (2020-05-07 17:40:24 +0100)
>
> ----------------------------------------------------------------
> Migration pull 2020-05-07
>
> Mostly tidy-ups, but two new features:
>   cpu-throttle-tailslow for making a gentler throttle
>   xbzrle encoding rate measurement for getting a feal for xbzrle
> performance.
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-05-07 18:51 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 17:01 [PULL 00/12] migration queue Dr. David Alan Gilbert (git)
2020-05-07 17:02 ` [PULL 01/12] migration: fix bad indentation in error_report() Dr. David Alan Gilbert (git)
2020-05-07 17:02 ` [PULL 02/12] migration/migration: improve error reporting for migrate parameters Dr. David Alan Gilbert (git)
2020-05-07 17:02 ` [PULL 03/12] monitor/hmp-cmds: add hmp_handle_error() for hmp_migrate_set_speed() Dr. David Alan Gilbert (git)
2020-05-07 17:02 ` [PULL 04/12] migration: move the units of migrate parameters from milliseconds to ms Dr. David Alan Gilbert (git)
2020-05-07 17:02 ` [PULL 05/12] docs/devel/migration: start a debugging section Dr. David Alan Gilbert (git)
2020-05-07 17:02 ` [PULL 06/12] migration/colo: Add missing error-propagation code Dr. David Alan Gilbert (git)
2020-05-07 17:02 ` [PULL 07/12] migration/throttle: Add cpu-throttle-tailslow migration parameter Dr. David Alan Gilbert (git)
2020-05-07 17:02 ` [PULL 08/12] migration/ram: Consolidate variable reset after placement in ram_load_postcopy() Dr. David Alan Gilbert (git)
2020-05-07 17:02 ` [PULL 09/12] migration/rdma: fix a memleak on error path in rdma_start_incoming_migration Dr. David Alan Gilbert (git)
2020-05-07 17:02 ` [PULL 10/12] migration/xbzrle: add encoding rate Dr. David Alan Gilbert (git)
2020-05-07 17:02 ` [PULL 11/12] migration/multifd: fix memleaks in multifd_new_send_channel_async Dr. David Alan Gilbert (git)
2020-05-07 17:02 ` [PULL 12/12] migration/multifd: Do error_free after migrate_set_error to avoid memleaks Dr. David Alan Gilbert (git)
2020-05-07 18:49 ` [PULL 00/12] migration queue Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).