Peter Xu <[email protected]> writes: > PROBLEM > ======= > > Migration reliably hangs with old machine types (7.2-) when these migration > capabilities are enabled: multifd, postcopy-ram, postcopy-preempt. > > Simplest reproducer : > > $ QTEST_QEMU_MACHINE_TYPE=pc-q35-7.2 \ > QTEST_QEMU_BINARY=./qemu-system-x86_64 \ > ./tests/qtest/migration-test --full \ > -r /x86_64/migration/multifd+postcopy/preempt/plain > > Current workaround is to kill destination QEMU, cancel the migration on > source (with/without yank), disable one or more of the features, and retry. > > The hang is caused by receiving preempt channel before most of the multifd > channels, the peek buffer operation will stuck forever. > > Incoming side QEMU hang (with relevant tracepoints enabled): > > migrate_set_state new state setup > migration_socket_incoming_accepted > migration_set_incoming_channel ioc=0x557d82c1caa0 ioctype=qio-channel-socket > migration_socket_incoming_accepted > migration_set_incoming_channel ioc=0x557d83023840 ioctype=qio-channel-socket > multifd_recv_new_channel channel 1 > migration_socket_incoming_accepted > migration_set_incoming_channel ioc=0x557d830c08c0 ioctype=qio-channel-socket > multifd_recv_new_channel channel 0 > multifd_recv_thread_start 1 > migration_socket_incoming_accepted > migration_set_incoming_channel ioc=0x557d8284a010 ioctype=qio-channel-socket > multifd_recv_thread_start 0 > multifd_recv_unfill channel 0 packet_num 0 flags 0x1 next packet size 0 > multifd_recv_unfill channel 1 packet_num 1 flags 0x1 next packet size 0 > [dest qemu hangs here] > > Incoming side backtrace: > > __syscall_cancel_arch > __internal_syscall_cancel > __syscall_cancel > recvmsg > qio_channel_socket_readv > qio_channel_readv_full > migration_channel_read_peek > migration_channel_identify > migration_channel_process_incoming > socket_accept_incoming_migration > qio_net_listener_channel_func > qio_channel_fd_source_dispatch > g_main_context_dispatch_unlocked.lto_priv > g_main_context_dispatch > glib_pollfds_poll > os_host_main_loop_wait > main_loop_wait > qemu_main_loop > qemu_default_main > main > > This patch tries to fix it, from incoming side first. This patch alone > should enable single-direction migration to this new QEMU. The next patch > will propose a fix for the outgoing path, fixing new->old migrations. > > BACKGROUND > ========== > > preempt_pre_7_2 used to be a flag marking QEMU releases 7.1/7.2 only > because they have a flaw on channel establishment on preempt channel. > > Now, after we have multifd + postcopy support, new QEMU binaries can > migrate old machine types with both features enabled. Juraj reported > constant migration hang at the very beginning for those old machine types > in our downstream systems with such setup. Juraj found it's because of > this flag. > > QEMU source initiates channels in this order on current master when both > features enabled with postcopy-preempt mode (machine type 7.2, implies > preempt_pre_7_2=on): > > - First, main channel connect() happens, > - Second, postcopy_preempt_setup() invokes connect() for preempt channel, > - Last, in migration_thread(), multifd_send_setup() invokes connect()s > for all the multifd channels > > However on an old QEMU, it expect the channels arrive in order of > "main"->"multifd"->"preempt". Even on a new QEMU, the destination expects > the same order of arrival on the channels when the channels are not > peekable. > > For peekable channels (non-tls sockets), this time we'll likely see preempt > channel appearing very soon, peeking at it causing main thread hang, like > mentioned in the PROBLEM section above. > > SOLUTION > ======== > > One major problem of current hang is we try to peek at preempt channel, > which offers nothing at the beginning. > > Since we'll need to accept stream from old binaries (QEMU 10.1 to 11.1), > arrival of preempt channel early is unavoidable. > > What we can do, though, is to allow concurrent monitoring of all channels > rather than one. Meanwhile, we shouldn't peek at anything that has no > data. > > migration_channel_read_peek() almost does the wait, but it's too late. > This patch proposes to watch all channels from the start without reading at > them at all, relying on an IO watch attached. This will make sure (1) for > preempt channel, we never read on it (watch can be gracefully disarmed when > we know it's preempt channel later) (2) since we have one IO watch for each > channel, then we can allow accept() to happen in almost any order, process > only when there's some header to read. > > To maintain these watches, a new GArray @channels_early is introduced, with > each element to be a (ioc, GSource) tuple (@MigEarlyIncomingChannel), so > that QEMU can manage all the IO watches later with it. Here I used GSource > rather than io_tag because the GSource may or may not be attached to the > default main thread gcontext: when OOB is used, it may be the iothread > context instead. IOW, g_source_remove() on io_tags stops working, which > only works for default main context. > > The trick this patch used is when we have the ability to wait on all > channel headers at the same time, we pick one by one from main and multifd > channels (since both of the types will contain headers dumped right at the > very beginning), leave preempt channel alone. It means, after dest QEMU > finds all main and multifd channels, there are three possibilities: > > - If preempt not enabled, we finished full channel establishment, done. > > - If preempt enabled, > > - If we have one registered IO watch with the IOC, it must be the > preempt channel, > > - If we have no further IO watch, wait for the next accept(), which > must be the preempt channel. > > This will make channel establish work on incoming side even with the old > QEMU binaries. > > While working on it, I managed to refactor the peeking logic quite a bit: > we used to have a very convoluted migration_channel_identify() function, > taking care of two different cases: > > (1) when the channel is peekable (like normal socket channels), > (2) when the channel is not peekable (like TLS or file migrations). > > This patch refactored that part to make it crystal clear on how we make > decisions on which channel is which, and split the peek/identify paths. > > One example incoming migration from pc-q35-7.2 after this patch applied, > and with relevant tracepoints set (8 multifd channels, 1 preempt channel): > > migrate_set_state new state setup > migration_channel_process_incoming ioc=0x557dc8af4e20 > ioctype=qio-channel-socket > migration_incoming_channel_watch ioc=0x557dc8af4e20 > ioctype=qio-channel-socket > migration_channel_process_incoming ioc=0x557dc8abc5d0 > ioctype=qio-channel-socket > migration_incoming_channel_watch ioc=0x557dc8abc5d0 > ioctype=qio-channel-socket > migration_channel_process_incoming ioc=0x557dc953d680 > ioctype=qio-channel-socket > migration_incoming_channel_watch ioc=0x557dc953d680 > ioctype=qio-channel-socket > migration_set_incoming_channel ioc=0x557dc8abc5d0 > ioctype=qio-channel-socket type=MULTIFD > migration_channel_process_incoming ioc=0x557dc898a820 > ioctype=qio-channel-socket > migration_incoming_channel_watch ioc=0x557dc898a820 > ioctype=qio-channel-socket > migration_set_incoming_channel ioc=0x557dc953d680 > ioctype=qio-channel-socket type=MULTIFD > migration_channel_process_incoming ioc=0x557dc919bd70 > ioctype=qio-channel-socket > migration_incoming_channel_watch ioc=0x557dc919bd70 > ioctype=qio-channel-socket > migration_set_incoming_channel ioc=0x557dc898a820 > ioctype=qio-channel-socket type=MULTIFD > migration_channel_process_incoming ioc=0x557dc8734b20 > ioctype=qio-channel-socket > migration_incoming_channel_watch ioc=0x557dc8734b20 > ioctype=qio-channel-socket > migration_set_incoming_channel ioc=0x557dc919bd70 > ioctype=qio-channel-socket type=MULTIFD > migration_channel_process_incoming ioc=0x557dc898bdf0 > ioctype=qio-channel-socket > migration_incoming_channel_watch ioc=0x557dc898bdf0 > ioctype=qio-channel-socket > migration_set_incoming_channel ioc=0x557dc8734b20 > ioctype=qio-channel-socket type=MULTIFD > migration_channel_process_incoming ioc=0x557dc968cfd0 > ioctype=qio-channel-socket > migration_incoming_channel_watch ioc=0x557dc968cfd0 > ioctype=qio-channel-socket > migration_set_incoming_channel ioc=0x557dc898bdf0 > ioctype=qio-channel-socket type=MULTIFD > migration_channel_process_incoming ioc=0x557dc8c31810 > ioctype=qio-channel-socket > migration_incoming_channel_watch ioc=0x557dc8c31810 > ioctype=qio-channel-socket > migration_set_incoming_channel ioc=0x557dc968cfd0 > ioctype=qio-channel-socket type=MULTIFD > migration_set_incoming_channel ioc=0x557dc8c31810 > ioctype=qio-channel-socket type=MULTIFD > migration_set_incoming_channel ioc=0x557dc8af4e20 > ioctype=qio-channel-socket type=MAIN > migration_start_incoming > migrate_set_state new state active > migration_channel_process_incoming ioc=0x557dc8966160 > ioctype=qio-channel-socket > migration_set_incoming_channel ioc=0x557dc8966160 > ioctype=qio-channel-socket type=PREEMPT > > While at it, I renamed the four channel incoming/outgoing tracepoints, make > all to follow migration_[incoming|outgoing]_channel_*(). Especially for > incoming, then we can enable migration_incoming_channel* for incoming > channel debuggings. > > FINAL NOTE ON TLS > ================= > > Note that this solution isn't a magic bullet fixing everything. TLS is not > fixable with this approach because TLS isn't peekable channel, hence the > watch won't help. > > We can have other ways to hack it to make it work, e.g. we can make TLS > channels peekable too, but I don't want to pollute the TLS iochannel only > for this: let's hope handshake will arrive soon so we can drop these code. > > For now, when TLS is involved with such old machine type (7.2-), one needs > to migrate it out with any combination of features except enabling all > four (multifd + TLS + postcopy + preempt) to a patched binary, after that, > all combinations will start working. > > BugLink: https://redhat.atlassian.net/browse/RHEL-254933 > Fixes: e27418861288 ("migration: enable multifd and postcopy together") > Reported-by: Juraj Marcin <[email protected]> > Signed-off-by: Peter Xu <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
