Hi Peter On 2025-08-08 13:05, Peter Xu wrote: > 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 <jmar...@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 <jmar...@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));
That sounds like a good idea, I will certainly try that, thank you. Best regards, Juraj Marcin > > > > > > (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 >
