[PATCH v2-hci-8.0 0/2] migration/cpr: lock the cpr state #VSTOR-145529

["Denis V. Lunev" <den@openvz.org>]

A guest died with SIGSEGV in find_fd() after a routine libvirtd restart.
The fd registry in cpr_state has no lock, and since we started saving
chardev connection fds it is reached from more than one thread.

A socket chardev saves and drops its connection fd in
tcp_chr_new_client() and tcp_chr_free_connection(), which run in the
chardev's GMainContext. For libvirt's monitor that context belongs to the
QMP iothread, while the guest agent channels and the serial pty stay on
the main loop. A libvirtd restart reconnects all of them at once, so two
threads run QLIST_INSERT_HEAD and QLIST_REMOVE on one list. The core on
mhw0501 shows the head left on an entry the iothread allocated and freed,
with the real list intact and unreachable behind it.

Taking the BQL in the chardev hooks is not an option. The chardev layer
never takes it, monitor_cleanup() stops the QMP iothread while holding
it, and the iothread exists precisely so out-of-band commands keep
working when the main thread will not release it. The lock belongs to the
registry.

The body is worded free of chardev and QLIST specifics on purpose: the
same text serves upstream, where cpr.c has been rewritten around a
GHashTable and nothing reaches the state off the main thread. Patch 1 is
that shared part, so it stops at the fd accessors. Patch 2 covers the
ramblock list, which upstream does not have.

v1 -> v2, all of it narrowing the lock to a leaf over the list alone:

- out of the vmstate walks. The save walk runs the subsection callbacks
  and hands each fd to the peer with a blocking write, and a load hook
  that saved an fd would deadlock against itself. The load side needs no
  lock anyway, running before qemu_create_early_backends().
- out of cpr_open_fd(), which held it across qemu_open().
- patch 1's commit message trimmed to match.

So cpr_state_save() now walks the fd list unlocked and can still race a
chardev accept. Closing that needs a copy of the list taken under the
lock, which first needs cpr_cpu_hotplug_needed() to use its opaque
instead of the global cpr_state.

Tested on dev-cl04 against this base: migrate-cpr-exec, migration-test
--full, check-unit and the raw iotests all pass. The qcow2 iotests leave
vz-kernel-cbt and vz-mt-vendor, which fail the same way on the parent
commit.

Denis V. Lunev (2):
  migration/cpr: lock the saved fds #VSTOR-145529
  migration/cpr: lock the ramblocks #VSTOR-145529

 migration/cpr.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 58 insertions(+), 11 deletions(-)


base-commit: 87f111d087cff956908ccb22daedb596b3734877
-- 
2.53.0