process_control_tx() took ownership of a port's rx/tx DeviceQueue once on VIRTIO_CONSOLE_PORT_OPEN(id, 1) and handed them to Port::start(), but never returned them to Console::queues[] on VIRTIO_CONSOLE_PORT_OPEN(id, 0) (port closed). The guest driver sends PORT_OPEN(1) again whenever a port is reopened (every userspace open()/close() of a virtio-console port, per drivers/char/virtio_console.c), so the second open of the same port hit queues[idx].take().expect(...) on an already-None slot and panicked, aborting the host process (observed via LXD's forklibkrun crashing while a lxd-agent activation port was reopened).
Make process_rx()/process_tx() return the Queue they own when their worker thread stops, and have Port::shutdown() collect it (including for ports with no input/output, which never got a worker thread to begin with) and hand it back to the caller. Console::process_control_tx() now calls Port::shutdown() on PORT_OPEN(id, 0) and puts the recovered queues back into queues[], so a later PORT_OPEN(id, 1) can legitimately take() them again instead of panicking. Also replace the remaining .expect() panics in the port-start path with a defensive skip-and-warn for the case of a spurious duplicate PORT_OPEN(1).
Extend the multiport-console test to close and reopen each port within the same VM run, reproducing the panic on unpatched code and passing with the fix.
Assisted-by: Cline:claude-sonnet-4.5
Activity
10
xypron21d agoI updated the patch to use a single queue for port open and port close.
slp17d ago@mtjhrc PTAAL
mtjhrc17d agoMy other comment (https://github.com/libkrun/libkrun/pull/918#discussion_r4231354948) is still unresolved. I realize it was vague about a solution because I’m not entirely sure myself.
As it stands, this PR could hang the main VMM event loop when the guest closes a console port connected to a full host-side pipe. It reuses Port::shutdown(), which waits for the TX worker to drain its output. The TX worker can get stuck in output.wait_until_writable(): https://github.com/libkrun/libkrun/blob/8ec6ebcb325ec05002a12485f43cbb6cf3610188/src/devices/src/virtio/console/process_tx.rs#L98-L102
We need to preserve that draining behavior during VM shutdown. Commands like ./chroot_vm ... cat something | grep whatever, podman usage, and our integration test suite rely on pending output being written before the host process exits.
Draining output during VM shutdown is different from handling a port close while the VM is still running. While the VM is running, we should not block the VMM event loop waiting for the host to consume output.
My previous comment suggested either asynchronous draining or a pause/yield operation. Having thought more about this, I think we should pursue pausing them. The main idea is to pause processing, not wait for pending output to finish draining.
Keeping the worker threads alive in a paused state looks easier: they could retain their queues and any in-progress descriptor state. Stopping the threads and returning their queues would also work, but returning only the queues would not preserve progress through a partially written TX descriptor. The workers access guest memory directly rather than maintaining a separate data buffer, but that partial-write progress still matters.
TX would also need an interruptible writable wait, similar to RX’s stopfd, so it can pause without waiting for the host to consume output.
Tested on Windows. I was able to replicate and validate the fix. Just a question about the code