On Mon, Feb 15, 2021 at 06:35:15PM +0000, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berra...@redhat.com) wrote:
> > This is an incremental step in converting vmstate loading code to report
> > via Error objects instead of printing directly to the console/monitor.
> >
> > Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
> > ---
> > migration/colo.c | 3 +-
> > migration/savevm.c | 73 +++++++++++++++++++++++++++++++---------------
> > migration/savevm.h | 3 +-
> > 3 files changed, 52 insertions(+), 27 deletions(-)
> >
> > diff --git a/migration/colo.c b/migration/colo.c
> > index e344b7cf32..4a050ac579 100644
> > --- a/migration/colo.c
> > +++ b/migration/colo.c
> > @@ -705,11 +705,10 @@ static void
> > colo_incoming_process_checkpoint(MigrationIncomingState *mis,
> >
> > qemu_mutex_lock_iothread();
> > 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);
> > qemu_mutex_unlock_iothread();
> >
> > if (ret < 0) {
> > - error_setg(errp, "Load VM's live state (ram) error");
> > return;
> > }
> >
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index dd41292d4e..e47aec435c 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -1819,6 +1819,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
> > QEMUFile *f = mis->from_src_file;
> > int load_res;
> > MigrationState *migr = migrate_get_current();
> > + Error *local_err = NULL;
> >
> > object_ref(OBJECT(migr));
> >
> > @@ -1833,7 +1834,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
> > * in qemu_file, and thus we must be blocking now.
> > */
> > qemu_file_set_blocking(f, true);
> > - load_res = qemu_loadvm_state_main(f, mis);
> > + load_res = qemu_loadvm_state_main(f, mis, &local_err);
> >
> > /*
> > * This is tricky, but, mis->from_src_file can change after it
> > @@ -1849,6 +1850,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
> > if (load_res < 0) {
> > qemu_file_set_error(f, load_res);
> > dirty_bitmap_mig_cancel_incoming();
> > + error_report_err(local_err);
> > if (postcopy_state_get() == POSTCOPY_INCOMING_RUNNING &&
> > !migrate_postcopy_ram() && migrate_dirty_bitmaps())
> > {
> > @@ -1859,12 +1861,10 @@ static void *postcopy_ram_listen_thread(void
> > *opaque)
> > __func__, load_res);
> > load_res = 0; /* prevent further exit() */
> > } else {
> > - error_report("%s: loadvm failed: %d", __func__, load_res);
> > migrate_set_state(&mis->state,
> > MIGRATION_STATUS_POSTCOPY_ACTIVE,
> > MIGRATION_STATUS_FAILED);
> > }
> > - }
> > - if (load_res >= 0) {
> > + } else {
> > /*
> > * This looks good, but it's possible that the device loading in
> > the
> > * main thread hasn't finished yet, and so we might not be in 'RUN'
> > @@ -2116,14 +2116,17 @@ static int
> > loadvm_postcopy_handle_resume(MigrationIncomingState *mis)
> > * @mis: Incoming state
> > * @length: Length of packaged data to read
> > *
> > - * Returns: Negative values on error
> > - *
> > + * Returns:
> > + * 0: success
> > + * LOADVM_QUIT: success, but stop
> > + * -1: error
> > */
> > static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
> > {
> > int ret;
> > size_t length;
> > QIOChannelBuffer *bioc;
> > + Error *local_err = NULL;
> >
> > length = qemu_get_be32(mis->from_src_file);
> > trace_loadvm_handle_cmd_packaged(length);
> > @@ -2149,8 +2152,11 @@ static int
> > loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
> >
> > QEMUFile *packf = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
> >
> > - ret = qemu_loadvm_state_main(packf, mis);
> > + ret = qemu_loadvm_state_main(packf, mis, &local_err);
> > trace_loadvm_handle_cmd_packaged_main(ret);
> > + if (ret < 0) {
> > + error_report_err(local_err);
> > + }
> > qemu_fclose(packf);
> > object_unref(OBJECT(bioc));
> >
> > @@ -2568,7 +2574,14 @@ static bool
> > postcopy_pause_incoming(MigrationIncomingState *mis)
> > return true;
> > }
> >
> > -int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
> > +/*
> > + * Returns:
> > + * 0: success
> > + * LOADVM_QUIT: success, but stop
> > + * -1: error
> > + */
> > +int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
> > + Error **errp)
> > {
> > uint8_t section_type;
> > int ret = 0;
> > @@ -2579,7 +2592,9 @@ retry:
> >
> > if (qemu_file_get_error(f)) {
> > ret = qemu_file_get_error(f);
> > - break;
> > + error_setg(errp,
> > + "Failed to load device state section ID: %d", ret);
>
> Can I ask why these don't use strerror(ret) ?
No good reason.
>
> The test I'm running is, start a VM with an actual guest and a useful
> amount of ram:
>
> ./x86_64-softmmu/qemu-system-x86_64 -M pc,accel=kvm -nographic -m 8G -drive
> if=virtio,file=/home/vmimages/fedora-33-nest.qcow
>
> ./x86_64-softmmu/qemu-system-x86_64 -M pc,accel=kvm -nographic -m 8G -drive
> if=virtio,file=/home/vmimages/fedora-33-nest.qcow -incoming tcp:0:4444
>
> source:
> migrate_set_speed 1m
> migrate -d tcp:0:4444
> <Now quickly>
> migrate_cancel
>
> In the old world I get:
> qemu-system-x86_64: load of migration failed: Input/output error
>
> In your world I get:
> qemu-system-x86_64: Failed to load device state section ID: -5
>
> (5 being EIO)
Yep, looks like I should fix that.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
