[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 13/20] migration: Move channel setup out of postcopy_try_reco
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH 13/20] migration: Move channel setup out of postcopy_try_recover() |
Date: |
Wed, 23 Feb 2022 09:47:03 +0000 |
User-agent: |
Mutt/2.1.5 (2021-12-30) |
* Peter Xu (peterx@redhat.com) wrote:
> On Tue, Feb 22, 2022 at 10:57:34AM +0000, Dr. David Alan Gilbert wrote:
> > * Peter Xu (peterx@redhat.com) wrote:
> > > We used to use postcopy_try_recover() to replace
> > > migration_incoming_setup() to
> > > setup incoming channels. That's fine for the old world, but in the new
> > > world
> > > there can be more than one channels that need setup. Better move the
> > > channel
> > > setup out of it so that postcopy_try_recover() only handles the last
> > > phase of
> > > switching to the recovery phase.
> > >
> > > To do that in migration_fd_process_incoming(), move the
> > > postcopy_try_recover()
> > > call to be after migration_incoming_setup(), which will setup the
> > > channels.
> > > While in migration_ioc_process_incoming(), postpone the recover() routine
> > > right
> > > before we'll jump into migration_incoming_process().
> > >
> > > A side benefit is we don't need to pass in QEMUFile* to
> > > postcopy_try_recover()
> > > anymore. Remove it.
> > >
> > > Signed-off-by: Peter Xu <peterx@redhat.com>
> >
> > OK, but note one question below:
> >
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> Thanks.
>
> >
> > > ---
> > > migration/migration.c | 23 +++++++++++------------
> > > 1 file changed, 11 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index 67520d3105..b2e6446457 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -665,19 +665,20 @@ void migration_incoming_process(void)
> > > }
> > >
> > > /* Returns true if recovered from a paused migration, otherwise false */
> > > -static bool postcopy_try_recover(QEMUFile *f)
> > > +static bool postcopy_try_recover(void)
> > > {
> > > MigrationIncomingState *mis = migration_incoming_get_current();
> > >
> > > if (mis->state == MIGRATION_STATUS_POSTCOPY_PAUSED) {
> > > /* Resumed from a paused postcopy migration */
> > >
> > > - mis->from_src_file = f;
> > > + /* This should be set already in migration_incoming_setup() */
> > > + assert(mis->from_src_file);
> > > /* Postcopy has standalone thread to do vm load */
> > > - qemu_file_set_blocking(f, true);
> > > + qemu_file_set_blocking(mis->from_src_file, true);
> >
> > Does that set_blocking happen on the 2nd channel somewhere?
>
> Nop. I think the rational is that by default all channels are blocking.
>
> Then what happened is: migration code only sets the main channel to
> non-blocking on incoming, that's in migration_incoming_setup(). Hence for
> postcopy recovery we need to tweak it to blocking here.
OK, yes, so the rule seems to be if it's done in it's own thread, we
make it blocking.
> The 2nd new channel is not operated by migration_incoming_setup(), but by
> postcopy_preempt_new_channel(), so it keeps the original blocking state,
> which should be blocking.
>
> If we want to make that clear, we can proactively set non-blocking too in
> postcopy_preempt_new_channel() on the 2nd channel. It's just that it
> should be optional as long as blocking is the default for any new fd of a
> socket.
OK, I notice that in 9e4d2b9 made it explicit on the outgoing side.
Dave
> Thanks,
>
> --
> Peter Xu
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
- [PATCH 09/20] migration: Move migrate_allow_multifd and helpers into migration.c, (continued)
- [PATCH 09/20] migration: Move migrate_allow_multifd and helpers into migration.c, Peter Xu, 2022/02/16
- [PATCH 10/20] migration: Enlarge postcopy recovery to capture !-EIO too, Peter Xu, 2022/02/16
- [PATCH 11/20] migration: postcopy_pause_fault_thread() never fails, Peter Xu, 2022/02/16
- [PATCH 12/20] migration: Export ram_load_postcopy(), Peter Xu, 2022/02/16
- [PATCH 13/20] migration: Move channel setup out of postcopy_try_recover(), Peter Xu, 2022/02/16
[PATCH 14/20] migration: Add migration_incoming_transport_cleanup(), Peter Xu, 2022/02/16
[PATCH 15/20] migration: Allow migrate-recover to run multiple times, Peter Xu, 2022/02/16
[PATCH 16/20] migration: Add postcopy-preempt capability, Peter Xu, 2022/02/16
[PATCH 17/20] migration: Postcopy preemption preparation on channel creation, Peter Xu, 2022/02/16