* Peter Xu (pet...@redhat.com) wrote: > On Tue, Jul 19, 2022 at 04:15:49PM +0100, Dr. David Alan Gilbert wrote: > > * Peter Xu (pet...@redhat.com) wrote: > > > Create a new socket for postcopy to be prepared to send postcopy requested > > > pages via this specific channel, so as to not get blocked by precopy > > > pages. > > > > > > A new thread is also created on dest qemu to receive data from this new > > > channel > > > based on the ram_load_postcopy() routine. > > > > > > The ram_load_postcopy(POSTCOPY) branch and the thread has not started to > > > function, and that'll be done in follow up patches. > > > > > > Cleanup the new sockets on both src/dst QEMUs, meanwhile look after the > > > new > > > thread too to make sure it'll be recycled properly. > > > > I'm hitting a CI failure here: > > https://gitlab.com/dagrh/qemu/-/jobs/2741659845 > > > > c.o -c ../migration/migration.c > > ../migration/migration.c: In function ‘migration_ioc_process_incoming’: > > ../migration/migration.c:766:8: error: ‘start_migration’ may be used > > uninitialized in this function [-Werror=maybe-uninitialized] > > 766 | if (start_migration) { > > | ^ > > > > > Reviewed-by: Daniel P. Berrang?? <berra...@redhat.com> > > > Reviewed-by: Juan Quintela <quint...@redhat.com> > > > Signed-off-by: Peter Xu <pet...@redhat.com> > > > --- > > > migration/migration.c | 62 +++++++++++++++++++++++---- > > > migration/migration.h | 8 ++++ > > > migration/postcopy-ram.c | 92 ++++++++++++++++++++++++++++++++++++++-- > > > migration/postcopy-ram.h | 10 +++++ > > > migration/ram.c | 25 ++++++++--- > > > migration/ram.h | 4 +- > > > migration/savevm.c | 20 ++++----- > > > migration/socket.c | 22 +++++++++- > > > migration/socket.h | 1 + > > > migration/trace-events | 5 ++- > > > 10 files changed, 218 insertions(+), 31 deletions(-) > > > > > > diff --git a/migration/migration.c b/migration/migration.c > > > index ce7bb68cdc..9484fec0b2 100644 > > > --- a/migration/migration.c > > > +++ b/migration/migration.c > > > @@ -321,6 +321,12 @@ void migration_incoming_state_destroy(void) > > > mis->page_requested = NULL; > > > } > > > > > > + if (mis->postcopy_qemufile_dst) { > > > + > > > migration_ioc_unregister_yank_from_file(mis->postcopy_qemufile_dst); > > > + qemu_fclose(mis->postcopy_qemufile_dst); > > > + mis->postcopy_qemufile_dst = NULL; > > > + } > > > + > > > yank_unregister_instance(MIGRATION_YANK_INSTANCE); > > > } > > > > > > @@ -714,15 +720,21 @@ void migration_fd_process_incoming(QEMUFile *f, > > > Error **errp) > > > migration_incoming_process(); > > > } > > > > > > +static bool migration_needs_multiple_sockets(void) > > > +{ > > > + return migrate_use_multifd() || migrate_postcopy_preempt(); > > > +} > > > + > > > void migration_ioc_process_incoming(QIOChannel *ioc, Error **errp) > > > { > > > MigrationIncomingState *mis = migration_incoming_get_current(); > > > Error *local_err = NULL; > > > bool start_migration; > > > + QEMUFile *f; > > > > > > if (!mis->from_src_file) { > > > /* The first connection (multifd may have multiple) */ > > > - QEMUFile *f = qemu_file_new_input(ioc); > > > + f = qemu_file_new_input(ioc); > > > > > > if (!migration_incoming_setup(f, errp)) { > > > return; > > > @@ -730,13 +742,18 @@ void migration_ioc_process_incoming(QIOChannel > > > *ioc, Error **errp) > > > > > > /* > > > * Common migration only needs one channel, so we can start > > > - * right now. Multifd needs more than one channel, we wait. > > > + * right now. Some features need more than one channel, we wait. > > > */ > > > - start_migration = !migrate_use_multifd(); > > > + start_migration = !migration_needs_multiple_sockets(); > > > } else { > > > /* Multiple connections */ > > > - assert(migrate_use_multifd()); > > > - start_migration = multifd_recv_new_channel(ioc, &local_err); > > > + assert(migration_needs_multiple_sockets()); > > > + if (migrate_use_multifd()) { > > > + start_migration = multifd_recv_new_channel(ioc, &local_err); > > > + } else if (migrate_postcopy_preempt()) { > > > + f = qemu_file_new_input(ioc); > > > + start_migration = postcopy_preempt_new_channel(mis, f); > > > + } > > > > So that doesn't always set start_migration? > > Logically it should, because we asserted on > migration_needs_multiple_sockets(), while multifd and preempt mode are the > only two that may need multiple sockets. So it must go into either of > them. > > A fix could be like this: > > diff --git a/migration/migration.c b/migration/migration.c > index 76cf2a72c0..7c7e529ca7 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -753,7 +753,8 @@ void migration_ioc_process_incoming(QIOChannel *ioc, > Error **errp) > assert(migration_needs_multiple_sockets()); > if (migrate_use_multifd()) { > start_migration = multifd_recv_new_channel(ioc, &local_err); > - } else if (migrate_postcopy_preempt()) { > + } else { > + assert(migrate_postcopy_preempt()); > f = qemu_file_new_input(ioc); > start_migration = postcopy_preempt_new_channel(mis, f); > } > > Or we simply set start_migration=false as initial value. > > Should I repost the series?
No just post a fixup and I'll merge it. Dave > Thanks, > > -- > Peter Xu > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
