Hi Marc-André, Thank you for the review.
On Wed, Aug 06, 2025 at 11:54:36AM +0400, Marc-André Lureau wrote: > Hi > > On Tue, Aug 5, 2025 at 10:30 PM Arun Menon <[email protected]> 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 <[email protected]> > > > > Reviewed-by: Marc-André Lureau <[email protected]> > > > > --- > > 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. Agreed. > > > > > > 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 > > > > Regards, Arun
