Hi Fabiano, Thanks for the review. On Fri, Aug 15, 2025 at 04:51:04PM -0300, Fabiano Rosas wrote: > Arun Menon <arme...@redhat.com> writes: > > > 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_handle_recv_bitmap() 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> > > --- > > migration/savevm.c | 21 ++++++++++----------- > > 1 file changed, 10 insertions(+), 11 deletions(-) > > > > diff --git a/migration/savevm.c b/migration/savevm.c > > index > > 9098c4bd3394d7b9ed77e20afbb26fd9c9be6550..a7aede1b3df9164e322e68f3889df7c4166876f5 > > 100644 > > --- a/migration/savevm.c > > +++ b/migration/savevm.c > > @@ -2480,32 +2480,35 @@ static int > > loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp) > > * len (1 byte) + ramblock_name (<255 bytes) > > */ > > static int loadvm_handle_recv_bitmap(MigrationIncomingState *mis, > > - uint16_t len) > > + uint16_t len, Error **errp) > > { > > QEMUFile *file = mis->from_src_file; > > RAMBlock *rb; > > char block_name[256]; > > size_t cnt; > > + int ret; > > > > cnt = qemu_get_counted_string(file, block_name); > > if (!cnt) { > > - error_report("%s: failed to read block name", __func__); > > + error_setg(errp, "failed to read block name: %s", block_name); > > Could we not print the buffer that's just failed to be written? As a > matter of principle =) yes, we must not, its content will be empty. Thanks > > > return -EINVAL; > > } > > > > /* Validate before using the data */ > > - if (qemu_file_get_error(file)) { > > - return qemu_file_get_error(file); > > + ret = qemu_file_get_error(file); > > + if (ret < 0) { > > + error_setg(errp, "migration stream has error: %d", ret); > > I've been suggesting "stream error:", probably best to keep it uniform. Sure, will do. > > > + return ret; > > } > > > > if (len != cnt + 1) { > > - error_report("%s: invalid payload length (%d)", __func__, len); > > + error_setg(errp, "invalid payload length (%d)", len); > > return -EINVAL; > > } > > > > rb = qemu_ram_block_by_name(block_name); > > if (!rb) { > > - error_report("%s: block '%s' not found", __func__, block_name); > > + error_setg(errp, "block '%s' not found", block_name); > > return -EINVAL; > > } > > > > @@ -2642,11 +2645,7 @@ static int loadvm_process_command(QEMUFile *f, Error > > **errp) > > return 0; > > > > case MIG_CMD_RECV_BITMAP: > > - ret = loadvm_handle_recv_bitmap(mis, len); > > - if (ret < 0) { > > - error_setg(errp, "Failed to load device state command: %d", > > ret); > > - } > > - return ret; > > + return loadvm_handle_recv_bitmap(mis, len, errp); > > > > case MIG_CMD_ENABLE_COLO: > > ret = loadvm_process_enable_colo(mis); >
Regards, Arun