* [PATCH] migration: Stop migration immediately in RDMA error paths
@ 2023-10-24 15:33 Peter Xu
2023-10-24 15:40 ` Thomas Huth
0 siblings, 1 reply; 3+ messages in thread
From: Peter Xu @ 2023-10-24 15:33 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Zhijian Li, Markus Armbruster, Juan Quintela,
Fabiano Rosas, Thomas Huth
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);
+ break;
}
}
--
2.41.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] migration: Stop migration immediately in RDMA error paths
2023-10-24 15:33 [PATCH] migration: Stop migration immediately in RDMA error paths Peter Xu
@ 2023-10-24 15:40 ` Thomas Huth
2023-10-24 15:58 ` Peter Xu
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Huth @ 2023-10-24 15:40 UTC (permalink / raw)
To: Peter Xu, qemu-devel
Cc: Zhijian Li, Markus Armbruster, Juan Quintela, Fabiano Rosas
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;
> }
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] migration: Stop migration immediately in RDMA error paths
2023-10-24 15:40 ` Thomas Huth
@ 2023-10-24 15:58 ` Peter Xu
0 siblings, 0 replies; 3+ messages in thread
From: Peter Xu @ 2023-10-24 15:58 UTC (permalink / raw)
To: Thomas Huth
Cc: qemu-devel, Zhijian Li, Markus Armbruster, Juan Quintela,
Fabiano Rosas
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.
--
Peter Xu
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-10-24 15:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-24 15:33 [PATCH] migration: Stop migration immediately in RDMA error paths Peter Xu
2023-10-24 15:40 ` Thomas Huth
2023-10-24 15:58 ` Peter Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).