On Wed, May 31, 2017 at 08:33:01PM +0200, Juan Quintela wrote: > "Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote: > > * Peter Xu (pet...@redhat.com) wrote: > >> There are some places that binded "return path" with postcopy. Let's be > >> prepared for its usage even without postcopy. This patch mainly did this > >> on source side. > >> > >> Signed-off-by: Peter Xu <pet...@redhat.com> > >> --- > >> standalone patch isolated from the return path series. ok to be picked > >> up in case one day we'll re-face the return path enablement. > >> With it, we are ready on source side. The dst side change has been queued. > >> --- > >> migration/migration.c | 11 ++++++----- > >> migration/trace-events | 4 ++-- > >> 2 files changed, 8 insertions(+), 7 deletions(-) > >> > >> diff --git a/migration/migration.c b/migration/migration.c > >> index ad29e53..96e549e 100644 > >> --- a/migration/migration.c > >> +++ b/migration/migration.c > >> @@ -1850,13 +1850,12 @@ static void migration_completion(MigrationState > >> *s, int current_active_state, > >> * cleaning everything else up (since if there are no failures > >> * it will wait for the destination to send it's status in > >> * a SHUT command). > >> - * Postcopy opens rp if enabled (even if it's not avtivated) > >> */ > >> - if (migrate_postcopy_ram()) { > >> + if (s->rp_state.from_dst_file) { > >> int rp_error; > >> - trace_migration_completion_postcopy_end_before_rp(); > >> + trace_migration_return_path_end_before(); > >> rp_error = await_return_path_close_on_source(s); > >> - trace_migration_completion_postcopy_end_after_rp(rp_error); > >> + trace_migration_return_path_end_after(rp_error); > >> if (rp_error) { > >> goto fail_invalidate; > >> } > >> @@ -1931,13 +1930,15 @@ static void *migration_thread(void *opaque) > >> > >> qemu_savevm_state_header(s->to_dst_file); > >> > >> - if (migrate_postcopy_ram()) { > >> + if (s->to_dst_file) { > >> /* Now tell the dest that it should open its end so it can reply > >> */ > >> qemu_savevm_send_open_return_path(s->to_dst_file); > >> > >> /* And do a ping that will make stuff easier to debug */ > >> qemu_savevm_send_ping(s->to_dst_file, 1); > > > > I'm confused. > > The migration_thread on the source will always have a s->to_dst_file > > there so why test? > > > > But also, we can't send the 'open return path' and 'send ping' messages > > without a guard; sending them to old QEMUs will cause them to fail when > > they don't know what the message is. I suspect sending them to > > slightly-old QEMUs will cause them to fail when they try and send a > > return message if the destination can't send it. > > > > I can see a if (s->rp_state.from_dst_file) making some sense if it's > > done at the right point. > > You are right. > > Second chunk makes no sense. I told Peter to split the > "objectification" of migration state and this patch. This made more > sense when the user *had* required a return path.
Oops! I believe it should be what Dave mentioned (s->rp_state.from_dst_file rather than s->to_dst_file). Sorry! I'll repost to see whether we'd like the new one. Thanks, -- Peter Xu