Hi Peter,
On 5/10/23 00:02, Peter Xu wrote:
rp_state.error was a boolean used to show error happened in return path
thread. That's not only duplicating error reporting (migrate_set_error),
but also not good enough in that we only do error_report() and set it to
true, we never can keep a history of the exact error and show it in
query-migrate.
To make this better, a few things done:
- Use error_setg() rather than error_report() across the whole lifecycle
of return path thread, keeping the error in an Error*.
- Use migrate_set_error() to apply that captured error to the global
migration object when error occured in this thread.
- With above, no need to have mark_source_rp_bad(), remove it, alongside
with rp_state.error itself.
Signed-off-by: Peter Xu <pet...@redhat.com>
---
migration/migration.h | 1 -
migration/ram.h | 5 +-
migration/migration.c | 123 ++++++++++++++++++-----------------------
migration/ram.c | 41 +++++++-------
migration/trace-events | 4 +-
5 files changed, 79 insertions(+), 95 deletions(-)
-int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *block)
+int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *block, Error **errp)
{
int ret = -EINVAL;
/* from_dst_file is always valid because we're within rp_thread */
@@ -4193,16 +4194,16 @@ int ram_dirty_bitmap_reload(MigrationState *s, RAMBlock
*block)
ret = qemu_file_get_error(file);
if (ret || size != local_size) {
- error_report("%s: read bitmap failed for ramblock '%s': %d"
- " (size 0x%"PRIx64", got: 0x%"PRIx64")",
- __func__, block->idstr, ret, local_size, size);
+ error_setg(errp, "read bitmap failed for ramblock '%s': %d"
+ " (size 0x%"PRIx64", got: 0x%"PRIx64")",
+ block->idstr, ret, local_size, size);
ret = -EIO;
goto out;
}
if (end_mark != RAMBLOCK_RECV_BITMAP_ENDING) {
- error_report("%s: ramblock '%s' end mark incorrect: 0x%"PRIx64,
- __func__, block->idstr, end_mark);
+ error_setg(errp, "ramblock '%s' end mark incorrect: 0x%"PRIx64,
+ block->idstr, end_mark);
ret = -EINVAL;
goto out;
}
This function returns -EIO/-EINVAL errors, propagated to its 2 callers
- migrate_handle_rp_recv_bitmap()
- migrate_handle_rp_resume_ack()
which are only used in source_return_path_thread() where the return
value is only checked as boolean.
Could we simplify them returning a boolean (which is the pattern with
functions taking an Error** as last parameter)?
Regards,
Phil.
