| Message ID | 20230628215002.73546-6-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration: Better error handling in return path thread | expand |
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>
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.
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)
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(-)