diff mbox series

migration: Stop migration immediately in RDMA error paths

Message ID 20231024153313.500783-1-peterx@redhat.com (mailing list archive)
State New, archived
Headers show
Series migration: Stop migration immediately in RDMA error paths | expand

Commit Message

Peter Xu Oct. 24, 2023, 3:33 p.m. UTC
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) <lizhijian@fujitsu.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Peter Xu <peterx@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-thuth@redhat.com

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

Comments

Thomas Huth Oct. 24, 2023, 3:40 p.m. UTC | #1
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) <lizhijian@fujitsu.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Peter Xu <peterx@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-thuth@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;
>           }
>       }
>
Peter Xu Oct. 24, 2023, 3:58 p.m. UTC | #2
On Tue, Oct 24, 2023 at 05:40:09PM +0200, Thomas Huth wrote:
> > @@ -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?

Yeah.. v2 was sent already, sorry for the noise.
diff mbox series

Patch

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);
+            break;
         }
     }