* Peter Xu (pet...@redhat.com) wrote: > On Tue, Oct 10, 2017 at 05:38:01PM +0800, Peter Xu wrote: > > [...] > > > > > But I agree about the reasoning. How > > > > about one more patch to postpone the "active" to "postcopy-active" > > > > state change after the package is handled correctly? Like: > > > > > > > > -------------- > > > > diff --git a/migration/savevm.c b/migration/savevm.c > > > > > > > > index b5c3214034..8317b2a7e2 100644 > > > > --- a/migration/savevm.c > > > > +++ b/migration/savevm.c > > > > @@ -1573,8 +1573,6 @@ static void *postcopy_ram_listen_thread(void > > > > *opaque) > > > > > > > > QEMUFile *f = mis->from_src_file; > > > > > > > > int load_res; > > > > > > > > - migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, > > > > > > > > - MIGRATION_STATUS_POSTCOPY_ACTIVE); > > > > > > > > qemu_sem_post(&mis->listen_thread_sem); > > > > > > > > trace_postcopy_ram_listen_thread_start(); > > > > > > > > > > > > @@ -1817,6 +1815,9 @@ static int > > > > loadvm_handle_cmd_packaged(MigrationIncomingState *mis) > > > > > > > > qemu_fclose(packf); > > > > object_unref(OBJECT(bioc)); > > > > > > > > + migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, > > > > > > > > + MIGRATION_STATUS_POSTCOPY_ACTIVE); > > > > > > > > + > > > > return ret; > > > > } > > > > -------------- > > > > > > > > This function will only be called with "postcopy-active" state. > > > > > > I *think* that's safe; you've got to be careful, but I can't see > > > anyone on the destination that cares about the destinction. > > > > Indeed, but I'd say that's the best thing I can think of (and the > > simplest). Even, not sure whether it'll be more clear if we set > > postcopy-active state right before starting the VM on destination, > > say, at the beginning of loadvm_postcopy_handle_run_bh(). > > When thinking about this, I had another question. > > How do we handle the case if we failed to send the device states in > postcopy_start()? In that, we do qemu_savevm_send_packaged() then we > assume we are good and return with success. However > qemu_savevm_send_packaged() only means that the data is queued in > write buffer of source host, it does not mean that destination has > loaded the device states correctly. It's still possible that > destination VM failed to receive the whole packaged data, but source > thought it had done so without problem. > > Then source will continue with postcopy-active, destination VM will > instead fail, then fail the source. VM should be lost then since it's > postcopy rather than precopy. > > Meanwhile, this cannot be handled by postcopy recovery, since IIUC > postcopy recovery only works after the states are at least loaded on > destination VM (I'll avoid going deeper to think a more complex > protocol for postcopy recovery, please see below). > > I think the best/simplest thing to do when encountering this error is > that, when this happens we just fail the migration on source and > continue running on source, which should be the same failure handling > path with precopy. But still it seems that we don't have a good > mechanism to detect the error when sending MIG_CMD_PACKAGED message > fails in some way (we can add one ACK from dst->src, however it breaks > old VMs). > > Before going further, would my worry make any sense?
Yes, I think it does; it wouldn't be unusual for a device-load to fail due to some problem on the destination host or a problem in device serialisation. I also think we should be OK to restart on the source; although we have to be careful - can we really know what the previous devices (that loaded succesfully) did? Hopefully they didn't change the state of the storage/networking because the destination CPUs haven't started. > (I hope this can be a separate problem from postcopy recovery series, > if it is indeed a problem. For postcopy recovery, I hope the idea of > postponing setup POSTCOPY_ACTIVE would suffice) Sure. Dave > -- > Peter Xu -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
