On Fri, Jul 25, 2025 at 4:20 PM Arun Menon <arme...@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 loadvm_postcopy_handle_advise() must report an error
> in errp, in case of failure.
>
> Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
> Signed-off-by: Arun Menon <arme...@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> migration/savevm.c | 40 +++++++++++++++++++---------------------
> 1 file changed, 19 insertions(+), 21 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index
> 60a055f3e1f248b09e5e5d721f14d2eeafd0a7ad..eb843f4869f7b49a17fe0fb1b0e36db993e4024e
> 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1912,39 +1912,39 @@ enum LoadVMExitCodes {
> * quickly.
> */
> static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
> - uint16_t len)
> + uint16_t len, Error **errp)
> {
> PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_ADVISE);
> uint64_t remote_pagesize_summary, local_pagesize_summary, remote_tps;
> size_t page_size = qemu_target_page_size();
> - Error *local_err = NULL;
>
> trace_loadvm_postcopy_handle_advise();
> if (ps != POSTCOPY_INCOMING_NONE) {
> - error_report("CMD_POSTCOPY_ADVISE in wrong postcopy state (%d)",
> ps);
> + error_setg(errp, "CMD_POSTCOPY_ADVISE in wrong postcopy state
> (%d)",
> + ps);
> return -1;
> }
>
> switch (len) {
> case 0:
> if (migrate_postcopy_ram()) {
> - error_report("RAM postcopy is enabled but have 0 byte
> advise");
> + error_setg(errp, "RAM postcopy is enabled but have 0 byte
> advise");
> return -EINVAL;
> }
> return 0;
> case 8 + 8:
> if (!migrate_postcopy_ram()) {
> - error_report("RAM postcopy is disabled but have 16 byte
> advise");
> + error_setg(errp,
> + "RAM postcopy is disabled but have 16 byte
> advise");
> return -EINVAL;
> }
> break;
> default:
> - error_report("CMD_POSTCOPY_ADVISE invalid length (%d)", len);
> + error_setg(errp, "CMD_POSTCOPY_ADVISE invalid length (%d)", len);
> return -EINVAL;
> }
>
> - if (!postcopy_ram_supported_by_host(mis, &local_err)) {
> - error_report_err(local_err);
> + if (!postcopy_ram_supported_by_host(mis, errp)) {
> postcopy_state_set(POSTCOPY_INCOMING_NONE);
> return -1;
> }
> @@ -1967,9 +1967,10 @@ static int
> loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
> * also fails when passed to an older qemu that doesn't
> * do huge pages.
> */
> - error_report("Postcopy needs matching RAM page sizes (s=%" PRIx64
> - " d=%"
> PRIx64 ")",
> - remote_pagesize_summary, local_pagesize_summary);
> + error_setg(errp,
> + "Postcopy needs matching RAM page sizes "
> + "(s=%" PRIx64 " d=%" PRIx64 ")",
> + remote_pagesize_summary, local_pagesize_summary);
> return -1;
> }
>
> @@ -1979,17 +1980,18 @@ static int
> loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
> * Again, some differences could be dealt with, but for now keep
> it
> * simple.
> */
> - error_report("Postcopy needs matching target page sizes (s=%d
> d=%zd)",
> - (int)remote_tps, page_size);
> + error_setg(errp,
> + "Postcopy needs matching target page sizes (s=%d
> d=%zd)",
> + (int)remote_tps, page_size);
> return -1;
> }
>
> - if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_ADVISE, &local_err)) {
> - error_report_err(local_err);
> + if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_ADVISE, errp)) {
> return -1;
> }
>
> - if (ram_postcopy_incoming_init(mis, NULL) < 0) {
> + if (ram_postcopy_incoming_init(mis, errp) < 0) {
> + error_prepend(errp, "Postcopy RAM incoming init failed: ");
> return -1;
> }
>
> @@ -2617,11 +2619,7 @@ static int loadvm_process_command(QEMUFile *f,
> Error **errp)
> return loadvm_handle_cmd_packaged(mis, errp);
>
> case MIG_CMD_POSTCOPY_ADVISE:
> - ret = loadvm_postcopy_handle_advise(mis, len);
> - if (ret < 0) {
> - error_setg(errp, "Failed to load device state command: %d",
> ret);
> - }
> - return ret;
> + return loadvm_postcopy_handle_advise(mis, len, errp);
>
> case MIG_CMD_POSTCOPY_LISTEN:
> ret = loadvm_postcopy_handle_listen(mis);
>
> --
> 2.50.0
>
>
