Re: [PATCH] migration: Stop migration immediately in RDMA error paths

[Thomas Huth]

On 24/10/2023 17.33, Peter Xu wrote:

In multiple places, RDMA errors are handled in a strange way, where it only
sets qemu_file_set_error() but not stop the migration immediately.

It's not obvious what will happen later if there is already an error.  Make
all such failures stop migration immediately.

Cc: Zhijian Li (Fujitsu) <lizhij...@fujitsu.com>
Cc: Markus Armbruster <arm...@redhat.com>
Cc: Juan Quintela <quint...@redhat.com>
Cc: Fabiano Rosas <faro...@suse.de>
Reported-by: Thomas Huth <th...@redhat.com>
Signed-off-by: Peter Xu <pet...@redhat.com>
---

This patch is based on Thomas's patch:

[PATCH v2] migration/ram: Fix compilation with -Wshadow=local
https://lore.kernel.org/r/20231024092220.55305-1-th...@redhat.com

Above patch should have been queued by both Markus and Juan.
---
  migration/ram.c | 11 +++++++----
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 212add4481..28cb2deffb 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3034,11 +3034,13 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
      ret = rdma_registration_start(f, RAM_CONTROL_SETUP);
      if (ret < 0) {
          qemu_file_set_error(f, ret);
+        return ret;
      }

ret = rdma_registration_stop(f, RAM_CONTROL_SETUP);

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

migration_ops = g_malloc0(sizeof(MigrationOps));

@@ -3104,6 +3106,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
          ret = rdma_registration_start(f, RAM_CONTROL_ROUND);
          if (ret < 0) {
              qemu_file_set_error(f, ret);
+            goto out;
          }

t0 = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);

@@ -3208,8 +3211,6 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
      rs->last_stage = !migration_in_colo_state();

WITH_RCU_READ_LOCK_GUARD() {

-        int rdma_reg_ret;
-
          if (!migration_in_postcopy()) {
              migration_bitmap_sync_precopy(rs, true);
          }
@@ -3217,6 +3218,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
          ret = rdma_registration_start(f, RAM_CONTROL_FINISH);
          if (ret < 0) {
              qemu_file_set_error(f, ret);
+            break;
          }

/* try transferring iterative blocks of memory */

@@ -3240,9 +3242,10 @@ static int ram_save_complete(QEMUFile *f, void *opaque)

ram_flush_compressed_data(rs); - rdma_reg_ret = rdma_registration_stop(f, RAM_CONTROL_FINISH);

-        if (rdma_reg_ret < 0) {
+        ret = rdma_registration_stop(f, RAM_CONTROL_FINISH);
+        if (ret < 0) {
              qemu_file_set_error(f, rdma_reg_ret);

I think you have to replace rdma_reg_ret here with ret again?

 Thomas

+            break;
          }
      }