Hi
On Tue, Aug 5, 2025 at 10:30 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_ram_handle_discard() must report an
> error
> in errp, in case of failure.
>
> Signed-off-by: Arun Menon <arme...@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> migration/savevm.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index
> eb90873a750ded354b3db31cba40b44d1be79864..3abe4193e02aae9c813ff07fb388a7ee470c8a6a
> 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2004,7 +2004,7 @@ static int
> loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
> * There can be 0..many of these messages, each encoding multiple pages.
> */
> static int loadvm_postcopy_ram_handle_discard(MigrationIncomingState *mis,
> - uint16_t len)
> + uint16_t len, Error **errp)
> {
> int tmp;
> char ramid[256];
> @@ -2017,6 +2017,7 @@ static int
> loadvm_postcopy_ram_handle_discard(MigrationIncomingState *mis,
> /* 1st discard */
> tmp = postcopy_ram_prepare_discard(mis);
> if (tmp) {
> + error_setg(errp, "Failed to prepare for RAM discard: %d",
> tmp);
> return tmp;
> }
> break;
> @@ -2026,8 +2027,9 @@ static int
> loadvm_postcopy_ram_handle_discard(MigrationIncomingState *mis,
> break;
>
> default:
> - error_report("CMD_POSTCOPY_RAM_DISCARD in wrong postcopy state
> (%d)",
> - ps);
> + error_setg(errp,
> + "CMD_POSTCOPY_RAM_DISCARD in wrong postcopy state
> (%d)",
> + ps);
> return -1;
> }
> /* We're expecting a
> @@ -2036,29 +2038,30 @@ static int
> loadvm_postcopy_ram_handle_discard(MigrationIncomingState *mis,
> * then at least 1 16 byte chunk
> */
> if (len < (1 + 1 + 1 + 1 + 2 * 8)) {
> - error_report("CMD_POSTCOPY_RAM_DISCARD invalid length (%d)", len);
> + error_setg(errp, "CMD_POSTCOPY_RAM_DISCARD invalid length (%d)",
> len);
> return -1;
> }
>
> tmp = qemu_get_byte(mis->from_src_file);
> if (tmp != postcopy_ram_discard_version) {
> - error_report("CMD_POSTCOPY_RAM_DISCARD invalid version (%d)",
> tmp);
> + error_setg(errp, "CMD_POSTCOPY_RAM_DISCARD invalid version (%d)",
> tmp);
> return -1;
> }
>
> if (!qemu_get_counted_string(mis->from_src_file, ramid)) {
> - error_report("CMD_POSTCOPY_RAM_DISCARD Failed to read RAMBlock
> ID");
> + error_setg(errp,
> + "CMD_POSTCOPY_RAM_DISCARD Failed to read RAMBlock ID");
> return -1;
> }
> tmp = qemu_get_byte(mis->from_src_file);
> if (tmp != 0) {
> - error_report("CMD_POSTCOPY_RAM_DISCARD missing nil (%d)", tmp);
> + error_setg(errp, "CMD_POSTCOPY_RAM_DISCARD missing nil (%d)",
> tmp);
> return -1;
> }
>
> len -= 3 + strlen(ramid);
> if (len % 16) {
> - error_report("CMD_POSTCOPY_RAM_DISCARD invalid length (%d)", len);
> + error_setg(errp, "CMD_POSTCOPY_RAM_DISCARD invalid length (%d)",
> len);
> return -1;
> }
> trace_loadvm_postcopy_ram_handle_discard_header(ramid, len);
> @@ -2070,6 +2073,7 @@ static int
> loadvm_postcopy_ram_handle_discard(MigrationIncomingState *mis,
> len -= 16;
> int ret = ram_discard_range(ramid, start_addr, block_length);
> if (ret) {
> + error_setg(errp, "Failed to discard RAM range %s: %d", ramid,
> ret);
>
note: the ram_discard_range() and ram_block_discard_range() functions also
calls error_report() Maybe they can be converted too.., let's not do this
in this already long series.
> return ret;
> }
> }
> @@ -2629,11 +2633,7 @@ static int loadvm_process_command(QEMUFile *f,
> Error **errp)
> return loadvm_postcopy_handle_run(mis, errp);
>
> case MIG_CMD_POSTCOPY_RAM_DISCARD:
> - ret = loadvm_postcopy_ram_handle_discard(mis, len);
> - if (ret < 0) {
> - error_setg(errp, "Failed to load device state command: %d",
> ret);
> - }
> - return ret;
> + return loadvm_postcopy_ram_handle_discard(mis, len, errp);
>
> case MIG_CMD_POSTCOPY_RESUME:
> loadvm_postcopy_handle_resume(mis);
>
> --
> 2.50.1
>
>
