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 <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > > 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
