Re: [RFC PATCH 3/4] migration: Make listen thread joinable

[Peter Xu <peterx@redhat.com>]

On Fri, Aug 08, 2025 at 01:08:39PM +0200, Juraj Marcin wrote:
> Hi Peter,
> 
> On 2025-08-07 16:57, Peter Xu wrote:
> > On Thu, Aug 07, 2025 at 01:49:11PM +0200, Juraj Marcin wrote:
> > > From: Juraj Marcin <jmarcin@redhat.com>
> > > 
> > > This patch allows joining the migration listen thread. This is done in
> > > preparation for the introduction of "postcopy-setup" state at the
> > > beginning of a postcopy migration, when destination can fail gracefully
> > > and source side then resume to a running state.
> > > 
> > > In case of such failure, to gracefully perform all cleanup in the main
> > > migration thread, we need to wait for the listen thread to exit, which
> > > can be done by joining it.
> > > 
> > > Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
> > > ---
> > >  migration/migration.c | 1 +
> > >  migration/savevm.c    | 2 +-
> > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index e5ce2940d5..8174e811eb 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -901,6 +901,7 @@ process_incoming_migration_co(void *opaque)
> > >               * Postcopy was started, cleanup should happen at the end of the
> > >               * postcopy thread.
> > >               */
> > > +            qemu_thread_detach(&mis->listen_thread);
> > >              trace_process_incoming_migration_co_postcopy_end_main();
> > >              goto out;
> > >          }
> > > diff --git a/migration/savevm.c b/migration/savevm.c
> > > index fabbeb296a..d2360be53c 100644
> > > --- a/migration/savevm.c
> > > +++ b/migration/savevm.c
> > > @@ -2217,7 +2217,7 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> > >      mis->have_listen_thread = true;
> > >      postcopy_thread_create(mis, &mis->listen_thread,
> > >                             MIGRATION_THREAD_DST_LISTEN,
> > > -                           postcopy_ram_listen_thread, QEMU_THREAD_DETACHED);
> > > +                           postcopy_ram_listen_thread, QEMU_THREAD_JOINABLE);
> > 
> > This is good; I actually forgot it used to be detached..
> > 
> > Instead of proactively detach it above, could we always properly join it
> 
> However, after the main thread finishes loading device state from the
> package, process_incoming_migration_co() exits, and IIUC main thread is
> then no longer occupied with migration. So, if we should instead join
> the listen thread, we probably should yield the coroutine until the
> listen thread can be joined, so we are not blocking the main thread?

Or could we schedule a bottom half at the end of
postcopy_ram_listen_thread() to join itself?  We could move something over
into the BH:

    ... join() ...
    mis->have_listen_thread = false;
    migration_incoming_state_destroy();
    object_unref(OBJECT(migr));

> 
> > (and hopefully every migration thread)?  Then we could drop patch 1 too.
> 
> If I haven't missed any, there are no detached migration threads except
> listen and get dirty rate threads.

Yep.

From mgmt pov, IMHO it's always good we create joinable threads.  But we
can leave the calc_dirty_rate thread until necessary.

-- 
Peter Xu