[PULL 00/12] Migration 20230518 patches

[PULL 00/12] Migration 20230518 patches

[Juan Quintela]

The following changes since commit 266ccbb27b3ec6661f22395ec2c41d854c94d761:

  Merge tag 'pull-target-arm-20230518' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2023-05-18 06:08:30 -0700)

are available in the Git repository at:

  https://gitlab.com/juan.quintela/qemu.git tags/migration-20230518-pull-request

for you to fetch changes up to ba9d2cbc01b4e33f9a97edcd77247831a333eac2:

  migration: Fix duplicated included in meson.build (2023-05-18 18:41:53 +0200)

----------------------------------------------------------------
Migration Pull request

Hi

Based on latest reviewed parts of migration:
- Disable colo (vladimir)
- Migration atomic counters (juan)

Please apply.

----------------------------------------------------------------

Juan Quintela (9):
  migration: Don't use INT64_MAX for unlimited rate
  qemu-file: Account for rate_limit usage on qemu_fflush()
  migration: Move rate_limit_max and rate_limit_used to migration_stats
  migration: Move migration_total_bytes() to migration-stats.c
  migration: Add a trace for migration_transferred_bytes
  migration: Use migration_transferred_bytes() to calculate rate_limit
  migration: We don't need the field rate_limit_used anymore
  migration/multifd: Compute transferred bytes correctly
  migration: Fix duplicated included in meson.build

Vladimir Sementsov-Ogievskiy (3):
  configure: add --disable-colo-proxy option
  migration: split migration_incoming_co
  migration: process_incoming_migration_co(): move colo part to colo

 include/migration/colo.h            |  9 ++++-
 include/migration/qemu-file-types.h | 12 +++++-
 migration/migration-stats.h         | 55 ++++++++++++++++++++++++++++
 migration/migration.h               |  9 ++++-
 migration/options.h                 |  7 ----
 migration/qemu-file.h               | 11 ------
 hw/ppc/spapr.c                      |  4 +-
 hw/s390x/s390-stattrib.c            |  2 +-
 migration/block-dirty-bitmap.c      |  2 +-
 migration/block.c                   |  5 ++-
 migration/colo.c                    | 43 ++++++++++++++++++++--
 migration/migration-stats.c         | 51 ++++++++++++++++++++++++++
 migration/migration.c               | 57 ++++++++---------------------
 migration/multifd.c                 | 14 ++++---
 migration/options.c                 |  7 ++--
 migration/qemu-file.c               | 55 ++--------------------------
 migration/ram.c                     |  2 +-
 migration/rdma.c                    |  5 +--
 migration/savevm.c                  |  2 +-
 stubs/colo-compare.c                |  7 ++++
 stubs/colo.c                        |  6 +--
 meson_options.txt                   |  2 +
 migration/meson.build               |  3 +-
 migration/trace-events              |  3 ++
 net/meson.build                     | 13 +++++--
 scripts/meson-buildoptions.sh       |  3 ++
 stubs/meson.build                   |  1 +
 27 files changed, 242 insertions(+), 148 deletions(-)
 create mode 100644 stubs/colo-compare.c

-- 
2.40.1

[PULL 01/12] configure: add --disable-colo-proxy option

[Juan Quintela]

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Add option to not build filter-rewriter and colo-compare when
they are not needed.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Message-Id: <20230515130640.46035-2-vsementsov@yandex-team.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 stubs/colo-compare.c          |  7 +++++++
 meson_options.txt             |  2 ++
 net/meson.build               | 13 ++++++++++---
 scripts/meson-buildoptions.sh |  3 +++
 stubs/meson.build             |  1 +
 5 files changed, 23 insertions(+), 3 deletions(-)
 create mode 100644 stubs/colo-compare.c

diff --git a/stubs/colo-compare.c b/stubs/colo-compare.c
new file mode 100644
index 0000000000..ec726665be
--- /dev/null
+++ b/stubs/colo-compare.c
@@ -0,0 +1,7 @@
+#include "qemu/osdep.h"
+#include "qemu/notify.h"
+#include "net/colo-compare.h"
+
+void colo_compare_cleanup(void)
+{
+}
diff --git a/meson_options.txt b/meson_options.txt
index 11aec2a441..d1b6738c1b 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -293,6 +293,8 @@ option('live_block_migration', type: 'feature', value: 'auto',
        description: 'block migration in the main migration stream')
 option('replication', type: 'feature', value: 'auto',
        description: 'replication support')
+option('colo_proxy', type: 'feature', value: 'auto',
+       description: 'colo-proxy support')
 option('bochs', type: 'feature', value: 'auto',
        description: 'bochs image format support')
 option('cloop', type: 'feature', value: 'auto',
diff --git a/net/meson.build b/net/meson.build
index 87afca3e93..6f4ecde57f 100644
--- a/net/meson.build
+++ b/net/meson.build
@@ -1,13 +1,10 @@
 softmmu_ss.add(files(
   'announce.c',
   'checksum.c',
-  'colo-compare.c',
-  'colo.c',
   'dump.c',
   'eth.c',
   'filter-buffer.c',
   'filter-mirror.c',
-  'filter-rewriter.c',
   'filter.c',
   'hub.c',
   'net-hmp-cmds.c',
@@ -19,6 +16,16 @@ softmmu_ss.add(files(
   'util.c',
 ))
 
+if get_option('replication').allowed() or \
+    get_option('colo_proxy').allowed()
+  softmmu_ss.add(files('colo-compare.c'))
+  softmmu_ss.add(files('colo.c'))
+endif
+
+if get_option('colo_proxy').allowed()
+  softmmu_ss.add(files('filter-rewriter.c'))
+endif
+
 softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
 
 if have_l2tpv3
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 52fb079a60..a66eb31dae 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -86,6 +86,7 @@ meson_options_help() {
   printf "%s\n" '  capstone        Whether and how to find the capstone library'
   printf "%s\n" '  cloop           cloop image format support'
   printf "%s\n" '  cocoa           Cocoa user interface (macOS only)'
+  printf "%s\n" '  colo-proxy      colo-proxy support'
   printf "%s\n" '  coreaudio       CoreAudio sound support'
   printf "%s\n" '  crypto-afalg    Linux AF_ALG crypto backend driver'
   printf "%s\n" '  curl            CURL block device driver'
@@ -245,6 +246,8 @@ _meson_option_parse() {
     --disable-cloop) printf "%s" -Dcloop=disabled ;;
     --enable-cocoa) printf "%s" -Dcocoa=enabled ;;
     --disable-cocoa) printf "%s" -Dcocoa=disabled ;;
+    --enable-colo-proxy) printf "%s" -Dcolo_proxy=enabled ;;
+    --disable-colo-proxy) printf "%s" -Dcolo_proxy=disabled ;;
     --enable-coreaudio) printf "%s" -Dcoreaudio=enabled ;;
     --disable-coreaudio) printf "%s" -Dcoreaudio=disabled ;;
     --enable-coroutine-pool) printf "%s" -Dcoroutine_pool=true ;;
diff --git a/stubs/meson.build b/stubs/meson.build
index 8412cad15f..a56645e2f7 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -46,6 +46,7 @@ stub_ss.add(files('target-monitor-defs.c'))
 stub_ss.add(files('trace-control.c'))
 stub_ss.add(files('uuid.c'))
 stub_ss.add(files('colo.c'))
+stub_ss.add(files('colo-compare.c'))
 stub_ss.add(files('vmstate.c'))
 stub_ss.add(files('vm-stop.c'))
 stub_ss.add(files('win32-kbd-hook.c'))
-- 
2.40.1

[PULL 02/12] migration: split migration_incoming_co

[Juan Quintela]

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Originally, migration_incoming_co was introduced by
25d0c16f625feb3b6
   "migration: Switch to COLO process after finishing loadvm"
to be able to enter from COLO code to one specific yield point, added
by 25d0c16f625feb3b6.

Later in 923709896b1b0
 "migration: poll the cm event for destination qemu"
we reused this variable to wake the migration incoming coroutine from
RDMA code.

That was doubtful idea. Entering coroutines is a very fragile thing:
you should be absolutely sure which yield point you are going to enter.

I don't know how much is it safe to enter during qemu_loadvm_state()
which I think what RDMA want to do. But for sure RDMA shouldn't enter
the special COLO-related yield-point. As well, COLO code doesn't want
to enter during qemu_loadvm_state(), it want to enter it's own specific
yield-point.

As well, when in 8e48ac95865ac97d
 "COLO: Add block replication into colo process" we added
bdrv_invalidate_cache_all() call (now it's called activate_all())
it became possible to enter the migration incoming coroutine during
that call which is wrong too.

So, let't make these things separate and disjoint: loadvm_co for RDMA,
non-NULL during qemu_loadvm_state(), and colo_incoming_co for COLO,
non-NULL only around specific yield.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <20230515130640.46035-3-vsementsov@yandex-team.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.h | 9 ++++++++-
 migration/colo.c      | 4 ++--
 migration/migration.c | 8 ++++++--
 migration/rdma.c      | 5 ++---
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/migration/migration.h b/migration/migration.h
index 7721c7658b..48a46123a0 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -162,8 +162,15 @@ struct MigrationIncomingState {
 
     int state;
 
+    /*
+     * The incoming migration coroutine, non-NULL during qemu_loadvm_state().
+     * Used to wake the migration incoming coroutine from rdma code. How much is
+     * it safe - it's a question.
+     */
+    Coroutine *loadvm_co;
+
     /* The coroutine we should enter (back) after failover */
-    Coroutine *migration_incoming_co;
+    Coroutine *colo_incoming_co;
     QemuSemaphore colo_incoming_sem;
 
     /*
diff --git a/migration/colo.c b/migration/colo.c
index 6c7c313956..a688ac553a 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -145,8 +145,8 @@ static void secondary_vm_do_failover(void)
     qemu_sem_post(&mis->colo_incoming_sem);
 
     /* For Secondary VM, jump to incoming co */
-    if (mis->migration_incoming_co) {
-        qemu_coroutine_enter(mis->migration_incoming_co);
+    if (mis->colo_incoming_co) {
+        qemu_coroutine_enter(mis->colo_incoming_co);
     }
 }
 
diff --git a/migration/migration.c b/migration/migration.c
index 00d8ba8da0..a6f2f6cacd 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -520,12 +520,14 @@ process_incoming_migration_co(void *opaque)
         goto fail;
     }
 
-    mis->migration_incoming_co = qemu_coroutine_self();
     mis->largest_page_size = qemu_ram_pagesize_largest();
     postcopy_state_set(POSTCOPY_INCOMING_NONE);
     migrate_set_state(&mis->state, MIGRATION_STATUS_NONE,
                       MIGRATION_STATUS_ACTIVE);
+
+    mis->loadvm_co = qemu_coroutine_self();
     ret = qemu_loadvm_state(mis->from_src_file);
+    mis->loadvm_co = NULL;
 
     ps = postcopy_state_get();
     trace_process_incoming_migration_co_end(ret, ps);
@@ -566,7 +568,10 @@ process_incoming_migration_co(void *opaque)
 
         qemu_thread_create(&colo_incoming_thread, "COLO incoming",
              colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE);
+
+        mis->colo_incoming_co = qemu_coroutine_self();
         qemu_coroutine_yield();
+        mis->colo_incoming_co = NULL;
 
         qemu_mutex_unlock_iothread();
         /* Wait checkpoint incoming thread exit before free resource */
@@ -578,7 +583,6 @@ process_incoming_migration_co(void *opaque)
 
     mis->bh = qemu_bh_new(process_incoming_migration_bh, mis);
     qemu_bh_schedule(mis->bh);
-    mis->migration_incoming_co = NULL;
     return;
 fail:
     local_err = NULL;
diff --git a/migration/rdma.c b/migration/rdma.c
index 2cd8f1cc66..2e4dcff1c9 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3342,9 +3342,8 @@ static void rdma_cm_poll_handler(void *opaque)
             }
         }
         rdma_ack_cm_event(cm_event);
-
-        if (mis->migration_incoming_co) {
-            qemu_coroutine_enter(mis->migration_incoming_co);
+        if (mis->loadvm_co) {
+            qemu_coroutine_enter(mis->loadvm_co);
         }
         return;
     }
-- 
2.40.1

[PULL 03/12] migration: process_incoming_migration_co(): move colo part to colo

[Juan Quintela]

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Let's make better public interface for COLO: instead of
colo_process_incoming_thread and not trivial logic around creating the
thread let's make simple colo_incoming_co(), hiding implementation from
generic code.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <20230515130640.46035-4-vsementsov@yandex-team.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/migration/colo.h |  9 ++++++++-
 migration/colo.c         | 39 ++++++++++++++++++++++++++++++++++++++-
 migration/migration.c    | 28 ++--------------------------
 stubs/colo.c             |  6 ++----
 4 files changed, 50 insertions(+), 32 deletions(-)

diff --git a/include/migration/colo.h b/include/migration/colo.h
index 7ef315473e..eaac07f26d 100644
--- a/include/migration/colo.h
+++ b/include/migration/colo.h
@@ -28,7 +28,6 @@ bool migration_in_colo_state(void);
 int migration_incoming_enable_colo(void);
 void migration_incoming_disable_colo(void);
 bool migration_incoming_colo_enabled(void);
-void *colo_process_incoming_thread(void *opaque);
 bool migration_incoming_in_colo_state(void);
 
 COLOMode get_colo_mode(void);
@@ -44,5 +43,13 @@ void colo_do_failover(void);
  */
 void colo_checkpoint_delay_set(void);
 
+/*
+ * Starts COLO incoming process. Called from process_incoming_migration_co()
+ * after loading the state.
+ *
+ * Called with BQL locked, may temporary release BQL.
+ */
+int coroutine_fn colo_incoming_co(void);
+
 void colo_shutdown(void);
 #endif
diff --git a/migration/colo.c b/migration/colo.c
index a688ac553a..72f4f7b37e 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -817,7 +817,7 @@ void colo_shutdown(void)
     }
 }
 
-void *colo_process_incoming_thread(void *opaque)
+static void *colo_process_incoming_thread(void *opaque)
 {
     MigrationIncomingState *mis = opaque;
     QEMUFile *fb = NULL;
@@ -918,3 +918,40 @@ out:
     rcu_unregister_thread();
     return NULL;
 }
+
+int coroutine_fn colo_incoming_co(void)
+{
+    MigrationIncomingState *mis = migration_incoming_get_current();
+    Error *local_err = NULL;
+    QemuThread th;
+
+    assert(qemu_mutex_iothread_locked());
+
+    if (!migration_incoming_colo_enabled()) {
+        return 0;
+    }
+
+    /* Make sure all file formats throw away their mutable metadata */
+    bdrv_activate_all(&local_err);
+    if (local_err) {
+        error_report_err(local_err);
+        return -EINVAL;
+    }
+
+    qemu_thread_create(&th, "COLO incoming", colo_process_incoming_thread,
+                       mis, QEMU_THREAD_JOINABLE);
+
+    mis->colo_incoming_co = qemu_coroutine_self();
+    qemu_coroutine_yield();
+    mis->colo_incoming_co = NULL;
+
+    qemu_mutex_unlock_iothread();
+    /* Wait checkpoint incoming thread exit before free resource */
+    qemu_thread_join(&th);
+    qemu_mutex_lock_iothread();
+
+    /* We hold the global iothread lock, so it is safe here */
+    colo_release_ram_cache();
+
+    return 0;
+}
diff --git a/migration/migration.c b/migration/migration.c
index a6f2f6cacd..039bba4804 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -511,7 +511,6 @@ process_incoming_migration_co(void *opaque)
     MigrationIncomingState *mis = migration_incoming_get_current();
     PostcopyState ps;
     int ret;
-    Error *local_err = NULL;
 
     assert(mis->from_src_file);
 
@@ -555,37 +554,14 @@ process_incoming_migration_co(void *opaque)
         goto fail;
     }
 
-    /* we get COLO info, and know if we are in COLO mode */
-    if (migration_incoming_colo_enabled()) {
-        QemuThread colo_incoming_thread;
-
-        /* Make sure all file formats throw away their mutable metadata */
-        bdrv_activate_all(&local_err);
-        if (local_err) {
-            error_report_err(local_err);
-            goto fail;
-        }
-
-        qemu_thread_create(&colo_incoming_thread, "COLO incoming",
-             colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE);
-
-        mis->colo_incoming_co = qemu_coroutine_self();
-        qemu_coroutine_yield();
-        mis->colo_incoming_co = NULL;
-
-        qemu_mutex_unlock_iothread();
-        /* Wait checkpoint incoming thread exit before free resource */
-        qemu_thread_join(&colo_incoming_thread);
-        qemu_mutex_lock_iothread();
-        /* We hold the global iothread lock, so it is safe here */
-        colo_release_ram_cache();
+    if (colo_incoming_co() < 0) {
+        goto fail;
     }
 
     mis->bh = qemu_bh_new(process_incoming_migration_bh, mis);
     qemu_bh_schedule(mis->bh);
     return;
 fail:
-    local_err = NULL;
     migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
                       MIGRATION_STATUS_FAILED);
     qemu_fclose(mis->from_src_file);
diff --git a/stubs/colo.c b/stubs/colo.c
index cf9816d368..f33379d0fd 100644
--- a/stubs/colo.c
+++ b/stubs/colo.c
@@ -10,11 +10,9 @@ void colo_shutdown(void)
 {
 }
 
-void *colo_process_incoming_thread(void *opaque)
+int coroutine_fn colo_incoming_co(void)
 {
-    error_report("Impossible happend: trying to start COLO thread when COLO "
-                 "module is not built in");
-    abort();
+    return 0;
 }
 
 void colo_checkpoint_delay_set(void)
-- 
2.40.1

[PULL 04/12] migration: Don't use INT64_MAX for unlimited rate

[Juan Quintela]

Define and use RATE_LIMIT_DISABLED instead.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Message-Id: <20230515195709.63843-2-quintela@redhat.com>
---
 migration/migration-stats.h | 6 ++++++
 migration/migration.c       | 4 ++--
 migration/qemu-file.c       | 6 +++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/migration/migration-stats.h b/migration/migration-stats.h
index cf8a4f0410..e7f1269769 100644
--- a/migration/migration-stats.h
+++ b/migration/migration-stats.h
@@ -15,6 +15,12 @@
 
 #include "qemu/stats64.h"
 
+/*
+ * If rate_limit_max is 0, there is special code to remove the rate
+ * limit.
+ */
+#define RATE_LIMIT_DISABLED 0
+
 /*
  * These are the ram migration statistic counters.  It is loosely
  * based on MigrationStats.  We change to Stat64 any counter that
diff --git a/migration/migration.c b/migration/migration.c
index 039bba4804..3ceaf29798 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2304,7 +2304,7 @@ static void migration_completion(MigrationState *s)
                  * them if migration fails or is cancelled.
                  */
                 s->block_inactive = !migrate_colo();
-                qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
+                qemu_file_set_rate_limit(s->to_dst_file, RATE_LIMIT_DISABLED);
                 ret = qemu_savevm_state_complete_precopy(s->to_dst_file, false,
                                                          s->block_inactive);
             }
@@ -3048,7 +3048,7 @@ static void *bg_migration_thread(void *opaque)
     rcu_register_thread();
     object_ref(OBJECT(s));
 
-    qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
+    qemu_file_set_rate_limit(s->to_dst_file, RATE_LIMIT_DISABLED);
 
     setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
     /*
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 597054759d..9728002de5 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -27,6 +27,7 @@
 #include "qemu/error-report.h"
 #include "qemu/iov.h"
 #include "migration.h"
+#include "migration-stats.h"
 #include "qemu-file.h"
 #include "trace.h"
 #include "options.h"
@@ -732,7 +733,10 @@ int qemu_file_rate_limit(QEMUFile *f)
     if (qemu_file_get_error(f)) {
         return 1;
     }
-    if (f->rate_limit_max > 0 && f->rate_limit_used > f->rate_limit_max) {
+    if (f->rate_limit_max == RATE_LIMIT_DISABLED) {
+        return 0;
+    }
+    if (f->rate_limit_used > f->rate_limit_max) {
         return 1;
     }
     return 0;
-- 
2.40.1

[PULL 05/12] qemu-file: Account for rate_limit usage on qemu_fflush()

[Juan Quintela]

That is the moment we know we have transferred something.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20230515195709.63843-5-quintela@redhat.com>
---
 migration/qemu-file.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 9728002de5..3d66c5c512 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -302,7 +302,9 @@ void qemu_fflush(QEMUFile *f)
                                    &local_error) < 0) {
             qemu_file_set_error_obj(f, -EIO, local_error);
         } else {
-            f->total_transferred += iov_size(f->iov, f->iovcnt);
+            uint64_t size = iov_size(f->iov, f->iovcnt);
+            qemu_file_acct_rate_limit(f, size);
+            f->total_transferred += size;
         }
 
         qemu_iovec_release_ram(f);
@@ -519,7 +521,6 @@ void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, size_t size,
         return;
     }
 
-    f->rate_limit_used += size;
     add_to_iovec(f, buf, size, may_free);
 }
 
@@ -537,7 +538,6 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size)
             l = size;
         }
         memcpy(f->buf + f->buf_index, buf, l);
-        f->rate_limit_used += l;
         add_buf_to_iovec(f, l);
         if (qemu_file_get_error(f)) {
             break;
@@ -554,7 +554,6 @@ void qemu_put_byte(QEMUFile *f, int v)
     }
 
     f->buf[f->buf_index] = v;
-    f->rate_limit_used++;
     add_buf_to_iovec(f, 1);
 }
 
-- 
2.40.1

[PULL 06/12] migration: Move rate_limit_max and rate_limit_used to migration_stats

[Juan Quintela]

These way we can make them atomic and use this functions from any
place.  I also moved all functions that use rate_limit to
migration-stats.

Functions got renamed, they are not qemu_file anymore.

qemu_file_rate_limit -> migration_rate_exceeded
qemu_file_set_rate_limit -> migration_rate_set
qemu_file_get_rate_limit -> migration_rate_get
qemu_file_reset_rate_limit -> migration_rate_reset
qemu_file_acct_rate_limit -> migration_rate_account.

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
Message-Id: <20230515195709.63843-6-quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/migration/qemu-file-types.h | 12 ++++++-
 migration/migration-stats.h         | 46 +++++++++++++++++++++++++
 migration/options.h                 |  7 ----
 migration/qemu-file.h               | 11 ------
 hw/ppc/spapr.c                      |  4 +--
 hw/s390x/s390-stattrib.c            |  2 +-
 migration/block-dirty-bitmap.c      |  2 +-
 migration/block.c                   |  5 +--
 migration/migration-stats.c         | 44 ++++++++++++++++++++++++
 migration/migration.c               | 14 ++++----
 migration/multifd.c                 |  2 +-
 migration/options.c                 |  7 ++--
 migration/qemu-file.c               | 52 ++---------------------------
 migration/ram.c                     |  2 +-
 migration/savevm.c                  |  2 +-
 migration/meson.build               |  2 +-
 16 files changed, 124 insertions(+), 90 deletions(-)

diff --git a/include/migration/qemu-file-types.h b/include/migration/qemu-file-types.h
index 1436f9ce92..9ba163f333 100644
--- a/include/migration/qemu-file-types.h
+++ b/include/migration/qemu-file-types.h
@@ -165,6 +165,16 @@ size_t coroutine_mixed_fn qemu_get_counted_string(QEMUFile *f, char buf[256]);
 
 void qemu_put_counted_string(QEMUFile *f, const char *name);
 
-int qemu_file_rate_limit(QEMUFile *f);
+/**
+ * migration_rate_exceeded: Check if we have exceeded rate for this interval
+ *
+ * Checks if we have already transferred more data that we are allowed
+ * in the current interval.
+ *
+ * @f: QEMUFile used for main migration channel
+ *
+ * Returns if we should stop sending data for this interval.
+ */
+bool migration_rate_exceeded(QEMUFile *f);
 
 #endif
diff --git a/migration/migration-stats.h b/migration/migration-stats.h
index e7f1269769..7b64dc7cc2 100644
--- a/migration/migration-stats.h
+++ b/migration/migration-stats.h
@@ -15,6 +15,12 @@
 
 #include "qemu/stats64.h"
 
+/*
+ * Amount of time to allocate to each "chunk" of bandwidth-throttled
+ * data.
+ */
+#define BUFFER_DELAY     100
+
 /*
  * If rate_limit_max is 0, there is special code to remove the rate
  * limit.
@@ -75,6 +81,14 @@ typedef struct {
      * Number of bytes sent during precopy stage.
      */
     Stat64 precopy_bytes;
+    /*
+     * Maximum amount of data we can send in a cycle.
+     */
+    Stat64 rate_limit_max;
+    /*
+     * Amount of data we have sent in the current cycle.
+     */
+    Stat64 rate_limit_used;
     /*
      * Total number of bytes transferred.
      */
@@ -87,4 +101,36 @@ typedef struct {
 
 extern MigrationAtomicStats mig_stats;
 
+/**
+ * migration_rate_account: Increase the number of bytes transferred.
+ *
+ * Report on a number of bytes the have been transferred that need to
+ * be applied to the rate limiting calcuations.
+ *
+ * @len: amount of bytes transferred
+ */
+void migration_rate_account(uint64_t len);
+
+/**
+ * migration_rate_get: Get the maximum amount that can be transferred.
+ *
+ * Returns the maximum number of bytes that can be transferred in a cycle.
+ */
+uint64_t migration_rate_get(void);
+
+/**
+ * migration_rate_reset: Reset the rate limit counter.
+ *
+ * This is called when we know we start a new transfer cycle.
+ */
+void migration_rate_reset(void);
+
+/**
+ * migration_rate_set: Set the maximum amount that can be transferred.
+ *
+ * Sets the maximum amount of bytes that can be transferred in one cycle.
+ *
+ * @new_rate: new maximum amount
+ */
+void migration_rate_set(uint64_t new_rate);
 #endif
diff --git a/migration/options.h b/migration/options.h
index 5cca3326d6..45991af3c2 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -17,13 +17,6 @@
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
 
-/* constants */
-
-/* Amount of time to allocate to each "chunk" of bandwidth-throttled
- * data. */
-#define BUFFER_DELAY     100
-#define XFER_LIMIT_RATIO (1000 / BUFFER_DELAY)
-
 /* migration properties */
 
 extern Property migration_properties[];
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index bcc39081f2..e649718492 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -130,17 +130,6 @@ void qemu_file_skip(QEMUFile *f, int size);
  * accounting information tracks the total migration traffic.
  */
 void qemu_file_credit_transfer(QEMUFile *f, size_t size);
-void qemu_file_reset_rate_limit(QEMUFile *f);
-/*
- * qemu_file_acct_rate_limit:
- *
- * Report on a number of bytes the have been transferred
- * out of band from the main file object I/O methods, and
- * need to be applied to the rate limiting calcuations
- */
-void qemu_file_acct_rate_limit(QEMUFile *f, uint64_t len);
-void qemu_file_set_rate_limit(QEMUFile *f, uint64_t new_rate);
-uint64_t qemu_file_get_rate_limit(QEMUFile *f);
 int qemu_file_get_error_obj(QEMUFile *f, Error **errp);
 int qemu_file_get_error_obj_any(QEMUFile *f1, QEMUFile *f2, Error **errp);
 void qemu_file_set_error_obj(QEMUFile *f, int ret, Error *err);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ddc9c7b1a1..1baea16c96 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2166,7 +2166,7 @@ static void htab_save_first_pass(QEMUFile *f, SpaprMachineState *spapr,
                 break;
             }
         }
-    } while ((index < htabslots) && !qemu_file_rate_limit(f));
+    } while ((index < htabslots) && !migration_rate_exceeded(f));
 
     if (index >= htabslots) {
         assert(index == htabslots);
@@ -2237,7 +2237,7 @@ static int htab_save_later_pass(QEMUFile *f, SpaprMachineState *spapr,
             assert(index == htabslots);
             index = 0;
         }
-    } while ((examined < htabslots) && (!qemu_file_rate_limit(f) || final));
+    } while ((examined < htabslots) && (!migration_rate_exceeded(f) || final));
 
     if (index >= htabslots) {
         assert(index == htabslots);
diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
index aed919ad7d..220e845d12 100644
--- a/hw/s390x/s390-stattrib.c
+++ b/hw/s390x/s390-stattrib.c
@@ -209,7 +209,7 @@ static int cmma_save(QEMUFile *f, void *opaque, int final)
         return -ENOMEM;
     }
 
-    while (final ? 1 : qemu_file_rate_limit(f) == 0) {
+    while (final ? 1 : migration_rate_exceeded(f) == 0) {
         reallen = sac->get_stattr(sas, &start_gfn, buflen, buf);
         if (reallen < 0) {
             g_free(buf);
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 20f36e6bd8..032fc5f405 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -706,7 +706,7 @@ static void bulk_phase(QEMUFile *f, DBMSaveState *s, bool limit)
     QSIMPLEQ_FOREACH(dbms, &s->dbms_list, entry) {
         while (!dbms->bulk_completed) {
             bulk_phase_send_chunk(f, s, dbms);
-            if (limit && qemu_file_rate_limit(f)) {
+            if (limit && migration_rate_exceeded(f)) {
                 return;
             }
         }
diff --git a/migration/block.c b/migration/block.c
index 12617b4152..b9580a6c7e 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -23,6 +23,7 @@
 #include "block/dirty-bitmap.h"
 #include "migration/misc.h"
 #include "migration.h"
+#include "migration-stats.h"
 #include "migration/register.h"
 #include "qemu-file.h"
 #include "migration/vmstate.h"
@@ -625,7 +626,7 @@ static int flush_blks(QEMUFile *f)
 
     blk_mig_lock();
     while ((blk = QSIMPLEQ_FIRST(&block_mig_state.blk_list)) != NULL) {
-        if (qemu_file_rate_limit(f)) {
+        if (migration_rate_exceeded(f)) {
             break;
         }
         if (blk->ret < 0) {
@@ -762,7 +763,7 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
     /* control the rate of transfer */
     blk_mig_lock();
     while (block_mig_state.read_done * BLK_MIG_BLOCK_SIZE <
-           qemu_file_get_rate_limit(f) &&
+           migration_rate_get() &&
            block_mig_state.submitted < MAX_PARALLEL_IO &&
            (block_mig_state.submitted + block_mig_state.read_done) <
            MAX_IO_BUFFERS) {
diff --git a/migration/migration-stats.c b/migration/migration-stats.c
index 2f2cea965c..0890220ba5 100644
--- a/migration/migration-stats.c
+++ b/migration/migration-stats.c
@@ -12,6 +12,50 @@
 
 #include "qemu/osdep.h"
 #include "qemu/stats64.h"
+#include "qemu-file.h"
 #include "migration-stats.h"
 
 MigrationAtomicStats mig_stats;
+
+bool migration_rate_exceeded(QEMUFile *f)
+{
+    if (qemu_file_get_error(f)) {
+        return true;
+    }
+
+    uint64_t rate_limit_used = stat64_get(&mig_stats.rate_limit_used);
+    uint64_t rate_limit_max = stat64_get(&mig_stats.rate_limit_max);
+
+    if (rate_limit_max == RATE_LIMIT_DISABLED) {
+        return false;
+    }
+    if (rate_limit_max > 0 && rate_limit_used > rate_limit_max) {
+        return true;
+    }
+    return false;
+}
+
+uint64_t migration_rate_get(void)
+{
+    return stat64_get(&mig_stats.rate_limit_max);
+}
+
+#define XFER_LIMIT_RATIO (1000 / BUFFER_DELAY)
+
+void migration_rate_set(uint64_t limit)
+{
+    /*
+     * 'limit' is per second.  But we check it each BUFER_DELAY miliseconds.
+     */
+    stat64_set(&mig_stats.rate_limit_max, limit / XFER_LIMIT_RATIO);
+}
+
+void migration_rate_reset(void)
+{
+    stat64_set(&mig_stats.rate_limit_used, 0);
+}
+
+void migration_rate_account(uint64_t len)
+{
+    stat64_add(&mig_stats.rate_limit_used, len);
+}
diff --git a/migration/migration.c b/migration/migration.c
index 3ceaf29798..fc8974e274 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2120,7 +2120,7 @@ static int postcopy_start(MigrationState *ms)
      * will notice we're in POSTCOPY_ACTIVE and not actually
      * wrap their state up here
      */
-    qemu_file_set_rate_limit(ms->to_dst_file, bandwidth);
+    migration_rate_set(bandwidth);
     if (migrate_postcopy_ram()) {
         /* Ping just for debugging, helps line traces up */
         qemu_savevm_send_ping(ms->to_dst_file, 2);
@@ -2304,7 +2304,7 @@ static void migration_completion(MigrationState *s)
                  * them if migration fails or is cancelled.
                  */
                 s->block_inactive = !migrate_colo();
-                qemu_file_set_rate_limit(s->to_dst_file, RATE_LIMIT_DISABLED);
+                migration_rate_set(RATE_LIMIT_DISABLED);
                 ret = qemu_savevm_state_complete_precopy(s->to_dst_file, false,
                                                          s->block_inactive);
             }
@@ -2699,7 +2699,7 @@ static void migration_update_counters(MigrationState *s,
             stat64_get(&mig_stats.dirty_bytes_last_sync) / bandwidth;
     }
 
-    qemu_file_reset_rate_limit(s->to_dst_file);
+    migration_rate_reset();
 
     update_iteration_initial_status(s);
 
@@ -2852,7 +2852,7 @@ bool migration_rate_limit(void)
 
     bool urgent = false;
     migration_update_counters(s, now);
-    if (qemu_file_rate_limit(s->to_dst_file)) {
+    if (migration_rate_exceeded(s->to_dst_file)) {
 
         if (qemu_file_get_error(s->to_dst_file)) {
             return false;
@@ -2974,7 +2974,7 @@ static void *migration_thread(void *opaque)
     trace_migration_thread_setup_complete();
 
     while (migration_is_active(s)) {
-        if (urgent || !qemu_file_rate_limit(s->to_dst_file)) {
+        if (urgent || !migration_rate_exceeded(s->to_dst_file)) {
             MigIterateState iter_state = migration_iteration_run(s);
             if (iter_state == MIG_ITERATE_SKIP) {
                 continue;
@@ -3048,7 +3048,7 @@ static void *bg_migration_thread(void *opaque)
     rcu_register_thread();
     object_ref(OBJECT(s));
 
-    qemu_file_set_rate_limit(s->to_dst_file, RATE_LIMIT_DISABLED);
+    migration_rate_set(RATE_LIMIT_DISABLED);
 
     setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
     /*
@@ -3220,7 +3220,7 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
         notifier_list_notify(&migration_state_notifiers, s);
     }
 
-    qemu_file_set_rate_limit(s->to_dst_file, rate_limit);
+    migration_rate_set(rate_limit);
     qemu_file_set_blocking(s->to_dst_file, true);
 
     /*
diff --git a/migration/multifd.c b/migration/multifd.c
index 5c4298eadf..5052091ce2 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -431,7 +431,7 @@ static int multifd_send_pages(QEMUFile *f)
     multifd_send_state->pages = p->pages;
     p->pages = pages;
     transferred = ((uint64_t) pages->num) * p->page_size + p->packet_len;
-    qemu_file_acct_rate_limit(f, transferred);
+    migration_rate_account(transferred);
     qemu_mutex_unlock(&p->mutex);
     stat64_add(&mig_stats.transferred, transferred);
     stat64_add(&mig_stats.multifd_bytes, transferred);
diff --git a/migration/options.c b/migration/options.c
index c2a278ee2d..b62ab30cd5 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -23,6 +23,7 @@
 #include "migration/colo.h"
 #include "migration/misc.h"
 #include "migration.h"
+#include "migration-stats.h"
 #include "qemu-file.h"
 #include "ram.h"
 #include "options.h"
@@ -1242,8 +1243,7 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
     if (params->has_max_bandwidth) {
         s->parameters.max_bandwidth = params->max_bandwidth;
         if (s->to_dst_file && !migration_in_postcopy()) {
-            qemu_file_set_rate_limit(s->to_dst_file,
-                                s->parameters.max_bandwidth);
+            migration_rate_set(s->parameters.max_bandwidth);
         }
     }
 
@@ -1272,8 +1272,7 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
     if (params->has_max_postcopy_bandwidth) {
         s->parameters.max_postcopy_bandwidth = params->max_postcopy_bandwidth;
         if (s->to_dst_file && migration_in_postcopy()) {
-            qemu_file_set_rate_limit(s->to_dst_file,
-                    s->parameters.max_postcopy_bandwidth);
+            migration_rate_set(s->parameters.max_postcopy_bandwidth);
         }
     }
     if (params->has_max_cpu_throttle) {
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 3d66c5c512..9c67b52fe0 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -41,17 +41,6 @@ struct QEMUFile {
     QIOChannel *ioc;
     bool is_writable;
 
-    /*
-     * Maximum amount of data in bytes to transfer during one
-     * rate limiting time window
-     */
-    uint64_t rate_limit_max;
-    /*
-     * Total amount of data in bytes queued for transfer
-     * during this rate limiting time window
-     */
-    uint64_t rate_limit_used;
-
     /* The sum of bytes transferred on the wire */
     uint64_t total_transferred;
 
@@ -303,7 +292,7 @@ void qemu_fflush(QEMUFile *f)
             qemu_file_set_error_obj(f, -EIO, local_error);
         } else {
             uint64_t size = iov_size(f->iov, f->iovcnt);
-            qemu_file_acct_rate_limit(f, size);
+            migration_rate_account(size);
             f->total_transferred += size;
         }
 
@@ -356,7 +345,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
         int ret = f->hooks->save_page(f, block_offset,
                                       offset, size, bytes_sent);
         if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
-            qemu_file_acct_rate_limit(f, size);
+            migration_rate_account(size);
         }
 
         if (ret != RAM_SAVE_CONTROL_DELAYED &&
@@ -727,43 +716,6 @@ uint64_t qemu_file_transferred(QEMUFile *f)
     return f->total_transferred;
 }
 
-int qemu_file_rate_limit(QEMUFile *f)
-{
-    if (qemu_file_get_error(f)) {
-        return 1;
-    }
-    if (f->rate_limit_max == RATE_LIMIT_DISABLED) {
-        return 0;
-    }
-    if (f->rate_limit_used > f->rate_limit_max) {
-        return 1;
-    }
-    return 0;
-}
-
-uint64_t qemu_file_get_rate_limit(QEMUFile *f)
-{
-    return f->rate_limit_max;
-}
-
-void qemu_file_set_rate_limit(QEMUFile *f, uint64_t limit)
-{
-    /*
-     * 'limit' is per second.  But we check it each 100 miliseconds.
-     */
-    f->rate_limit_max = limit / XFER_LIMIT_RATIO;
-}
-
-void qemu_file_reset_rate_limit(QEMUFile *f)
-{
-    f->rate_limit_used = 0;
-}
-
-void qemu_file_acct_rate_limit(QEMUFile *f, uint64_t len)
-{
-    f->rate_limit_used += len;
-}
-
 void qemu_put_be16(QEMUFile *f, unsigned int v)
 {
     qemu_put_byte(f, v >> 8);
diff --git a/migration/ram.c b/migration/ram.c
index f69d8d42b0..15cb15bceb 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3116,7 +3116,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
 
         t0 = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
         i = 0;
-        while ((ret = qemu_file_rate_limit(f)) == 0 ||
+        while ((ret = migration_rate_exceeded(f)) == 0 ||
                postcopy_has_request(rs)) {
             int pages;
 
diff --git a/migration/savevm.c b/migration/savevm.c
index e33788343a..03795ce8dc 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1338,7 +1338,7 @@ int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy)
             !(se->ops->has_postcopy && se->ops->has_postcopy(se->opaque))) {
             continue;
         }
-        if (qemu_file_rate_limit(f)) {
+        if (migration_rate_exceeded(f)) {
             return 0;
         }
         trace_savevm_section_start(se->idstr, se->section_id);
diff --git a/migration/meson.build b/migration/meson.build
index dc8b1daef5..b3d0c537c8 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -1,5 +1,6 @@
 # Files needed by unit tests
 migration_files = files(
+  'migration-stats.c',
   'page_cache.c',
   'xbzrle.c',
   'vmstate-types.c',
@@ -18,7 +19,6 @@ softmmu_ss.add(files(
   'fd.c',
   'global_state.c',
   'migration-hmp-cmds.c',
-  'migration-stats.c',
   'migration.c',
   'multifd.c',
   'multifd-zlib.c',
-- 
2.40.1

[PULL 07/12] migration: Move migration_total_bytes() to migration-stats.c

[Juan Quintela]

Once there rename it to migration_transferred_bytes() and pass a
QEMUFile instead of a migration object.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20230515195709.63843-7-quintela@redhat.com>
---
 migration/migration-stats.h | 11 +++++++++++
 migration/migration-stats.c |  5 +++++
 migration/migration.c       | 13 +++----------
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/migration/migration-stats.h b/migration/migration-stats.h
index 7b64dc7cc2..827ea80c9b 100644
--- a/migration/migration-stats.h
+++ b/migration/migration-stats.h
@@ -133,4 +133,15 @@ void migration_rate_reset(void);
  * @new_rate: new maximum amount
  */
 void migration_rate_set(uint64_t new_rate);
+
+/**
+ * migration_transferred_bytes: Return number of bytes transferred
+ *
+ * @f: QEMUFile used for main migration channel
+ *
+ * Returns how many bytes have we transferred since the beginning of
+ * the migration.  It accounts for bytes sent through any migration
+ * channel, multifd, qemu_file, rdma, ....
+ */
+uint64_t migration_transferred_bytes(QEMUFile *f);
 #endif
diff --git a/migration/migration-stats.c b/migration/migration-stats.c
index 0890220ba5..39e98d6fa9 100644
--- a/migration/migration-stats.c
+++ b/migration/migration-stats.c
@@ -59,3 +59,8 @@ void migration_rate_account(uint64_t len)
 {
     stat64_add(&mig_stats.rate_limit_used, len);
 }
+
+uint64_t migration_transferred_bytes(QEMUFile *f)
+{
+    return qemu_file_transferred(f) + stat64_get(&mig_stats.multifd_bytes);
+}
diff --git a/migration/migration.c b/migration/migration.c
index fc8974e274..952100c8d7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2625,16 +2625,9 @@ static MigThrError migration_detect_error(MigrationState *s)
     }
 }
 
-/* How many bytes have we transferred since the beginning of the migration */
-static uint64_t migration_total_bytes(MigrationState *s)
-{
-    return qemu_file_transferred(s->to_dst_file) +
-        stat64_get(&mig_stats.multifd_bytes);
-}
-
 static void migration_calculate_complete(MigrationState *s)
 {
-    uint64_t bytes = migration_total_bytes(s);
+    uint64_t bytes = migration_transferred_bytes(s->to_dst_file);
     int64_t end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
     int64_t transfer_time;
 
@@ -2660,7 +2653,7 @@ static void update_iteration_initial_status(MigrationState *s)
      * wrong speed calculation.
      */
     s->iteration_start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
-    s->iteration_initial_bytes = migration_total_bytes(s);
+    s->iteration_initial_bytes = migration_transferred_bytes(s->to_dst_file);
     s->iteration_initial_pages = ram_get_total_transferred_pages();
 }
 
@@ -2675,7 +2668,7 @@ static void migration_update_counters(MigrationState *s,
         return;
     }
 
-    current_bytes = migration_total_bytes(s);
+    current_bytes = migration_transferred_bytes(s->to_dst_file);
     transferred = current_bytes - s->iteration_initial_bytes;
     time_spent = current_time - s->iteration_start_time;
     bandwidth = (double)transferred / time_spent;
-- 
2.40.1

[PULL 08/12] migration: Add a trace for migration_transferred_bytes

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20230515195709.63843-8-quintela@redhat.com>
---
 migration/migration-stats.c | 7 ++++++-
 migration/trace-events      | 3 +++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/migration/migration-stats.c b/migration/migration-stats.c
index 39e98d6fa9..feec7d7369 100644
--- a/migration/migration-stats.c
+++ b/migration/migration-stats.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "qemu/stats64.h"
 #include "qemu-file.h"
+#include "trace.h"
 #include "migration-stats.h"
 
 MigrationAtomicStats mig_stats;
@@ -62,5 +63,9 @@ void migration_rate_account(uint64_t len)
 
 uint64_t migration_transferred_bytes(QEMUFile *f)
 {
-    return qemu_file_transferred(f) + stat64_get(&mig_stats.multifd_bytes);
+    uint64_t multifd = stat64_get(&mig_stats.multifd_bytes);
+    uint64_t qemu_file = qemu_file_transferred(f);
+
+    trace_migration_transferred_bytes(qemu_file, multifd);
+    return qemu_file + multifd;
 }
diff --git a/migration/trace-events b/migration/trace-events
index f39818c329..cdaef7a1ea 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -186,6 +186,9 @@ process_incoming_migration_co_end(int ret, int ps) "ret=%d postcopy-state=%d"
 process_incoming_migration_co_postcopy_end_main(void) ""
 postcopy_preempt_enabled(bool value) "%d"
 
+# migration-stats
+migration_transferred_bytes(uint64_t qemu_file, uint64_t multifd) "qemu_file %" PRIu64 " multifd %" PRIu64
+
 # channel.c
 migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p ioctype=%s"
 migration_set_outgoing_channel(void *ioc, const char *ioctype, const char *hostname, void *err)  "ioc=%p ioctype=%s hostname=%s err=%p"
-- 
2.40.1

[PULL 09/12] migration: Use migration_transferred_bytes() to calculate rate_limit

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20230515195709.63843-9-quintela@redhat.com>
---
 migration/migration-stats.h | 8 +++++++-
 migration/migration-stats.c | 7 +++++--
 migration/migration.c       | 2 +-
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/migration/migration-stats.h b/migration/migration-stats.h
index 827ea80c9b..4c4daa2e97 100644
--- a/migration/migration-stats.h
+++ b/migration/migration-stats.h
@@ -81,6 +81,10 @@ typedef struct {
      * Number of bytes sent during precopy stage.
      */
     Stat64 precopy_bytes;
+    /*
+     * Amount of transferred data at the start of current cycle.
+     */
+    Stat64 rate_limit_start;
     /*
      * Maximum amount of data we can send in a cycle.
      */
@@ -122,8 +126,10 @@ uint64_t migration_rate_get(void);
  * migration_rate_reset: Reset the rate limit counter.
  *
  * This is called when we know we start a new transfer cycle.
+ *
+ * @f: QEMUFile used for main migration channel
  */
-void migration_rate_reset(void);
+void migration_rate_reset(QEMUFile *f);
 
 /**
  * migration_rate_set: Set the maximum amount that can be transferred.
diff --git a/migration/migration-stats.c b/migration/migration-stats.c
index feec7d7369..97759a45f3 100644
--- a/migration/migration-stats.c
+++ b/migration/migration-stats.c
@@ -24,7 +24,9 @@ bool migration_rate_exceeded(QEMUFile *f)
         return true;
     }
 
-    uint64_t rate_limit_used = stat64_get(&mig_stats.rate_limit_used);
+    uint64_t rate_limit_start = stat64_get(&mig_stats.rate_limit_start);
+    uint64_t rate_limit_current = migration_transferred_bytes(f);
+    uint64_t rate_limit_used = rate_limit_current - rate_limit_start;
     uint64_t rate_limit_max = stat64_get(&mig_stats.rate_limit_max);
 
     if (rate_limit_max == RATE_LIMIT_DISABLED) {
@@ -51,9 +53,10 @@ void migration_rate_set(uint64_t limit)
     stat64_set(&mig_stats.rate_limit_max, limit / XFER_LIMIT_RATIO);
 }
 
-void migration_rate_reset(void)
+void migration_rate_reset(QEMUFile *f)
 {
     stat64_set(&mig_stats.rate_limit_used, 0);
+    stat64_set(&mig_stats.rate_limit_start, migration_transferred_bytes(f));
 }
 
 void migration_rate_account(uint64_t len)
diff --git a/migration/migration.c b/migration/migration.c
index 952100c8d7..5de7f734b9 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2692,7 +2692,7 @@ static void migration_update_counters(MigrationState *s,
             stat64_get(&mig_stats.dirty_bytes_last_sync) / bandwidth;
     }
 
-    migration_rate_reset();
+    migration_rate_reset(s->to_dst_file);
 
     update_iteration_initial_status(s);
 
-- 
2.40.1

[PULL 10/12] migration: We don't need the field rate_limit_used anymore

[Juan Quintela]

Since previous commit, we calculate how much data we have send with
migration_transferred_bytes() so no need to maintain this counter and
remember to always update it.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20230515195709.63843-10-quintela@redhat.com>
---
 migration/migration-stats.h | 14 --------------
 migration/migration-stats.c |  6 ------
 migration/multifd.c         |  1 -
 migration/qemu-file.c       |  4 ----
 4 files changed, 25 deletions(-)

diff --git a/migration/migration-stats.h b/migration/migration-stats.h
index 4c4daa2e97..ac2260e987 100644
--- a/migration/migration-stats.h
+++ b/migration/migration-stats.h
@@ -89,10 +89,6 @@ typedef struct {
      * Maximum amount of data we can send in a cycle.
      */
     Stat64 rate_limit_max;
-    /*
-     * Amount of data we have sent in the current cycle.
-     */
-    Stat64 rate_limit_used;
     /*
      * Total number of bytes transferred.
      */
@@ -105,16 +101,6 @@ typedef struct {
 
 extern MigrationAtomicStats mig_stats;
 
-/**
- * migration_rate_account: Increase the number of bytes transferred.
- *
- * Report on a number of bytes the have been transferred that need to
- * be applied to the rate limiting calcuations.
- *
- * @len: amount of bytes transferred
- */
-void migration_rate_account(uint64_t len);
-
 /**
  * migration_rate_get: Get the maximum amount that can be transferred.
  *
diff --git a/migration/migration-stats.c b/migration/migration-stats.c
index 97759a45f3..f98c8260be 100644
--- a/migration/migration-stats.c
+++ b/migration/migration-stats.c
@@ -55,15 +55,9 @@ void migration_rate_set(uint64_t limit)
 
 void migration_rate_reset(QEMUFile *f)
 {
-    stat64_set(&mig_stats.rate_limit_used, 0);
     stat64_set(&mig_stats.rate_limit_start, migration_transferred_bytes(f));
 }
 
-void migration_rate_account(uint64_t len)
-{
-    stat64_add(&mig_stats.rate_limit_used, len);
-}
-
 uint64_t migration_transferred_bytes(QEMUFile *f)
 {
     uint64_t multifd = stat64_get(&mig_stats.multifd_bytes);
diff --git a/migration/multifd.c b/migration/multifd.c
index 5052091ce2..aabf9b6d98 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -431,7 +431,6 @@ static int multifd_send_pages(QEMUFile *f)
     multifd_send_state->pages = p->pages;
     p->pages = pages;
     transferred = ((uint64_t) pages->num) * p->page_size + p->packet_len;
-    migration_rate_account(transferred);
     qemu_mutex_unlock(&p->mutex);
     stat64_add(&mig_stats.transferred, transferred);
     stat64_add(&mig_stats.multifd_bytes, transferred);
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 9c67b52fe0..acc282654a 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -292,7 +292,6 @@ void qemu_fflush(QEMUFile *f)
             qemu_file_set_error_obj(f, -EIO, local_error);
         } else {
             uint64_t size = iov_size(f->iov, f->iovcnt);
-            migration_rate_account(size);
             f->total_transferred += size;
         }
 
@@ -344,9 +343,6 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
     if (f->hooks && f->hooks->save_page) {
         int ret = f->hooks->save_page(f, block_offset,
                                       offset, size, bytes_sent);
-        if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
-            migration_rate_account(size);
-        }
 
         if (ret != RAM_SAVE_CONTROL_DELAYED &&
             ret != RAM_SAVE_CONTROL_NOT_SUPP) {
-- 
2.40.1

[PULL 11/12] migration/multifd: Compute transferred bytes correctly

[Juan Quintela]

In the past, we had to put the in the main thread all the operations
related with sizes due to qemu_file not beeing thread safe.  As now
all counters are atomic, we can update the counters just after the
do the write.  As an aditional bonus, we are able to use the right
value for the compression methods.  Right now we were assuming that
there were no compression at all.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Message-Id: <20230515195709.63843-17-quintela@redhat.com>
---
 migration/multifd.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index aabf9b6d98..0bf5958a9c 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -175,6 +175,7 @@ void multifd_register_ops(int method, MultiFDMethods *ops)
 static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
 {
     MultiFDInit_t msg = {};
+    size_t size = sizeof(msg);
     int ret;
 
     msg.magic = cpu_to_be32(MULTIFD_MAGIC);
@@ -182,10 +183,12 @@ static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
     msg.id = p->id;
     memcpy(msg.uuid, &qemu_uuid.data, sizeof(msg.uuid));
 
-    ret = qio_channel_write_all(p->c, (char *)&msg, sizeof(msg), errp);
+    ret = qio_channel_write_all(p->c, (char *)&msg, size, errp);
     if (ret != 0) {
         return -1;
     }
+    stat64_add(&mig_stats.multifd_bytes, size);
+    stat64_add(&mig_stats.transferred, size);
     return 0;
 }
 
@@ -395,7 +398,6 @@ static int multifd_send_pages(QEMUFile *f)
     static int next_channel;
     MultiFDSendParams *p = NULL; /* make happy gcc */
     MultiFDPages_t *pages = multifd_send_state->pages;
-    uint64_t transferred;
 
     if (qatomic_read(&multifd_send_state->exiting)) {
         return -1;
@@ -430,10 +432,7 @@ static int multifd_send_pages(QEMUFile *f)
     p->packet_num = multifd_send_state->packet_num++;
     multifd_send_state->pages = p->pages;
     p->pages = pages;
-    transferred = ((uint64_t) pages->num) * p->page_size + p->packet_len;
     qemu_mutex_unlock(&p->mutex);
-    stat64_add(&mig_stats.transferred, transferred);
-    stat64_add(&mig_stats.multifd_bytes, transferred);
     qemu_sem_post(&p->sem);
 
     return 1;
@@ -715,6 +714,8 @@ static void *multifd_send_thread(void *opaque)
                 if (ret != 0) {
                     break;
                 }
+                stat64_add(&mig_stats.multifd_bytes, p->packet_len);
+                stat64_add(&mig_stats.transferred, p->packet_len);
             } else {
                 /* Send header using the same writev call */
                 p->iov[0].iov_len = p->packet_len;
@@ -727,6 +728,8 @@ static void *multifd_send_thread(void *opaque)
                 break;
             }
 
+            stat64_add(&mig_stats.multifd_bytes, p->next_packet_size);
+            stat64_add(&mig_stats.transferred, p->next_packet_size);
             qemu_mutex_lock(&p->mutex);
             p->pending_job--;
             qemu_mutex_unlock(&p->mutex);
-- 
2.40.1

[PULL 12/12] migration: Fix duplicated included in meson.build

[Juan Quintela]

This is the commint with the merge error (not in the submited patch).

commit 52623f23b0d114837a0d6278180b3e3ae8947117
Author: Lukas Straub <lukasstraub2@web.de>
Date:   Thu Apr 20 11:48:35 2023 +0200

    ram-compress.c: Make target independent

    Make ram-compress.c target independent.

Fixes: 52623f23b0d114837a0d6278180b3e3ae8947117
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Message-Id: <20230509170217.83246-1-quintela@redhat.com>
---
 migration/meson.build | 1 -
 1 file changed, 1 deletion(-)

diff --git a/migration/meson.build b/migration/meson.build
index b3d0c537c8..a8e01e70ae 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -22,7 +22,6 @@ softmmu_ss.add(files(
   'migration.c',
   'multifd.c',
   'multifd-zlib.c',
-  'multifd-zlib.c',
   'ram-compress.c',
   'options.c',
   'postcopy-ram.c',
-- 
2.40.1

Re: [PULL 00/12] Migration 20230518 patches

[Richard Henderson]

On 5/18/23 10:12, Juan Quintela wrote:
> The following changes since commit 266ccbb27b3ec6661f22395ec2c41d854c94d761:
> 
>    Merge tag 'pull-target-arm-20230518' ofhttps://git.linaro.org/people/pmaydell/qemu-arm  into staging (2023-05-18 06:08:30 -0700)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/juan.quintela/qemu.git  tags/migration-20230518-pull-request
> 
> for you to fetch changes up to ba9d2cbc01b4e33f9a97edcd77247831a333eac2:
> 
>    migration: Fix duplicated included in meson.build (2023-05-18 18:41:53 +0200)
> 
> ----------------------------------------------------------------
> Migration Pull request
> 
> Hi
> 
> Based on latest reviewed parts of migration:
> - Disable colo (vladimir)
> - Migration atomic counters (juan)
> 
> Please apply.

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate.


r~

Re: [PULL 09/12] migration: Use migration_transferred_bytes() to calculate rate_limit

[Fiona Ebner]

Am 18.05.23 um 19:13 schrieb Juan Quintela:
> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
> index feec7d7369..97759a45f3 100644
> --- a/migration/migration-stats.c
> +++ b/migration/migration-stats.c
> @@ -24,7 +24,9 @@ bool migration_rate_exceeded(QEMUFile *f)
>          return true;
>      }
>  
> -    uint64_t rate_limit_used = stat64_get(&mig_stats.rate_limit_used);
> +    uint64_t rate_limit_start = stat64_get(&mig_stats.rate_limit_start);
> +    uint64_t rate_limit_current = migration_transferred_bytes(f);
> +    uint64_t rate_limit_used = rate_limit_current - rate_limit_start;
>      uint64_t rate_limit_max = stat64_get(&mig_stats.rate_limit_max);
>  
>      if (rate_limit_max == RATE_LIMIT_DISABLED) {

Hi,
just wanted to let you know that the call to
migration_transferred_bytes(f) here can introduce a huge performance
penalty when taking a snapshot. I ran into the issue when testing
something else, with a single-disk snapshot. Without this call it takes
about two seconds, with the call about two minutes.

Best Regards,
Fiona

Re: [PULL 09/12] migration: Use migration_transferred_bytes() to calculate rate_limit

[Juan Quintela]

Fiona Ebner <f.ebner@proxmox.com> wrote:
> Am 18.05.23 um 19:13 schrieb Juan Quintela:
>> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
>> index feec7d7369..97759a45f3 100644
>> --- a/migration/migration-stats.c
>> +++ b/migration/migration-stats.c
>> @@ -24,7 +24,9 @@ bool migration_rate_exceeded(QEMUFile *f)
>>          return true;
>>      }
>>  
>> -    uint64_t rate_limit_used = stat64_get(&mig_stats.rate_limit_used);
>> +    uint64_t rate_limit_start = stat64_get(&mig_stats.rate_limit_start);
>> +    uint64_t rate_limit_current = migration_transferred_bytes(f);
>> +    uint64_t rate_limit_used = rate_limit_current - rate_limit_start;
>>      uint64_t rate_limit_max = stat64_get(&mig_stats.rate_limit_max);
>>  
>>      if (rate_limit_max == RATE_LIMIT_DISABLED) {
>
> Hi,
> just wanted to let you know that the call to
> migration_transferred_bytes(f) here can introduce a huge performance
> penalty when taking a snapshot. I ran into the issue when testing
> something else, with a single-disk snapshot. Without this call it takes
> about two seconds, with the call about two minutes.

Ouch.

Now that everything is reviewed for that series I can sent the new set
of patches.  As I drop the counter it should just get the speed back.

New series comming that removed rate_limit counter altogether.

Can you take a look after I send it?

Thanks for the report.

And now that we are at it.  How are you testing this?

As you appears to be the bigger user of snapshots (or at least the
louder).  Creating tests/qtest/snapshot-test.c could be a good idea.

1st to check this kind of breakage.
2nd so I be sure that we don't "pesimize" your use case.

Hint, hint.

2 seconds vs 2 minutes.

A more detailed explanation that what are you doing would be great.
I.e. you are taking lots of snapshots by second or what?

Later, Juan.

Re: [PULL 09/12] migration: Use migration_transferred_bytes() to calculate rate_limit

[Fiona Ebner]

Am 26.05.23 um 10:55 schrieb Juan Quintela:
> Fiona Ebner <f.ebner@proxmox.com> wrote:
>> Am 18.05.23 um 19:13 schrieb Juan Quintela:
>>> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
>>> index feec7d7369..97759a45f3 100644
>>> --- a/migration/migration-stats.c
>>> +++ b/migration/migration-stats.c
>>> @@ -24,7 +24,9 @@ bool migration_rate_exceeded(QEMUFile *f)
>>>          return true;
>>>      }
>>>  
>>> -    uint64_t rate_limit_used = stat64_get(&mig_stats.rate_limit_used);
>>> +    uint64_t rate_limit_start = stat64_get(&mig_stats.rate_limit_start);
>>> +    uint64_t rate_limit_current = migration_transferred_bytes(f);
>>> +    uint64_t rate_limit_used = rate_limit_current - rate_limit_start;
>>>      uint64_t rate_limit_max = stat64_get(&mig_stats.rate_limit_max);
>>>  
>>>      if (rate_limit_max == RATE_LIMIT_DISABLED) {
>>
>> Hi,
>> just wanted to let you know that the call to
>> migration_transferred_bytes(f) here can introduce a huge performance
>> penalty when taking a snapshot. I ran into the issue when testing
>> something else, with a single-disk snapshot. Without this call it takes
>> about two seconds, with the call about two minutes.
> 
> Ouch.
> 
> Now that everything is reviewed for that series I can sent the new set
> of patches.  As I drop the counter it should just get the speed back.
> 
> New series comming that removed rate_limit counter altogether.
> 
> Can you take a look after I send it?>

Sure. Please CC me, so I don't forget :)

> Thanks for the report.
> 
> And now that we are at it.  How are you testing this?

I ran into it while testing the "BQL during save setup" patch [0] on (at
that time current) master.

> As you appears to be the bigger user of snapshots (or at least the
> louder).

We do have quite a few users using snapshots, but it's actually not the
upstream snapshot-save, but a custom QMP command based on it and
migration code-wise. We do keep the VM running while taking the
snapshot, and write the state to a new raw file/block device. When it's
ready, the VM is stopped, the management software takes snapshots of the
disks (via QEMU for qcow2/RBD and via the storage layer for other
storages) and then the VM is resumed.

The caveat is that the saved state will be the one right before the
snapshot finished, not the one when the snapshot started (as with
background migration). If you really want to see the historically grown
details: [1].

It's much older than background-snapshot and we probably could switch to
using background-snapshot with some (minor) changes. I do plan to
evaluate that at some point, but haven't had the time yet.

> Creating tests/qtest/snapshot-test.c could be a good idea.

I can try to take a look at adding tests at some point, but I will be
very busy in the near future, because we have releases coming up. So if
anybody else wants to do it, don't wait for me :)

> 1st to check this kind of breakage.
> 2nd so I be sure that we don't "pesimize" your use case.
> 
> Hint, hint.
> 
> 2 seconds vs 2 minutes.
> 
> A more detailed explanation that what are you doing would be great.
> I.e. you are taking lots of snapshots by second or what?

No, it was just a single snapshot on a single drive for a VM with 2GiB
of RAM, nothing special. I also just "measured" it by counting, I
thought exact values won't matter when it's such a big difference.

Here's the command line:

> ./qemu-system-x86_64 \
>   -chardev 'socket,id=qmp,path=/var/run/qemu-server/106.qmp,server=on,wait=off' \
>   -mon 'chardev=qmp,mode=control' \
>   -pidfile /var/run/qemu-server/106.pid \
>   -smp '2,sockets=2,cores=1,maxcpus=2' \
>   -nodefaults \
>   -vnc 'unix:/var/run/qemu-server/106.vnc,password=on' \
>   -m 2048 \
>   -device 'VGA,id=vga,bus=pci.0,addr=0x2' \
>   -device 'virtio-scsi-pci,id=scsihw0,bus=pci.0,addr=0x5' \
>   -drive 'file=/var/lib/vz/images/106/vm-106-disk-1.qcow2,if=none,id=drive-scsi0,format=qcow2,cache=none,aio=io_uring,detect-zeroes=on,node-name=scsi0' \
>   -device 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=100' \
>   -machine 'type=pc'

and QMP params:

> 	    "snapshot-save",
> 	    'job-id' => "save$i",
> 	    tag => 'snap0',
> 	    vmstate => 'scsi0',
> 	    devices => ['scsi0'],


[0]: https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg06541.html
[1]:
https://git.proxmox.com/?p=pve-qemu.git;a=blob;f=debian/patches/pve/0018-PVE-add-savevm-async-for-background-state-snapshots.patch;h=d42d0f02f270934610b1ac90b2813b5ee617427d;hb=bd3c1fa52561e4a307e5b5b37754788408fc75a6

Best Regards,
Fiona

Re: [PULL 09/12] migration: Use migration_transferred_bytes() to calculate rate_limit

[Fiona Ebner]

Am 23.05.23 um 14:31 schrieb Fiona Ebner:
> Am 18.05.23 um 19:13 schrieb Juan Quintela:
>> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
>> index feec7d7369..97759a45f3 100644
>> --- a/migration/migration-stats.c
>> +++ b/migration/migration-stats.c
>> @@ -24,7 +24,9 @@ bool migration_rate_exceeded(QEMUFile *f)
>>          return true;
>>      }
>>  
>> -    uint64_t rate_limit_used = stat64_get(&mig_stats.rate_limit_used);
>> +    uint64_t rate_limit_start = stat64_get(&mig_stats.rate_limit_start);
>> +    uint64_t rate_limit_current = migration_transferred_bytes(f);
>> +    uint64_t rate_limit_used = rate_limit_current - rate_limit_start;
>>      uint64_t rate_limit_max = stat64_get(&mig_stats.rate_limit_max);
>>  
>>      if (rate_limit_max == RATE_LIMIT_DISABLED) {
> 
> Hi,
> just wanted to let you know that the call to
> migration_transferred_bytes(f) here can introduce a huge performance
> penalty when taking a snapshot. I ran into the issue when testing
> something else, with a single-disk snapshot. Without this call it takes
> about two seconds, with the call about two minutes.
> 
> 

Unfortunately, the regression is still in current master and v8.1-rc1.
Did the v2 [0] of the series never make it?

[0]: https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg07708.html

Best Regards,
Fiona