1. Patchew
  2. QEMU
  3. [PATCH v6 0/5] migration: do not exit on incoming failure
  4. View patch

[PATCH v6 4/5] migration: process_incoming_migration_co(): rework error reporting

Vladimir Sementsov-Ogievskiy posted 5 patches 1 year, 9 months ago
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
[PATCH v6 4/5] migration: process_incoming_migration_co(): rework error reporting
Posted by Vladimir Sementsov-Ogievskiy 1 year, 9 months ago 
Unify error reporting in the function. This simplifies the following
commit, which will not-exit-on-error behavior variant to the function.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 migration/migration.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index b307a4bc59..a9599838e6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -735,14 +735,16 @@ static void process_incoming_migration_bh(void *opaque)
 static void coroutine_fn
 process_incoming_migration_co(void *opaque)
 {
+    MigrationState *s = migrate_get_current();
     MigrationIncomingState *mis = migration_incoming_get_current();
     PostcopyState ps;
     int ret;
+    Error *local_err = NULL;
 
     assert(mis->from_src_file);
 
     if (compress_threads_load_setup(mis->from_src_file)) {
-        error_report("Failed to setup decompress threads");
+        error_setg(&local_err, "Failed to setup decompress threads");
         goto fail;
     }
 
@@ -779,19 +781,12 @@ process_incoming_migration_co(void *opaque)
     }
 
     if (ret < 0) {
-        MigrationState *s = migrate_get_current();
-
-        if (migrate_has_error(s)) {
-            WITH_QEMU_LOCK_GUARD(&s->error_mutex) {
-                error_report_err(s->error);
-                s->error = NULL;
-            }
-        }
-        error_report("load of migration failed: %s", strerror(-ret));
+        error_setg(&local_err, "load of migration failed: %s", strerror(-ret));
         goto fail;
     }
 
     if (colo_incoming_co() < 0) {
+        error_setg(&local_err, "colo incoming failed");
         goto fail;
     }
 
@@ -800,8 +795,16 @@ process_incoming_migration_co(void *opaque)
 fail:
     migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
                       MIGRATION_STATUS_FAILED);
+    migrate_set_error(s, local_err);
+    error_free(local_err);
+
     migration_incoming_state_destroy();
 
+    WITH_QEMU_LOCK_GUARD(&s->error_mutex) {
+        error_report_err(s->error);
+        s->error = NULL;
+    }
+
     exit(EXIT_FAILURE);
 }
 
-- 
2.34.1
Re: [PATCH v6 4/5] migration: process_incoming_migration_co(): rework error reporting
Posted by Philippe Mathieu-Daudé 1 year, 9 months ago 
On 30/4/24 10:56, Vladimir Sementsov-Ogievskiy wrote:
> Unify error reporting in the function. This simplifies the following
> commit, which will not-exit-on-error behavior variant to the function.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>   migration/migration.c | 23 +++++++++++++----------
>   1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index b307a4bc59..a9599838e6 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -735,14 +735,16 @@ static void process_incoming_migration_bh(void *opaque)
>   static void coroutine_fn
>   process_incoming_migration_co(void *opaque)
>   {
> +    MigrationState *s = migrate_get_current();

(see below)

>       MigrationIncomingState *mis = migration_incoming_get_current();
>       PostcopyState ps;
>       int ret;
> +    Error *local_err = NULL;
>   
>       assert(mis->from_src_file);
>   
>       if (compress_threads_load_setup(mis->from_src_file)) {
> -        error_report("Failed to setup decompress threads");
> +        error_setg(&local_err, "Failed to setup decompress threads");
>           goto fail;
>       }
>   
> @@ -779,19 +781,12 @@ process_incoming_migration_co(void *opaque)
>       }
>   
>       if (ret < 0) {
> -        MigrationState *s = migrate_get_current();
> -
> -        if (migrate_has_error(s)) {
> -            WITH_QEMU_LOCK_GUARD(&s->error_mutex) {
> -                error_report_err(s->error);
> -                s->error = NULL;
> -            }
> -        }
> -        error_report("load of migration failed: %s", strerror(-ret));
> +        error_setg(&local_err, "load of migration failed: %s", strerror(-ret));
>           goto fail;
>       }
>   
>       if (colo_incoming_co() < 0) {
> +        error_setg(&local_err, "colo incoming failed");
>           goto fail;
>       }
>   
> @@ -800,8 +795,16 @@ process_incoming_migration_co(void *opaque)
>   fail:

Maybe just assign @s in error path here?

        s = migrate_get_current();

>       migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>                         MIGRATION_STATUS_FAILED);
> +    migrate_set_error(s, local_err);
> +    error_free(local_err);
> +
>       migration_incoming_state_destroy();
>   
> +    WITH_QEMU_LOCK_GUARD(&s->error_mutex) {
> +        error_report_err(s->error);
> +        s->error = NULL;
> +    }
> +
>       exit(EXIT_FAILURE);
>   }
>   

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Re: [PATCH v6 4/5] migration: process_incoming_migration_co(): rework error reporting
Posted by Vladimir Sementsov-Ogievskiy 1 year, 9 months ago 
On 30.04.24 12:16, Philippe Mathieu-Daudé wrote:
> On 30/4/24 10:56, Vladimir Sementsov-Ogievskiy wrote:
>> Unify error reporting in the function. This simplifies the following
>> commit, which will not-exit-on-error behavior variant to the function.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   migration/migration.c | 23 +++++++++++++----------
>>   1 file changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index b307a4bc59..a9599838e6 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -735,14 +735,16 @@ static void process_incoming_migration_bh(void *opaque)
>>   static void coroutine_fn
>>   process_incoming_migration_co(void *opaque)
>>   {
>> +    MigrationState *s = migrate_get_current();
> 
> (see below)
> 
>>       MigrationIncomingState *mis = migration_incoming_get_current();
>>       PostcopyState ps;
>>       int ret;
>> +    Error *local_err = NULL;
>>       assert(mis->from_src_file);
>>       if (compress_threads_load_setup(mis->from_src_file)) {
>> -        error_report("Failed to setup decompress threads");
>> +        error_setg(&local_err, "Failed to setup decompress threads");
>>           goto fail;
>>       }
>> @@ -779,19 +781,12 @@ process_incoming_migration_co(void *opaque)
>>       }
>>       if (ret < 0) {
>> -        MigrationState *s = migrate_get_current();
>> -
>> -        if (migrate_has_error(s)) {
>> -            WITH_QEMU_LOCK_GUARD(&s->error_mutex) {
>> -                error_report_err(s->error);
>> -                s->error = NULL;
>> -            }
>> -        }
>> -        error_report("load of migration failed: %s", strerror(-ret));
>> +        error_setg(&local_err, "load of migration failed: %s", strerror(-ret));
>>           goto fail;
>>       }
>>       if (colo_incoming_co() < 0) {
>> +        error_setg(&local_err, "colo incoming failed");
>>           goto fail;
>>       }
>> @@ -800,8 +795,16 @@ process_incoming_migration_co(void *opaque)
>>   fail:
> 
> Maybe just assign @s in error path here?
> 
>         s = migrate_get_current();

I'd keep as is. If continue improving the function, I'd better split the logic to seperate function with classic "Error **errp" argument. And keep reporting error in caller.

> 
>>       migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>>                         MIGRATION_STATUS_FAILED);
>> +    migrate_set_error(s, local_err);
>> +    error_free(local_err);
>> +
>>       migration_incoming_state_destroy();
>> +    WITH_QEMU_LOCK_GUARD(&s->error_mutex) {
>> +        error_report_err(s->error);
>> +        s->error = NULL;
>> +    }
>> +
>>       exit(EXIT_FAILURE);
>>   }
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 

-- 
Best regards,
Vladimir

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.