On Fri, 23 Jan 2026 at 03:41, Peter Xu <[email protected]> wrote: > Introduce a helper to show if we want to enable return path.
* return path -> the return path > We used to imply that for postcopy-ram, that's not normally how we add cap > dependencies, however it has been like it for years. Add some comment and > stick with it. /* This reads more suitable for cover-letter. */ Maybe - Postcopy requires and enables the return path connection, add due comments for it. > Signed-off-by: Peter Xu <[email protected]> > --- > migration/migration.h | 1 + > migration/migration.c | 15 ++++++++++++++- > migration/rdma.c | 8 +++----- > migration/tls.c | 4 ++-- > 4 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/migration/migration.h b/migration/migration.h > index ccc4e536a5..32c2e70205 100644 > --- a/migration/migration.h > +++ b/migration/migration.h > @@ -605,5 +605,6 @@ void migration_bitmap_sync_precopy(bool last_stage); > /* migration/block-dirty-bitmap.c */ > void dirty_bitmap_mig_init(void); > bool should_send_vmdesc(void); > +bool migrate_use_return_path(void); > > #endif > diff --git a/migration/migration.c b/migration/migration.c > index 1bcde301f7..f550485ea5 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -109,6 +109,19 @@ static bool close_return_path_on_source(MigrationState > *s); > static void migration_completion_end(MigrationState *s); > static void migrate_hup_delete(MigrationState *s); > > +/* > + * Postcopy-ram will imply return path. * Postcopy-ram requires and enables the return path connection. > + * Normally we don't describe capability dependencies like this; we could > + * have failed the migration request if postcopy-ram is selected withour > + * return-path. Said that, we had this as part of postcopy-ram capability > + * ABI for years.. let's stick with it. > + */ * withour -> without the > +bool migrate_use_return_path(void) > +{ > + return migrate_postcopy_ram() || migrate_return_path(); > +} > + * It would have been nicer if we could include '&& !rdma->is_return_path' here, but that does not seem doable. Is '&& !rdma->is_return_path' check really required? > static void migration_downtime_start(MigrationState *s) > { > trace_vmstate_downtime_checkpoint("src-downtime-start"); > @@ -4067,7 +4080,7 @@ void migration_connect(MigrationState *s, Error > *error_in) > * precopy, only if user specified "return-path" capability would > * QEMU uses the return path. > */ > - if (migrate_postcopy_ram() || migrate_return_path()) { > + if (migrate_use_return_path()) { > open_return_path_on_source(s); > } > > diff --git a/migration/rdma.c b/migration/rdma.c > index cced173379..a55c80908c 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -3186,8 +3186,7 @@ static int qemu_rdma_accept(RDMAContext *rdma) > * initialize the RDMAContext for return path for postcopy after first > * connection request reached. > */ > - if ((migrate_postcopy() || migrate_return_path()) > - && !rdma->is_return_path) { > + if (migrate_use_return_path() && !rdma->is_return_path) { > rdma_return_path = qemu_rdma_data_init(isock, NULL); * Here we check && !rdma->is_return_path > /* RDMA postcopy need a separate queue pair for return path */ > - if (migrate_postcopy() || migrate_return_path()) { > + if (migrate_use_return_path()) { > rdma_return_path = qemu_rdma_data_init(host_port, errp); * Here we don't check '&& !rdma->is_return_path' ...? > if (rdma_return_path == NULL) { > diff --git a/migration/tls.c b/migration/tls.c > index 56b5d1cc90..7fb49ddd82 100644 > --- a/migration/tls.c > +++ b/migration/tls.c > @@ -90,7 +90,7 @@ void migration_tls_channel_process_incoming(MigrationState > *s, > > trace_migration_tls_incoming_handshake_start(); > qio_channel_set_name(QIO_CHANNEL(tioc), "migration-tls-incoming"); > - if (migrate_postcopy_ram() || migrate_return_path()) { > + if (migrate_use_return_path()) { > qio_channel_set_feature(QIO_CHANNEL(tioc), > QIO_CHANNEL_FEATURE_CONCURRENT_IO); > } > @@ -154,7 +154,7 @@ void migration_tls_channel_connect(MigrationState *s, > trace_migration_tls_outgoing_handshake_start(hostname); > qio_channel_set_name(QIO_CHANNEL(tioc), "migration-tls-outgoing"); > > - if (migrate_postcopy_ram() || migrate_return_path()) { > + if (migrate_use_return_path()) { > qio_channel_set_feature(QIO_CHANNEL(tioc), > QIO_CHANNEL_FEATURE_CONCURRENT_IO); > } > -- * Change looks okay. /* The !rdma->is_return_path check is a little confusing. */ Reviewed-by: Prasad Pandit <[email protected]> Thank you. --- - Prasad
