[PATCH 5/7] migration: Display error in query-migrate irrelevant of status

Peter Xu posted 7 patches 2 years, 7 months ago
There is a newer version of this series
[PATCH 5/7] migration: Display error in query-migrate irrelevant of status
Posted by Peter Xu 2 years, 7 months ago
Display it as long as being set, irrelevant of FAILED status.  E.g., it may
also be applicable to PAUSED stage of postcopy, to provide hint on what has
gone wrong.

The error_mutex seems to be overlooked when referencing the error, add it
to be very safe.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 234dd3601d..7455353918 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1033,9 +1033,6 @@ static void fill_source_migration_info(MigrationInfo *info)
         break;
     case MIGRATION_STATUS_FAILED:
         info->has_status = true;
-        if (s->error) {
-            info->error_desc = g_strdup(error_get_pretty(s->error));
-        }
         break;
     case MIGRATION_STATUS_CANCELLED:
         info->has_status = true;
@@ -1045,6 +1042,11 @@ static void fill_source_migration_info(MigrationInfo *info)
         break;
     }
     info->status = state;
+
+    QEMU_LOCK_GUARD(&s->error_mutex);
+    if (s->error) {
+        info->error_desc = g_strdup(error_get_pretty(s->error));
+    }
 }
 
 static void fill_destination_migration_info(MigrationInfo *info)
-- 
2.41.0
Re: [PATCH 5/7] migration: Display error in query-migrate irrelevant of status
Posted by Fabiano Rosas 2 years, 7 months ago
Peter Xu <peterx@redhat.com> writes:

> Display it as long as being set, irrelevant of FAILED status.  E.g., it may
> also be applicable to PAUSED stage of postcopy, to provide hint on what has
> gone wrong.

This might have made the documentation slightly inaccurate:

# @error-desc: the human readable error description string, when
#     @status is 'failed'. Clients should not attempt to parse the
#     error strings.  (Since 2.7)

But it's not wrong, so:

Reviewed-by: Fabiano Rosas <farosas@suse.de>
Re: [PATCH 5/7] migration: Display error in query-migrate irrelevant of status
Posted by Peter Xu 2 years, 7 months ago
On Wed, Jun 28, 2023 at 08:01:22PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > Display it as long as being set, irrelevant of FAILED status.  E.g., it may
> > also be applicable to PAUSED stage of postcopy, to provide hint on what has
> > gone wrong.
> 
> This might have made the documentation slightly inaccurate:

Hmm yes, maybe I should touch that up so as to include "postcopy-paused",
or just remove the statement that it must be in a "failed" stage.

> 
> # @error-desc: the human readable error description string, when
> #     @status is 'failed'. Clients should not attempt to parse the
> #     error strings.  (Since 2.7)
> 
> But it's not wrong, so:
> 
> Reviewed-by: Fabiano Rosas <farosas@suse.de>

Thanks for taking a look.

-- 
Peter Xu