From 0b78d313eb40ae7e37e41946693544a1d0ea53cf Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Thu, 1 Oct 2026 22:41:50 +0800 Subject: [PATCH] fix(qemu): release the datapath after the process is gone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QEMU is the guest side of every adapter TAP, and uBridge's tap delete refuses a device another process holds open ('Device or resource busy') — a best-effort delete whose failure is suppressed. QEMU's stop did the delete first and stopped the process after, so every node stop or project delete leaked the persistent TAPs: found by inspecting the host after a project delete (16 taps left behind, carrier down, no holder), and the held-tap behaviour reproduced against uBridge directly. Stop the QEMU process first, then remove the TAPs and the per-link kernel bridges — process side first, then the devices, the same lesson as IOU's reverse stop order. Verified live: node stop and project delete now leave no taps and no processes behind, while a restart recreates the TAPs. The adapter anchor naming also goes through the shared utils.kernel_anchor helper, like the other node types. --- docs/features/qemu-kernel-datapath.md | 18 +++++++--- gns3server/compute/qemu/qemu_vm.py | 19 +++++++---- .../compute/qemu/test_qemu_kernel_datapath.py | 33 ++++++++++++++++++- 3 files changed, 58 insertions(+), 12 deletions(-) diff --git a/docs/features/qemu-kernel-datapath.md b/docs/features/qemu-kernel-datapath.md index 9ee177e77..91ee4fc8d 100644 --- a/docs/features/qemu-kernel-datapath.md +++ b/docs/features/qemu-kernel-datapath.md @@ -83,11 +83,15 @@ datapaths (delete + re-create, or a project reopen). * **Life (running)** — the TAP is never created or destroyed again. Link create/delete/switch, suspend, capture and markers only change *what is attached to it*. -* **Death (node stop)** — `_remove_taps` un-persists the TAPs (`tap delete`; - QEMU's open fd keeps each device alive until it exits) and deletes the - per-link kernel bridges the node still holds. Deleting a node never runs the - link-teardown path, so those bridges would otherwise stay behind as empty - orphans; a restart rebuilds them from the NIO. +* **Death (node stop)** — the QEMU process is stopped **first**, then + `_remove_taps` un-persists the TAPs (`tap delete`) and deletes the + per-link kernel bridges the node still holds. The order matters: uBridge's + `tap delete` refuses a device another process holds open ("Device or + resource busy") and that best-effort delete is suppressed, so deleting the + TAPs while QEMU still ran leaked every persistent TAP (process side first, + then the devices — the same lesson as IOU's reverse stop order). Deleting + a node never runs the link-teardown path, so those bridges would otherwise + stay behind as empty orphans; a restart rebuilds them from the NIO. ## Link operations @@ -176,3 +180,7 @@ TAPs bound to the IOL fabric; serial links stay relay). **Dynamips has landed** too — see `docs/features/dynamips-kernel-datapath.md` (the hypervisor opens uBridge-created TAPs with `nio create_tap`: the same external-fd-holder shape QEMU uses; serial/ATM/POS ports stay relay). +**The Ethernet switch has landed** — see +`docs/features/ethernet-switch-kernel-datapath.md` (the switch absorbs a +peer's anchor into its own kernel bridge; switch-to-switch cascades stay +relay for now). diff --git a/gns3server/compute/qemu/qemu_vm.py b/gns3server/compute/qemu/qemu_vm.py index 9309e25b5..7467acdcf 100644 --- a/gns3server/compute/qemu/qemu_vm.py +++ b/gns3server/compute/qemu/qemu_vm.py @@ -40,6 +40,7 @@ import gns3server from gns3server.schemas.compute.qemu_nodes import Qemu, QemuPlatform from gns3server.utils import parse_version from gns3server.utils.asyncio import cancellable_wait_run_in_executor, subprocess_check_output +from gns3server.utils.kernel_anchor import kernel_anchor_name from ...utils import int_to_macaddress, is_ipv6_enabled, macaddress_to_int from ...utils.asyncio import monitor_process @@ -1232,7 +1233,6 @@ class QemuVM(KernelDatapathMixin, BaseNode): Stops this QEMU VM. """ - await self._stop_ubridge() async with self._execute_lock: # stop the QEMU process self._hw_virtualization = False @@ -1274,6 +1274,14 @@ class QemuVM(KernelDatapathMixin, BaseNode): await self._export_config() await super().stop() + # Release the datapath only once QEMU is gone: the process holds the + # adapter TAPs open (it is the guest side), and uBridge's tap delete + # refuses a held device ("Device or resource busy") — deleting them + # while QEMU still ran leaked the persistent TAPs, because that + # best-effort delete is suppressed. Order: process side first, then + # the devices (the same lesson as IOU's reverse stop order). + await self._stop_ubridge() + async def _open_qemu_monitor_connection_vm(self, timeout=10): """ Opens a connection to the QEMU monitor. @@ -1494,13 +1502,12 @@ class QemuVM(KernelDatapathMixin, BaseNode): def _tap_name(self, adapter_number, port_number=0): """ - Deterministic anchor TAP name for an adapter port. The ``gq`` prefix - keeps it out of the ``gns3`` bridge/TAP name space (and apart from - Docker's ``gv``/``gc`` veth names); 8 hex chars of the node id plus - adapter/port keep it unique and within IFNAMSIZ (15). + Deterministic anchor TAP name for an adapter port (the shared + utils.kernel_anchor naming contract — the controller names a peer's + anchor with the same function when an Ethernet switch absorbs it). """ - return f"gq{self._id.replace('-', '')[:8]}e{adapter_number}p{port_number}" + return kernel_anchor_name("qemu", self._id, adapter_number, port_number) def _kernel_host_ifc(self, adapter_number, port_number=0): """ diff --git a/tests/compute/qemu/test_qemu_kernel_datapath.py b/tests/compute/qemu/test_qemu_kernel_datapath.py index a568e4327..707f582c1 100644 --- a/tests/compute/qemu/test_qemu_kernel_datapath.py +++ b/tests/compute/qemu/test_qemu_kernel_datapath.py @@ -270,7 +270,7 @@ async def test_update_nio_binding_reapplies_filters_on_the_anchor(vm): vm._ubridge_send.assert_any_call(f'tc reset "{TAP0}"') vm._ubridge_send.assert_any_call(f'tc netem set "{TAP0}" delay 10') - assert not any(f'brctl addif' in str(c) for c in vm._ubridge_send.call_args_list) + assert not any(f"brctl addif" in str(c) for c in vm._ubridge_send.call_args_list) @pytest.mark.asyncio @@ -346,3 +346,34 @@ async def test_capture_on_a_relay_link_uses_the_relay_bridge(vm): await vm.stop_capture(0) vm._ubridge_send.assert_any_call(f"bridge stop_capture QEMU-{vm.id}-0") + + +@pytest.mark.asyncio +async def test_stop_stops_the_process_before_deleting_the_taps(vm): + """ + The QEMU process is the guest side of every adapter TAP, and uBridge's + tap delete refuses a held device ("Device or resource busy") — that + best-effort delete is suppressed, so removing the taps while QEMU still + ran leaked the persistent TAPs. The process must be gone first (the same + lesson as IOU's reverse stop order: process side, then the devices). + """ + + _running(vm) + order = [] + + async def fake_termination(process, timeout=None): + order.append("process") + + async def fake_stop_ubridge(): + order.append("taps") + + with ( + patch("gns3server.utils.asyncio.wait_for_process_termination", new=fake_termination), + patch.object(vm, "_stop_ubridge", new=fake_stop_ubridge), + asyncio_patch("gns3server.compute.qemu.qemu_vm.QemuVM._export_config"), + asyncio_patch("gns3server.compute.qemu.qemu_vm.QemuVM._clear_save_vm_stated"), + asyncio_patch("gns3server.compute.base_node.BaseNode.stop"), + ): + await vm.stop() + + assert order == ["process", "taps"]