This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that qemu_loadvm_state_main() must report an error
in errp, in case of failure.
loadvm_process_command also sets the errp object explicitly.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Arun Menon <armenon@redhat.com>
---
migration/colo.c | 5 +++--
migration/savevm.c | 33 ++++++++++++++++++++-------------
migration/savevm.h | 3 ++-
3 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/migration/colo.c b/migration/colo.c
index 0ba22ee76a13e313793f653f43a728e3c433bbc1..a96e4dba15516b71d1b315c736c3b4879ff04e58 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -659,6 +659,7 @@ void migrate_start_colo_process(MigrationState *s)
static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
QEMUFile *fb, QIOChannelBuffer *bioc, Error **errp)
{
+ ERRP_GUARD();
uint64_t total_size;
uint64_t value;
Error *local_err = NULL;
@@ -686,11 +687,11 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
bql_lock();
cpu_synchronize_all_states();
- ret = qemu_loadvm_state_main(mis->from_src_file, mis);
+ ret = qemu_loadvm_state_main(mis->from_src_file, mis, errp);
bql_unlock();
if (ret < 0) {
- error_setg(errp, "Load VM's live state (ram) error");
+ error_prepend(errp, "Load VM's live state (ram) error: ");
return;
}
diff --git a/migration/savevm.c b/migration/savevm.c
index f3b91c8ae0eee6078406081f0bd7f686fed28601..ad3dd9b172afc541f104d2187a79bafa8e380466 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2105,7 +2105,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
qemu_file_set_blocking(f, true);
/* TODO: sanity check that only postcopiable data will be loaded here */
- load_res = qemu_loadvm_state_main(f, mis);
+ load_res = qemu_loadvm_state_main(f, mis, NULL);
/*
* This is tricky, but, mis->from_src_file can change after it
@@ -2407,6 +2407,7 @@ static int loadvm_postcopy_handle_resume(MigrationIncomingState *mis)
*/
static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp)
{
+ ERRP_GUARD();
int ret;
size_t length;
QIOChannelBuffer *bioc;
@@ -2456,9 +2457,9 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp)
qemu_coroutine_yield();
} while (1);
- ret = qemu_loadvm_state_main(packf, mis);
+ ret = qemu_loadvm_state_main(packf, mis, errp);
if (ret < 0) {
- error_setg(errp, "VM state load failed: %d", ret);
+ error_prepend(errp, "Loading VM state failed: %d: ", ret);
}
trace_loadvm_handle_cmd_packaged_main(ret);
qemu_fclose(packf);
@@ -3074,8 +3075,10 @@ static bool postcopy_pause_incoming(MigrationIncomingState *mis)
return true;
}
-int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
+int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
+ Error **errp)
{
+ ERRP_GUARD();
uint8_t section_type;
int ret = 0;
@@ -3083,8 +3086,10 @@ retry:
while (true) {
section_type = qemu_get_byte(f);
- ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst, NULL);
+ ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst, errp);
if (ret) {
+ error_prepend(errp, "Failed to load device state section ID: %d: ",
+ ret);
break;
}
@@ -3105,7 +3110,7 @@ retry:
}
break;
case QEMU_VM_COMMAND:
- ret = loadvm_process_command(f, NULL);
+ ret = loadvm_process_command(f, errp);
trace_qemu_loadvm_state_section_command(ret);
if ((ret < 0) || (ret == LOADVM_QUIT)) {
goto out;
@@ -3115,7 +3120,7 @@ retry:
/* This is the end of migration */
goto out;
default:
- error_report("Unknown savevm section type %d", section_type);
+ error_setg(errp, "Unknown savevm section type %d", section_type);
ret = -EINVAL;
goto out;
}
@@ -3123,6 +3128,9 @@ retry:
out:
if (ret < 0) {
+ if (*errp == NULL) {
+ error_setg(errp, "Loading VM state failed: %d", ret);
+ }
qemu_file_set_error(f, ret);
/* Cancel bitmaps incoming regardless of recovery */
@@ -3143,6 +3151,8 @@ out:
migrate_postcopy_ram() && postcopy_pause_incoming(mis)) {
/* Reset f to point to the newly created channel */
f = mis->from_src_file;
+ error_free(*errp);
+ *errp = NULL;
goto retry;
}
}
@@ -3176,10 +3186,7 @@ int qemu_loadvm_state(QEMUFile *f, Error **errp)
cpu_synchronize_all_pre_loadvm();
- ret = qemu_loadvm_state_main(f, mis);
- if (ret < 0) {
- error_setg(errp, "Load VM state failed: %d", ret);
- }
+ ret = qemu_loadvm_state_main(f, mis, errp);
qemu_event_set(&mis->main_thread_load_event);
trace_qemu_loadvm_state_post_main(ret);
@@ -3260,9 +3267,9 @@ int qemu_load_device_state(QEMUFile *f, Error **errp)
int ret;
/* Load QEMU_VM_SECTION_FULL section */
- ret = qemu_loadvm_state_main(f, mis);
+ ret = qemu_loadvm_state_main(f, mis, errp);
if (ret < 0) {
- error_setg(errp, "Failed to load device state: %d", ret);
+ error_prepend(errp, "Failed to load device state: %d: ", ret);
return ret;
}
diff --git a/migration/savevm.h b/migration/savevm.h
index b12681839f0b1afa3255e45215d99c13a224b19f..c337e3e3d111a7f28a57b90f61e8f70b71803d4e 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -66,7 +66,8 @@ int qemu_save_device_state(QEMUFile *f);
int qemu_loadvm_state(QEMUFile *f, Error **errp);
void qemu_loadvm_state_cleanup(MigrationIncomingState *mis);
-int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
+int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
+ Error **errp);
int qemu_load_device_state(QEMUFile *f, Error **errp);
int qemu_loadvm_approve_switchover(void);
int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
--
2.50.1
On 2025/08/06 3:25, Arun Menon wrote:
> This is an incremental step in converting vmstate loading
> code to report error via Error objects instead of directly
> printing it to console/monitor.
> It is ensured that qemu_loadvm_state_main() must report an error
> in errp, in case of failure.
> loadvm_process_command also sets the errp object explicitly.
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
> migration/colo.c | 5 +++--
> migration/savevm.c | 33 ++++++++++++++++++++-------------
> migration/savevm.h | 3 ++-
> 3 files changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index 0ba22ee76a13e313793f653f43a728e3c433bbc1..a96e4dba15516b71d1b315c736c3b4879ff04e58 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -659,6 +659,7 @@ void migrate_start_colo_process(MigrationState *s)
> static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
> QEMUFile *fb, QIOChannelBuffer *bioc, Error **errp)
> {
> + ERRP_GUARD();
> uint64_t total_size;
> uint64_t value;
> Error *local_err = NULL;
> @@ -686,11 +687,11 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
>
> bql_lock();
> cpu_synchronize_all_states();
> - ret = qemu_loadvm_state_main(mis->from_src_file, mis);
> + ret = qemu_loadvm_state_main(mis->from_src_file, mis, errp);
> bql_unlock();
>
> if (ret < 0) {
> - error_setg(errp, "Load VM's live state (ram) error");
> + error_prepend(errp, "Load VM's live state (ram) error: ");
> return;
> }
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index f3b91c8ae0eee6078406081f0bd7f686fed28601..ad3dd9b172afc541f104d2187a79bafa8e380466 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2105,7 +2105,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
> qemu_file_set_blocking(f, true);
>
> /* TODO: sanity check that only postcopiable data will be loaded here */
> - load_res = qemu_loadvm_state_main(f, mis);
> + load_res = qemu_loadvm_state_main(f, mis, NULL);
>
> /*
> * This is tricky, but, mis->from_src_file can change after it
> @@ -2407,6 +2407,7 @@ static int loadvm_postcopy_handle_resume(MigrationIncomingState *mis)
> */
> static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp)
> {
> + ERRP_GUARD();
> int ret;
> size_t length;
> QIOChannelBuffer *bioc;
> @@ -2456,9 +2457,9 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp)
> qemu_coroutine_yield();
> } while (1);
>
> - ret = qemu_loadvm_state_main(packf, mis);
> + ret = qemu_loadvm_state_main(packf, mis, errp);
> if (ret < 0) {
> - error_setg(errp, "VM state load failed: %d", ret);
> + error_prepend(errp, "Loading VM state failed: %d: ", ret);
> }
> trace_loadvm_handle_cmd_packaged_main(ret);
> qemu_fclose(packf);
> @@ -3074,8 +3075,10 @@ static bool postcopy_pause_incoming(MigrationIncomingState *mis)
> return true;
> }
>
> -int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
> +int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
> + Error **errp)
> {
> + ERRP_GUARD();
> uint8_t section_type;
> int ret = 0;
>
> @@ -3083,8 +3086,10 @@ retry:
> while (true) {
> section_type = qemu_get_byte(f);
>
> - ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst, NULL);
> + ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst, errp);
> if (ret) {
> + error_prepend(errp, "Failed to load device state section ID: %d: ",
> + ret);
> break;
> }
>
> @@ -3105,7 +3110,7 @@ retry:
> }
> break;
> case QEMU_VM_COMMAND:
> - ret = loadvm_process_command(f, NULL);
> + ret = loadvm_process_command(f, errp);
> trace_qemu_loadvm_state_section_command(ret);
> if ((ret < 0) || (ret == LOADVM_QUIT)) {
> goto out;
> @@ -3115,7 +3120,7 @@ retry:
> /* This is the end of migration */
> goto out;
> default:
> - error_report("Unknown savevm section type %d", section_type);
> + error_setg(errp, "Unknown savevm section type %d", section_type);
> ret = -EINVAL;
> goto out;
> }
> @@ -3123,6 +3128,9 @@ retry:
>
> out:
> if (ret < 0) {
> + if (*errp == NULL) {
> + error_setg(errp, "Loading VM state failed: %d", ret);
> + }
> qemu_file_set_error(f, ret);
>
> /* Cancel bitmaps incoming regardless of recovery */
> @@ -3143,6 +3151,8 @@ out:
> migrate_postcopy_ram() && postcopy_pause_incoming(mis)) {
> /* Reset f to point to the newly created channel */
> f = mis->from_src_file;
> + error_free(*errp);
> + *errp = NULL;
error_free_or_abort() is a convenient alternative that can save "*errp =
NULL" and also catch a bug that called functions return a negative value
without setting an error.
> goto retry;
> }
> }
> @@ -3176,10 +3186,7 @@ int qemu_loadvm_state(QEMUFile *f, Error **errp)
>
> cpu_synchronize_all_pre_loadvm();
>
> - ret = qemu_loadvm_state_main(f, mis);
> - if (ret < 0) {
> - error_setg(errp, "Load VM state failed: %d", ret);
> - }
> + ret = qemu_loadvm_state_main(f, mis, errp);
> qemu_event_set(&mis->main_thread_load_event);
>
> trace_qemu_loadvm_state_post_main(ret);
> @@ -3260,9 +3267,9 @@ int qemu_load_device_state(QEMUFile *f, Error **errp)
> int ret;
>
> /* Load QEMU_VM_SECTION_FULL section */
> - ret = qemu_loadvm_state_main(f, mis);
> + ret = qemu_loadvm_state_main(f, mis, errp);
> if (ret < 0) {
> - error_setg(errp, "Failed to load device state: %d", ret);
> + error_prepend(errp, "Failed to load device state: %d: ", ret);
> return ret;
> }
>
> diff --git a/migration/savevm.h b/migration/savevm.h
> index b12681839f0b1afa3255e45215d99c13a224b19f..c337e3e3d111a7f28a57b90f61e8f70b71803d4e 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -66,7 +66,8 @@ int qemu_save_device_state(QEMUFile *f);
>
> int qemu_loadvm_state(QEMUFile *f, Error **errp);
> void qemu_loadvm_state_cleanup(MigrationIncomingState *mis);
> -int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
> +int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
> + Error **errp);
> int qemu_load_device_state(QEMUFile *f, Error **errp);
> int qemu_loadvm_approve_switchover(void);
> int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>
Hi
On Tue, Aug 5, 2025 at 10:29 PM Arun Menon <armenon@redhat.com> wrote:
> This is an incremental step in converting vmstate loading
> code to report error via Error objects instead of directly
> printing it to console/monitor.
> It is ensured that qemu_loadvm_state_main() must report an error
> in errp, in case of failure.
> loadvm_process_command also sets the errp object explicitly.
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
> migration/colo.c | 5 +++--
> migration/savevm.c | 33 ++++++++++++++++++++-------------
> migration/savevm.h | 3 ++-
> 3 files changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index
> 0ba22ee76a13e313793f653f43a728e3c433bbc1..a96e4dba15516b71d1b315c736c3b4879ff04e58
> 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -659,6 +659,7 @@ void migrate_start_colo_process(MigrationState *s)
> static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
> QEMUFile *fb, QIOChannelBuffer *bioc, Error **errp)
> {
> + ERRP_GUARD();
> uint64_t total_size;
> uint64_t value;
> Error *local_err = NULL;
> @@ -686,11 +687,11 @@ static void
> colo_incoming_process_checkpoint(MigrationIncomingState *mis,
>
> bql_lock();
> cpu_synchronize_all_states();
> - ret = qemu_loadvm_state_main(mis->from_src_file, mis);
> + ret = qemu_loadvm_state_main(mis->from_src_file, mis, errp);
> bql_unlock();
>
> if (ret < 0) {
> - error_setg(errp, "Load VM's live state (ram) error");
> + error_prepend(errp, "Load VM's live state (ram) error: ");
> return;
> }
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index
> f3b91c8ae0eee6078406081f0bd7f686fed28601..ad3dd9b172afc541f104d2187a79bafa8e380466
> 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2105,7 +2105,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
> qemu_file_set_blocking(f, true);
>
> /* TODO: sanity check that only postcopiable data will be loaded here
> */
> - load_res = qemu_loadvm_state_main(f, mis);
> + load_res = qemu_loadvm_state_main(f, mis, NULL);
>
What's the rationale to make the function silent or not gather the error?
It seems error reporting could be improved below this. Maybe pass
&error_warn here, and add a second TODO to improve error handling?
>
> /*
> * This is tricky, but, mis->from_src_file can change after it
> @@ -2407,6 +2407,7 @@ static int
> loadvm_postcopy_handle_resume(MigrationIncomingState *mis)
> */
> static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error
> **errp)
> {
> + ERRP_GUARD();
> int ret;
> size_t length;
> QIOChannelBuffer *bioc;
> @@ -2456,9 +2457,9 @@ static int
> loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp)
> qemu_coroutine_yield();
> } while (1);
>
> - ret = qemu_loadvm_state_main(packf, mis);
> + ret = qemu_loadvm_state_main(packf, mis, errp);
> if (ret < 0) {
> - error_setg(errp, "VM state load failed: %d", ret);
> + error_prepend(errp, "Loading VM state failed: %d: ", ret);
> }
> trace_loadvm_handle_cmd_packaged_main(ret);
> qemu_fclose(packf);
> @@ -3074,8 +3075,10 @@ static bool
> postcopy_pause_incoming(MigrationIncomingState *mis)
> return true;
> }
>
> -int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
> +int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
> + Error **errp)
> {
> + ERRP_GUARD();
> uint8_t section_type;
> int ret = 0;
>
> @@ -3083,8 +3086,10 @@ retry:
> while (true) {
> section_type = qemu_get_byte(f);
>
> - ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst,
> NULL);
> + ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst,
> errp);
> if (ret) {
> + error_prepend(errp, "Failed to load device state section ID:
> %d: ",
> + ret);
> break;
> }
>
> @@ -3105,7 +3110,7 @@ retry:
> }
> break;
> case QEMU_VM_COMMAND:
> - ret = loadvm_process_command(f, NULL);
> + ret = loadvm_process_command(f, errp);
> trace_qemu_loadvm_state_section_command(ret);
> if ((ret < 0) || (ret == LOADVM_QUIT)) {
> goto out;
> @@ -3115,7 +3120,7 @@ retry:
> /* This is the end of migration */
> goto out;
> default:
> - error_report("Unknown savevm section type %d", section_type);
> + error_setg(errp, "Unknown savevm section type %d",
> section_type);
> ret = -EINVAL;
> goto out;
> }
> @@ -3123,6 +3128,9 @@ retry:
>
> out:
> if (ret < 0) {
> + if (*errp == NULL) {
> + error_setg(errp, "Loading VM state failed: %d", ret);
> + }
> qemu_file_set_error(f, ret);
>
> /* Cancel bitmaps incoming regardless of recovery */
> @@ -3143,6 +3151,8 @@ out:
> migrate_postcopy_ram() && postcopy_pause_incoming(mis)) {
> /* Reset f to point to the newly created channel */
> f = mis->from_src_file;
> + error_free(*errp);
> + *errp = NULL;
> goto retry;
> }
> }
> @@ -3176,10 +3186,7 @@ int qemu_loadvm_state(QEMUFile *f, Error **errp)
>
> cpu_synchronize_all_pre_loadvm();
>
> - ret = qemu_loadvm_state_main(f, mis);
> - if (ret < 0) {
> - error_setg(errp, "Load VM state failed: %d", ret);
> - }
> + ret = qemu_loadvm_state_main(f, mis, errp);
> qemu_event_set(&mis->main_thread_load_event);
>
> trace_qemu_loadvm_state_post_main(ret);
> @@ -3260,9 +3267,9 @@ int qemu_load_device_state(QEMUFile *f, Error **errp)
> int ret;
>
> /* Load QEMU_VM_SECTION_FULL section */
> - ret = qemu_loadvm_state_main(f, mis);
> + ret = qemu_loadvm_state_main(f, mis, errp);
> if (ret < 0) {
> - error_setg(errp, "Failed to load device state: %d", ret);
> + error_prepend(errp, "Failed to load device state: %d: ", ret);
> return ret;
> }
>
> diff --git a/migration/savevm.h b/migration/savevm.h
> index
> b12681839f0b1afa3255e45215d99c13a224b19f..c337e3e3d111a7f28a57b90f61e8f70b71803d4e
> 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -66,7 +66,8 @@ int qemu_save_device_state(QEMUFile *f);
>
> int qemu_loadvm_state(QEMUFile *f, Error **errp);
> void qemu_loadvm_state_cleanup(MigrationIncomingState *mis);
> -int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
> +int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
> + Error **errp);
> int qemu_load_device_state(QEMUFile *f, Error **errp);
> int qemu_loadvm_approve_switchover(void);
> int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>
> --
> 2.50.1
>
>
Hi Marc-André,
Thanks for the review.
On Wed, Aug 06, 2025 at 11:34:23AM +0400, Marc-André Lureau wrote:
> Hi
>
> On Tue, Aug 5, 2025 at 10:29 PM Arun Menon <armenon@redhat.com> wrote:
>
> > This is an incremental step in converting vmstate loading
> > code to report error via Error objects instead of directly
> > printing it to console/monitor.
> > It is ensured that qemu_loadvm_state_main() must report an error
> > in errp, in case of failure.
> > loadvm_process_command also sets the errp object explicitly.
> >
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Arun Menon <armenon@redhat.com>
> > ---
> > migration/colo.c | 5 +++--
> > migration/savevm.c | 33 ++++++++++++++++++++-------------
> > migration/savevm.h | 3 ++-
> > 3 files changed, 25 insertions(+), 16 deletions(-)
> >
> > diff --git a/migration/colo.c b/migration/colo.c
> > index
> > 0ba22ee76a13e313793f653f43a728e3c433bbc1..a96e4dba15516b71d1b315c736c3b4879ff04e58
> > 100644
> > --- a/migration/colo.c
> > +++ b/migration/colo.c
> > @@ -659,6 +659,7 @@ void migrate_start_colo_process(MigrationState *s)
> > static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
> > QEMUFile *fb, QIOChannelBuffer *bioc, Error **errp)
> > {
> > + ERRP_GUARD();
> > uint64_t total_size;
> > uint64_t value;
> > Error *local_err = NULL;
> > @@ -686,11 +687,11 @@ static void
> > colo_incoming_process_checkpoint(MigrationIncomingState *mis,
> >
> > bql_lock();
> > cpu_synchronize_all_states();
> > - ret = qemu_loadvm_state_main(mis->from_src_file, mis);
> > + ret = qemu_loadvm_state_main(mis->from_src_file, mis, errp);
> > bql_unlock();
> >
> > if (ret < 0) {
> > - error_setg(errp, "Load VM's live state (ram) error");
> > + error_prepend(errp, "Load VM's live state (ram) error: ");
> > return;
> > }
> >
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index
> > f3b91c8ae0eee6078406081f0bd7f686fed28601..ad3dd9b172afc541f104d2187a79bafa8e380466
> > 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -2105,7 +2105,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
> > qemu_file_set_blocking(f, true);
> >
> > /* TODO: sanity check that only postcopiable data will be loaded here
> > */
> > - load_res = qemu_loadvm_state_main(f, mis);
> > + load_res = qemu_loadvm_state_main(f, mis, NULL);
> >
>
> What's the rationale to make the function silent or not gather the error?
> It seems error reporting could be improved below this. Maybe pass
> &error_warn here, and add a second TODO to improve error handling?
There is a separate patch which handles postcopy_ram_listen_thread
- PATCH 22/27
Therefore I left this as is. But yes, we can pass &error_warn here.
>
>
> >
> > /*
> > * This is tricky, but, mis->from_src_file can change after it
> > @@ -2407,6 +2407,7 @@ static int
> > loadvm_postcopy_handle_resume(MigrationIncomingState *mis)
> > */
> > static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error
> > **errp)
> > {
> > + ERRP_GUARD();
> > int ret;
> > size_t length;
> > QIOChannelBuffer *bioc;
> > @@ -2456,9 +2457,9 @@ static int
> > loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp)
> > qemu_coroutine_yield();
> > } while (1);
> >
> > - ret = qemu_loadvm_state_main(packf, mis);
> > + ret = qemu_loadvm_state_main(packf, mis, errp);
> > if (ret < 0) {
> > - error_setg(errp, "VM state load failed: %d", ret);
> > + error_prepend(errp, "Loading VM state failed: %d: ", ret);
> > }
> > trace_loadvm_handle_cmd_packaged_main(ret);
> > qemu_fclose(packf);
> > @@ -3074,8 +3075,10 @@ static bool
> > postcopy_pause_incoming(MigrationIncomingState *mis)
> > return true;
> > }
> >
> > -int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
> > +int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
> > + Error **errp)
> > {
> > + ERRP_GUARD();
> > uint8_t section_type;
> > int ret = 0;
> >
> > @@ -3083,8 +3086,10 @@ retry:
> > while (true) {
> > section_type = qemu_get_byte(f);
> >
> > - ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst,
> > NULL);
> > + ret = qemu_file_get_error_obj_any(f, mis->postcopy_qemufile_dst,
> > errp);
> > if (ret) {
> > + error_prepend(errp, "Failed to load device state section ID:
> > %d: ",
> > + ret);
> > break;
> > }
> >
> > @@ -3105,7 +3110,7 @@ retry:
> > }
> > break;
> > case QEMU_VM_COMMAND:
> > - ret = loadvm_process_command(f, NULL);
> > + ret = loadvm_process_command(f, errp);
> > trace_qemu_loadvm_state_section_command(ret);
> > if ((ret < 0) || (ret == LOADVM_QUIT)) {
> > goto out;
> > @@ -3115,7 +3120,7 @@ retry:
> > /* This is the end of migration */
> > goto out;
> > default:
> > - error_report("Unknown savevm section type %d", section_type);
> > + error_setg(errp, "Unknown savevm section type %d",
> > section_type);
> > ret = -EINVAL;
> > goto out;
> > }
> > @@ -3123,6 +3128,9 @@ retry:
> >
> > out:
> > if (ret < 0) {
> > + if (*errp == NULL) {
> > + error_setg(errp, "Loading VM state failed: %d", ret);
> > + }
> > qemu_file_set_error(f, ret);
> >
> > /* Cancel bitmaps incoming regardless of recovery */
> > @@ -3143,6 +3151,8 @@ out:
> > migrate_postcopy_ram() && postcopy_pause_incoming(mis)) {
> > /* Reset f to point to the newly created channel */
> > f = mis->from_src_file;
> > + error_free(*errp);
> > + *errp = NULL;
> > goto retry;
> > }
> > }
> > @@ -3176,10 +3186,7 @@ int qemu_loadvm_state(QEMUFile *f, Error **errp)
> >
> > cpu_synchronize_all_pre_loadvm();
> >
> > - ret = qemu_loadvm_state_main(f, mis);
> > - if (ret < 0) {
> > - error_setg(errp, "Load VM state failed: %d", ret);
> > - }
> > + ret = qemu_loadvm_state_main(f, mis, errp);
> > qemu_event_set(&mis->main_thread_load_event);
> >
> > trace_qemu_loadvm_state_post_main(ret);
> > @@ -3260,9 +3267,9 @@ int qemu_load_device_state(QEMUFile *f, Error **errp)
> > int ret;
> >
> > /* Load QEMU_VM_SECTION_FULL section */
> > - ret = qemu_loadvm_state_main(f, mis);
> > + ret = qemu_loadvm_state_main(f, mis, errp);
> > if (ret < 0) {
> > - error_setg(errp, "Failed to load device state: %d", ret);
> > + error_prepend(errp, "Failed to load device state: %d: ", ret);
> > return ret;
> > }
> >
> > diff --git a/migration/savevm.h b/migration/savevm.h
> > index
> > b12681839f0b1afa3255e45215d99c13a224b19f..c337e3e3d111a7f28a57b90f61e8f70b71803d4e
> > 100644
> > --- a/migration/savevm.h
> > +++ b/migration/savevm.h
> > @@ -66,7 +66,8 @@ int qemu_save_device_state(QEMUFile *f);
> >
> > int qemu_loadvm_state(QEMUFile *f, Error **errp);
> > void qemu_loadvm_state_cleanup(MigrationIncomingState *mis);
> > -int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
> > +int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
> > + Error **errp);
> > int qemu_load_device_state(QEMUFile *f, Error **errp);
> > int qemu_loadvm_approve_switchover(void);
> > int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> >
> > --
> > 2.50.1
> >
> >
Regards,
Arun
© 2016 - 2025 Red Hat, Inc.