[PULL 00/16] Migration 20230505 patches

[PULL 00/16] Migration 20230505 patches

[Juan Quintela]

The following changes since commit f6b761bdbd8ba63cee7428d52fb6b46e4224ddab:

  Merge tag 'qga-pull-2023-05-04' of https://github.com/kostyanf14/qemu into staging (2023-05-04 12:08:00 +0100)

are available in the Git repository at:

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

for you to fetch changes up to fae4009fb51b12927165667a9c9d6af93d31b1df:

  qemu-file: Make ram_control_save_page() use accessors for rate_limit (2023-05-05 02:01:59 +0200)

----------------------------------------------------------------
Migration Pull request (20230505 edition)

In this series:
- fix block_bitmap_mapping (juan)
- RDMA cleanup (juan)
- qemu file cleanup (juan)

Please apply.

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

Juan Quintela (16):
  migration: Fix block_bitmap_mapping migration
  migration/rdma: Don't pass the QIOChannelRDMA as an opaque
  migration: Document all migration_stats
  migration: Put zero_pages in alphabetical order
  migration: Rename xbzrle_enabled xbzrle_started
  migration: Make RAM_SAVE_FLAG_HOOK a normal case entry
  migration/rdma: simplify ram_control_load_hook()
  migration/rdma: We can calculate the rioc from the QEMUFile
  migration/rdma: It makes no sense to recive that flag without RDMA
  migration/rdma: Check for postcopy sooner
  migration: max_postcopy_bandwidth is a size parameter
  migration: qemu_file_total_transferred() function is monotonic
  qemu-file: No need to check for shutdown in qemu_file_rate_limit
  qemu-file: remove shutdown member
  qemu-file: Make total_transferred an uint64_t
  qemu-file: Make ram_control_save_page() use accessors for rate_limit

 migration/block-dirty-bitmap.c | 14 ++++-------
 migration/block.c              | 13 +++-------
 migration/migration-stats.h    | 45 ++++++++++++++++++++++++++++++++-
 migration/migration.c          |  4 +--
 migration/options.c            |  9 ++++++-
 migration/options.h            |  4 ++-
 migration/qemu-file.c          | 35 +++++++-------------------
 migration/qemu-file.h          |  4 +--
 migration/ram.c                | 26 +++++++++----------
 migration/rdma.c               | 46 +++++++++++++++++-----------------
 migration/savevm.c             |  6 ++---
 migration/vmstate.c            |  2 +-
 12 files changed, 114 insertions(+), 94 deletions(-)

-- 
2.40.0

[PULL 01/16] migration: Fix block_bitmap_mapping migration

[Juan Quintela]

It is valid that params->has_block_bitmap_mapping is true and
params->block_bitmap_mapping is NULL.  So we can't use the trick of
having a single function.

Move to two functions one for each value and the tests are fixed.

Fixes: b804b35b1c8a0edfd127ac20819c234be55ac7fc
       migration: Create migrate_block_bitmap_mapping() function

Reported-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20230503181036.14890-1-quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/block-dirty-bitmap.c | 14 +++++---------
 migration/options.c            |  7 +++++++
 migration/options.h            |  2 ++
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 6624f39bc6..20f36e6bd8 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -606,11 +606,9 @@ static int init_dirty_bitmap_migration(DBMSaveState *s)
     GHashTable *handled_by_blk = g_hash_table_new(NULL, NULL);
     BlockBackend *blk;
     GHashTable *alias_map = NULL;
-    const BitmapMigrationNodeAliasList *block_bitmap_mapping =
-        migrate_block_bitmap_mapping();
 
-    if (block_bitmap_mapping) {
-        alias_map = construct_alias_map(block_bitmap_mapping, true,
+    if (migrate_has_block_bitmap_mapping()) {
+        alias_map = construct_alias_map(migrate_block_bitmap_mapping(), true,
                                         &error_abort);
     }
 
@@ -1159,8 +1157,6 @@ static int dirty_bitmap_load_header(QEMUFile *f, DBMLoadState *s,
 static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
 {
     GHashTable *alias_map = NULL;
-    const BitmapMigrationNodeAliasList *block_bitmap_mapping =
-        migrate_block_bitmap_mapping();
     DBMLoadState *s = &((DBMState *)opaque)->load;
     int ret = 0;
 
@@ -1172,9 +1168,9 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
         return -EINVAL;
     }
 
-    if (block_bitmap_mapping) {
-        alias_map = construct_alias_map(block_bitmap_mapping,
-                                        false, &error_abort);
+    if (migrate_has_block_bitmap_mapping()) {
+        alias_map = construct_alias_map(migrate_block_bitmap_mapping(), false,
+                                        &error_abort);
     }
 
     do {
diff --git a/migration/options.c b/migration/options.c
index 53b7fc5d5d..7395787960 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -626,6 +626,13 @@ const BitmapMigrationNodeAliasList *migrate_block_bitmap_mapping(void)
     return s->parameters.block_bitmap_mapping;
 }
 
+bool migrate_has_block_bitmap_mapping(void)
+{
+    MigrationState *s = migrate_get_current();
+
+    return s->parameters.has_block_bitmap_mapping;
+}
+
 bool migrate_block_incremental(void)
 {
     MigrationState *s = migrate_get_current();
diff --git a/migration/options.h b/migration/options.h
index 3c322867cd..09841d6a63 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -71,6 +71,8 @@ bool migrate_cap_set(int cap, bool value, Error **errp);
 /* parameters */
 
 const BitmapMigrationNodeAliasList *migrate_block_bitmap_mapping(void);
+bool migrate_has_block_bitmap_mapping(void);
+
 bool migrate_block_incremental(void);
 uint32_t migrate_checkpoint_delay(void);
 int migrate_compress_level(void);
-- 
2.40.0

[PULL 02/16] migration/rdma: Don't pass the QIOChannelRDMA as an opaque

[Juan Quintela]

We can calculate it from the QEMUFile like the caller.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230503131847.11603-6-quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/rdma.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 7e747b2595..5b82085bd7 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3527,7 +3527,7 @@ static int dest_ram_sort_func(const void *a, const void *b)
  *
  * Keep doing this until the source tells us to stop.
  */
-static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque)
+static int qemu_rdma_registration_handle(QEMUFile *f)
 {
     RDMAControlHeader reg_resp = { .len = sizeof(RDMARegisterResult),
                                .type = RDMA_CONTROL_REGISTER_RESULT,
@@ -3539,7 +3539,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque)
                              };
     RDMAControlHeader blocks = { .type = RDMA_CONTROL_RAM_BLOCKS_RESULT,
                                  .repeat = 1 };
-    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
     RDMAContext *rdma;
     RDMALocalBlocks *local;
     RDMAControlHeader head;
@@ -3852,7 +3852,7 @@ static int rdma_load_hook(QEMUFile *f, uint64_t flags, void *data)
         return rdma_block_notification_handle(rioc, data);
 
     case RAM_CONTROL_HOOK:
-        return qemu_rdma_registration_handle(f, rioc);
+        return qemu_rdma_registration_handle(f);
 
     default:
         /* Shouldn't be called with any other values */
-- 
2.40.0

[PULL 03/16] migration: Document all migration_stats

[Juan Quintela]

Message-Id: <20230504103357.22130-2-quintela@redhat.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration-stats.h | 43 +++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/migration/migration-stats.h b/migration/migration-stats.h
index 149af932d7..50966328b2 100644
--- a/migration/migration-stats.h
+++ b/migration/migration-stats.h
@@ -22,17 +22,60 @@
  * one thread).
  */
 typedef struct {
+    /*
+     * Number of bytes that were dirty last time that we synced with
+     * the guest memory.  We use that to calculate the downtime.  As
+     * the remaining dirty amounts to what we know that is still dirty
+     * since last iteration, not counting what the guest has dirtied
+     * since we synchronized bitmaps.
+     */
     Stat64 dirty_bytes_last_sync;
+    /*
+     * Number of pages dirtied per second.
+     */
     Stat64 dirty_pages_rate;
+    /*
+     * Number of times we have synchronized guest bitmaps.
+     */
     Stat64 dirty_sync_count;
+    /*
+     * Number of times zero copy failed to send any page using zero
+     * copy.
+     */
     Stat64 dirty_sync_missed_zero_copy;
+    /*
+     * Number of bytes sent at migration completion stage while the
+     * guest is stopped.
+     */
     Stat64 downtime_bytes;
+    /*
+     * Number of pages transferred that were full of zeros.
+     */
     Stat64 zero_pages;
+    /*
+     * Number of bytes sent through multifd channels.
+     */
     Stat64 multifd_bytes;
+    /*
+     * Number of pages transferred that were not full of zeros.
+     */
     Stat64 normal_pages;
+    /*
+     * Number of bytes sent during postcopy.
+     */
     Stat64 postcopy_bytes;
+    /*
+     * Number of postcopy page faults that we have handled during
+     * postcopy stage.
+     */
     Stat64 postcopy_requests;
+    /*
+     * Number of bytes sent during precopy stage.
+     */
     Stat64 precopy_bytes;
+    /*
+     * Total number of bytes transferred.
+     */
     Stat64 transferred;
 } MigrationAtomicStats;
 
-- 
2.40.0

[PULL 04/16] migration: Put zero_pages in alphabetical order

[Juan Quintela]

I forgot to move it when I rename it from duplicated_pages.

Message-Id: <20230504103357.22130-3-quintela@redhat.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration-stats.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/migration-stats.h b/migration/migration-stats.h
index 50966328b2..cf8a4f0410 100644
--- a/migration/migration-stats.h
+++ b/migration/migration-stats.h
@@ -48,10 +48,6 @@ typedef struct {
      * guest is stopped.
      */
     Stat64 downtime_bytes;
-    /*
-     * Number of pages transferred that were full of zeros.
-     */
-    Stat64 zero_pages;
     /*
      * Number of bytes sent through multifd channels.
      */
@@ -77,6 +73,10 @@ typedef struct {
      * Total number of bytes transferred.
      */
     Stat64 transferred;
+    /*
+     * Number of pages transferred that were full of zeros.
+     */
+    Stat64 zero_pages;
 } MigrationAtomicStats;
 
 extern MigrationAtomicStats mig_stats;
-- 
2.40.0

[PULL 05/16] migration: Rename xbzrle_enabled xbzrle_started

[Juan Quintela]

Otherwise it is confusing with the function xbzrle_enabled().

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
Message-Id: <20230504115323.24407-1-quintela@redhat.com>
---
 migration/ram.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 7d81c4a39e..23ba1cefff 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -388,8 +388,8 @@ struct RAMState {
     uint64_t xbzrle_pages_prev;
     /* Amount of xbzrle encoded bytes since the beginning of the period */
     uint64_t xbzrle_bytes_prev;
-    /* Start using XBZRLE (e.g., after the first round). */
-    bool xbzrle_enabled;
+    /* Are we really using XBZRLE (e.g., after the first round). */
+    bool xbzrle_started;
     /* Are we on the last stage of migration */
     bool last_stage;
     /* compression statistics since the beginning of the period */
@@ -1420,7 +1420,7 @@ static int ram_save_page(RAMState *rs, PageSearchStatus *pss)
     trace_ram_save_page(block->idstr, (uint64_t)offset, p);
 
     XBZRLE_cache_lock();
-    if (rs->xbzrle_enabled && !migration_in_postcopy()) {
+    if (rs->xbzrle_started && !migration_in_postcopy()) {
         pages = save_xbzrle_page(rs, pss, &p, current_addr,
                                  block, offset);
         if (!rs->last_stage) {
@@ -1636,7 +1636,7 @@ static int find_dirty_block(RAMState *rs, PageSearchStatus *pss)
             pss->complete_round = true;
             /* After the first round, enable XBZRLE. */
             if (migrate_xbzrle()) {
-                rs->xbzrle_enabled = true;
+                rs->xbzrle_started = true;
             }
         }
         /* Didn't find anything this time, but try again on the new block */
@@ -2288,7 +2288,7 @@ static bool save_page_use_compression(RAMState *rs)
      * using the data compression. In theory, xbzrle can do better than
      * compression.
      */
-    if (rs->xbzrle_enabled) {
+    if (rs->xbzrle_started) {
         return false;
     }
 
@@ -2357,7 +2357,7 @@ static int ram_save_target_page_legacy(RAMState *rs, PageSearchStatus *pss)
         /* Must let xbzrle know, otherwise a previous (now 0'd) cached
          * page would be stale
          */
-        if (rs->xbzrle_enabled) {
+        if (rs->xbzrle_started) {
             XBZRLE_cache_lock();
             xbzrle_cache_zero_page(rs, block->offset + offset);
             XBZRLE_cache_unlock();
@@ -2738,7 +2738,7 @@ static void ram_state_reset(RAMState *rs)
     rs->last_seen_block = NULL;
     rs->last_page = 0;
     rs->last_version = ram_list.version;
-    rs->xbzrle_enabled = false;
+    rs->xbzrle_started = false;
 }
 
 #define MAX_WAIT 50 /* ms, half buffered_file limit */
-- 
2.40.0

[PULL 06/16] migration: Make RAM_SAVE_FLAG_HOOK a normal case entry

[Juan Quintela]

Fixes this commit, clearly a bad merge after a rebase or similar, it
should have been its own case since that point.

commit 5b0e9dd46fbda5152566a4a26fd96bc0d0452bf7
Author: Peter Lieven <pl@kamp.de>
Date:   Tue Jun 24 11:32:36 2014 +0200

    migration: catch unknown flag combinations in ram_load

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230504114443.23891-2-quintela@redhat.com>
---
 migration/ram.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 23ba1cefff..5e7bf20ca5 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4445,14 +4445,12 @@ static int ram_load_precopy(QEMUFile *f)
                 multifd_recv_sync_main();
             }
             break;
+        case RAM_SAVE_FLAG_HOOK:
+            ram_control_load_hook(f, RAM_CONTROL_HOOK, NULL);
+            break;
         default:
-            if (flags & RAM_SAVE_FLAG_HOOK) {
-                ram_control_load_hook(f, RAM_CONTROL_HOOK, NULL);
-            } else {
-                error_report("Unknown combination of migration flags: 0x%x",
-                             flags);
-                ret = -EINVAL;
-            }
+            error_report("Unknown combination of migration flags: 0x%x", flags);
+            ret = -EINVAL;
         }
         if (!ret) {
             ret = qemu_file_get_error(f);
-- 
2.40.0

[PULL 07/16] migration/rdma: simplify ram_control_load_hook()

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230504114443.23891-3-quintela@redhat.com>
---
 migration/qemu-file.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index ee04240a21..b7afc8d498 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -340,10 +340,8 @@ void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
 
 void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data)
 {
-    int ret = -EINVAL;
-
     if (f->hooks && f->hooks->hook_ram_load) {
-        ret = f->hooks->hook_ram_load(f, flags, data);
+        int ret = f->hooks->hook_ram_load(f, flags, data);
         if (ret < 0) {
             qemu_file_set_error(f, ret);
         }
@@ -353,7 +351,7 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data)
          * that expects there to be a hook on the destination.
          */
         if (flags == RAM_CONTROL_HOOK) {
-            qemu_file_set_error(f, ret);
+            qemu_file_set_error(f, -EINVAL);
         }
     }
 }
-- 
2.40.0

[PULL 08/16] migration/rdma: We can calculate the rioc from the QEMUFile

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230504114443.23891-4-quintela@redhat.com>
---
 migration/rdma.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 5b82085bd7..17c4b9206f 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3811,9 +3811,10 @@ out:
  * the source.
  */
 static int
-rdma_block_notification_handle(QIOChannelRDMA *rioc, const char *name)
+rdma_block_notification_handle(QEMUFile *f, const char *name)
 {
     RDMAContext *rdma;
+    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
     int curr;
     int found = -1;
 
@@ -3846,10 +3847,9 @@ rdma_block_notification_handle(QIOChannelRDMA *rioc, const char *name)
 
 static int rdma_load_hook(QEMUFile *f, uint64_t flags, void *data)
 {
-    QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
     switch (flags) {
     case RAM_CONTROL_BLOCK_REG:
-        return rdma_block_notification_handle(rioc, data);
+        return rdma_block_notification_handle(f, data);
 
     case RAM_CONTROL_HOOK:
         return qemu_rdma_registration_handle(f);
-- 
2.40.0

[PULL 09/16] migration/rdma: It makes no sense to recive that flag without RDMA

[Juan Quintela]

This could only happen if the source sent
RAM_SAVE_FLAG_HOOK (i.e. rdma) and destination don't have CONFIG_RDMA.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230504114443.23891-5-quintela@redhat.com>
---
 migration/qemu-file.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index b7afc8d498..578b6309ba 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -345,14 +345,6 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data)
         if (ret < 0) {
             qemu_file_set_error(f, ret);
         }
-    } else {
-        /*
-         * Hook is a hook specifically requested by the source sending a flag
-         * that expects there to be a hook on the destination.
-         */
-        if (flags == RAM_CONTROL_HOOK) {
-            qemu_file_set_error(f, -EINVAL);
-        }
     }
 }
 
-- 
2.40.0

[PULL 10/16] migration/rdma: Check for postcopy sooner

[Juan Quintela]

It makes no sense first try to see if there is an rdma error and then
do nothing on postcopy stage.  Change it so we check we are in
postcopy before doing anything.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230504114443.23891-6-quintela@redhat.com>
---
 migration/rdma.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 17c4b9206f..2cd8f1cc66 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3234,19 +3234,19 @@ static size_t qemu_rdma_save_page(QEMUFile *f,
     RDMAContext *rdma;
     int ret;
 
-    RCU_READ_LOCK_GUARD();
-    rdma = qatomic_rcu_read(&rioc->rdmaout);
-
-    if (!rdma) {
-        return -EIO;
-    }
-
-    CHECK_ERROR_STATE();
-
     if (migration_in_postcopy()) {
         return RAM_SAVE_CONTROL_NOT_SUPP;
     }
 
+    RCU_READ_LOCK_GUARD();
+    rdma = qatomic_rcu_read(&rioc->rdmaout);
+
+    if (!rdma) {
+        return -EIO;
+    }
+
+    CHECK_ERROR_STATE();
+
     qemu_fflush(f);
 
     /*
@@ -3866,6 +3866,10 @@ static int qemu_rdma_registration_start(QEMUFile *f,
     QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
     RDMAContext *rdma;
 
+    if (migration_in_postcopy()) {
+        return 0;
+    }
+
     RCU_READ_LOCK_GUARD();
     rdma = qatomic_rcu_read(&rioc->rdmaout);
     if (!rdma) {
@@ -3874,10 +3878,6 @@ static int qemu_rdma_registration_start(QEMUFile *f,
 
     CHECK_ERROR_STATE();
 
-    if (migration_in_postcopy()) {
-        return 0;
-    }
-
     trace_qemu_rdma_registration_start(flags);
     qemu_put_be64(f, RAM_SAVE_FLAG_HOOK);
     qemu_fflush(f);
@@ -3897,6 +3897,10 @@ static int qemu_rdma_registration_stop(QEMUFile *f,
     RDMAControlHeader head = { .len = 0, .repeat = 1 };
     int ret = 0;
 
+    if (migration_in_postcopy()) {
+        return 0;
+    }
+
     RCU_READ_LOCK_GUARD();
     rdma = qatomic_rcu_read(&rioc->rdmaout);
     if (!rdma) {
@@ -3905,10 +3909,6 @@ static int qemu_rdma_registration_stop(QEMUFile *f,
 
     CHECK_ERROR_STATE();
 
-    if (migration_in_postcopy()) {
-        return 0;
-    }
-
     qemu_fflush(f);
     ret = qemu_rdma_drain_cq(f, rdma);
 
-- 
2.40.0

[PULL 11/16] migration: max_postcopy_bandwidth is a size parameter

[Juan Quintela]

So make everything that uses it uint64_t no int64_t.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230504113841.23130-2-quintela@redhat.com>
---
 migration/migration.c | 4 ++--
 migration/options.c   | 2 +-
 migration/options.h   | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index feb5ab7493..232e387109 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2056,7 +2056,7 @@ static int postcopy_start(MigrationState *ms)
     QIOChannelBuffer *bioc;
     QEMUFile *fb;
     int64_t time_at_stop = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
-    int64_t bandwidth = migrate_max_postcopy_bandwidth();
+    uint64_t bandwidth = migrate_max_postcopy_bandwidth();
     bool restart_block = false;
     int cur_state = MIGRATION_STATUS_ACTIVE;
 
@@ -3176,7 +3176,7 @@ fail:
 void migrate_fd_connect(MigrationState *s, Error *error_in)
 {
     Error *local_err = NULL;
-    int64_t rate_limit;
+    uint64_t rate_limit;
     bool resume = s->state == MIGRATION_STATUS_POSTCOPY_PAUSED;
 
     /*
diff --git a/migration/options.c b/migration/options.c
index 7395787960..2e759cc306 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -717,7 +717,7 @@ uint64_t migrate_max_bandwidth(void)
     return s->parameters.max_bandwidth;
 }
 
-int64_t migrate_max_postcopy_bandwidth(void)
+uint64_t migrate_max_postcopy_bandwidth(void)
 {
     MigrationState *s = migrate_get_current();
 
diff --git a/migration/options.h b/migration/options.h
index 09841d6a63..5cca3326d6 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -85,7 +85,7 @@ int migrate_decompress_threads(void);
 uint64_t migrate_downtime_limit(void);
 uint8_t migrate_max_cpu_throttle(void);
 uint64_t migrate_max_bandwidth(void);
-int64_t migrate_max_postcopy_bandwidth(void);
+uint64_t migrate_max_postcopy_bandwidth(void);
 int migrate_multifd_channels(void);
 MultiFDCompression migrate_multifd_compression(void);
 int migrate_multifd_zlib_level(void);
-- 
2.40.0

[PULL 12/16] migration: qemu_file_total_transferred() function is monotonic

[Juan Quintela]

So delta_bytes can only be greater or equal to zero.  Never negative.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230504113841.23130-3-quintela@redhat.com>
---
 migration/block.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 6d532ac7a2..3499f75e37 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -801,13 +801,7 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
 
     qemu_put_be64(f, BLK_MIG_FLAG_EOS);
     delta_bytes = qemu_file_total_transferred(f) - last_bytes;
-    if (delta_bytes > 0) {
-        return 1;
-    } else if (delta_bytes < 0) {
-        return -1;
-    } else {
-        return 0;
-    }
+    return (delta_bytes > 0);
 }
 
 /* Called with iothread lock taken.  */
-- 
2.40.0

[PULL 13/16] qemu-file: No need to check for shutdown in qemu_file_rate_limit

[Juan Quintela]

After calling qemu_file_shutdown() we set the error as -EIO if there
is no another previous error, so no need to check it here.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230504113841.23130-6-quintela@redhat.com>
---
 migration/qemu-file.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 578b6309ba..a4ea808b15 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -732,9 +732,6 @@ int64_t qemu_file_total_transferred(QEMUFile *f)
 
 int qemu_file_rate_limit(QEMUFile *f)
 {
-    if (f->shutdown) {
-        return 1;
-    }
     if (qemu_file_get_error(f)) {
         return 1;
     }
-- 
2.40.0

[PULL 14/16] qemu-file: remove shutdown member

[Juan Quintela]

The first thing that we do after setting the shutdown value is set the
error as -EIO if there is not a previous error.

So this value is redundant.  Just remove it and use
qemu_file_get_error() in the places that it was tested.

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: <20230504113841.23130-7-quintela@redhat.com>
---
 migration/qemu-file.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index a4ea808b15..11b510aa29 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -63,8 +63,6 @@ struct QEMUFile {
 
     int last_error;
     Error *last_error_obj;
-    /* has the file has been shutdown */
-    bool shutdown;
 };
 
 /*
@@ -78,8 +76,6 @@ int qemu_file_shutdown(QEMUFile *f)
 {
     int ret = 0;
 
-    f->shutdown = true;
-
     /*
      * We must set qemufile error before the real shutdown(), otherwise
      * there can be a race window where we thought IO all went though
@@ -294,7 +290,7 @@ void qemu_fflush(QEMUFile *f)
         return;
     }
 
-    if (f->shutdown) {
+    if (qemu_file_get_error(f)) {
         return;
     }
     if (f->iovcnt > 0) {
@@ -397,7 +393,7 @@ static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
     f->buf_index = 0;
     f->buf_size = pending;
 
-    if (f->shutdown) {
+    if (qemu_file_get_error(f)) {
         return 0;
     }
 
@@ -486,7 +482,7 @@ static int add_to_iovec(QEMUFile *f, const uint8_t *buf, size_t size,
     } else {
         if (f->iovcnt >= MAX_IOV_SIZE) {
             /* Should only happen if a previous fflush failed */
-            assert(f->shutdown || !qemu_file_is_writable(f));
+            assert(qemu_file_get_error(f) || !qemu_file_is_writable(f));
             return 1;
         }
         if (may_free) {
-- 
2.40.0

[PULL 15/16] qemu-file: Make total_transferred an uint64_t

[Juan Quintela]

Change all the functions that use it.  It was already passed as
uint64_t.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230504113841.23130-8-quintela@redhat.com>
---
 migration/block.c     | 5 ++---
 migration/qemu-file.c | 8 ++++----
 migration/qemu-file.h | 4 ++--
 migration/savevm.c    | 6 ++----
 migration/vmstate.c   | 2 +-
 5 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 3499f75e37..a37678ce95 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -747,8 +747,7 @@ static int block_save_setup(QEMUFile *f, void *opaque)
 static int block_save_iterate(QEMUFile *f, void *opaque)
 {
     int ret;
-    int64_t last_bytes = qemu_file_total_transferred(f);
-    int64_t delta_bytes;
+    uint64_t last_bytes = qemu_file_total_transferred(f);
 
     trace_migration_block_save("iterate", block_mig_state.submitted,
                                block_mig_state.transferred);
@@ -800,7 +799,7 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
     }
 
     qemu_put_be64(f, BLK_MIG_FLAG_EOS);
-    delta_bytes = qemu_file_total_transferred(f) - last_bytes;
+    uint64_t delta_bytes = qemu_file_total_transferred(f) - last_bytes;
     return (delta_bytes > 0);
 }
 
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 11b510aa29..ccbefc347e 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -51,7 +51,7 @@ struct QEMUFile {
     int64_t rate_limit_used;
 
     /* The sum of bytes transferred on the wire */
-    int64_t total_transferred;
+    uint64_t total_transferred;
 
     int buf_index;
     int buf_size; /* 0 when writing */
@@ -708,9 +708,9 @@ int coroutine_mixed_fn qemu_get_byte(QEMUFile *f)
     return result;
 }
 
-int64_t qemu_file_total_transferred_fast(QEMUFile *f)
+uint64_t qemu_file_total_transferred_fast(QEMUFile *f)
 {
-    int64_t ret = f->total_transferred;
+    uint64_t ret = f->total_transferred;
     int i;
 
     for (i = 0; i < f->iovcnt; i++) {
@@ -720,7 +720,7 @@ int64_t qemu_file_total_transferred_fast(QEMUFile *f)
     return ret;
 }
 
-int64_t qemu_file_total_transferred(QEMUFile *f)
+uint64_t qemu_file_total_transferred(QEMUFile *f)
 {
     qemu_fflush(f);
     return f->total_transferred;
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index d16cd50448..4f26bf6961 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -83,7 +83,7 @@ int qemu_fclose(QEMUFile *f);
  *
  * Returns: the total bytes transferred
  */
-int64_t qemu_file_total_transferred(QEMUFile *f);
+uint64_t qemu_file_total_transferred(QEMUFile *f);
 
 /*
  * qemu_file_total_transferred_fast:
@@ -95,7 +95,7 @@ int64_t qemu_file_total_transferred(QEMUFile *f);
  *
  * Returns: the total bytes transferred and queued
  */
-int64_t qemu_file_total_transferred_fast(QEMUFile *f);
+uint64_t qemu_file_total_transferred_fast(QEMUFile *f);
 
 /*
  * put_buffer without copying the buffer.
diff --git a/migration/savevm.c b/migration/savevm.c
index a9d0a88e62..032044b1d5 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -927,11 +927,9 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se)
 static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se,
                                    JSONWriter *vmdesc)
 {
-    int64_t old_offset, size;
-
-    old_offset = qemu_file_total_transferred_fast(f);
+    uint64_t old_offset = qemu_file_total_transferred_fast(f);
     se->ops->save_state(f, se->opaque);
-    size = qemu_file_total_transferred_fast(f) - old_offset;
+    uint64_t size = qemu_file_total_transferred_fast(f) - old_offset;
 
     if (vmdesc) {
         json_writer_int64(vmdesc, "size", size);
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 83ca4c7d3e..351f56104e 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -349,7 +349,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
             void *first_elem = opaque + field->offset;
             int i, n_elems = vmstate_n_elems(opaque, field);
             int size = vmstate_size(opaque, field);
-            int64_t old_offset, written_bytes;
+            uint64_t old_offset, written_bytes;
             JSONWriter *vmdesc_loop = vmdesc;
 
             trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
-- 
2.40.0

[PULL 16/16] qemu-file: Make ram_control_save_page() use accessors for rate_limit

[Juan Quintela]

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: <20230504113841.23130-9-quintela@redhat.com>
---
 migration/qemu-file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index ccbefc347e..f4cfd05c67 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -352,7 +352,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) {
-            f->rate_limit_used += size;
+            qemu_file_acct_rate_limit(f, size);
         }
 
         if (ret != RAM_SAVE_CONTROL_DELAYED &&
-- 
2.40.0

Re: [PULL 00/16] Migration 20230505 patches

[Richard Henderson]

On 5/5/23 01:47, Juan Quintela wrote:
> The following changes since commit f6b761bdbd8ba63cee7428d52fb6b46e4224ddab:
> 
>    Merge tag 'qga-pull-2023-05-04' ofhttps://github.com/kostyanf14/qemu  into staging (2023-05-04 12:08:00 +0100)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/juan.quintela/qemu.git  tags/migration-20230505-pull-request
> 
> for you to fetch changes up to fae4009fb51b12927165667a9c9d6af93d31b1df:
> 
>    qemu-file: Make ram_control_save_page() use accessors for rate_limit (2023-05-05 02:01:59 +0200)
> 
> ----------------------------------------------------------------
> Migration Pull request (20230505 edition)
> 
> In this series:
> - fix block_bitmap_mapping (juan)
> - RDMA cleanup (juan)
> - qemu file cleanup (juan)
> 
> Please apply.

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


r~