From ab03afed098d4da9731b4ef3c922468b44fb7517 Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Thu, 9 Jul 2026 23:50:43 +0200 Subject: [PATCH 1/2] q-dev: mark ancestors of attached devices as busy we want to avoid the situation when we attach device which sub-devices are already attached and in the same time we want to keep this device visible. previous behavior removing the parent from QDB and hide it entirely instead of just blocking attachment, but then we have subdevice with parent that "does not exist" from point of Qubes The busy=True means that this device cannot be attached --- qubes/device_protocol.py | 52 +++++++ qubes/ext/block.py | 148 ++++++++++++++++++- qubes/ext/utils.py | 3 + qubes/tests/devices.py | 46 ++++++ qubes/tests/devices_block.py | 273 +++++++++++++++++++++++++++++++++++ 5 files changed, 520 insertions(+), 2 deletions(-) diff --git a/qubes/device_protocol.py b/qubes/device_protocol.py index 61a8248e3..e1e8426ba 100644 --- a/qubes/device_protocol.py +++ b/qubes/device_protocol.py @@ -67,6 +67,32 @@ def qbool(value): return bool(value) +def qbool_untrusted_busy(untrusted_busy, log=None) -> bool: + """ + Parse the untrusted ``busy`` marker read from a backend VM's QubesDB. + + A missing value (:py:obj:`None`) or a boolean-false literal + (``False``/``0``/``no``/``off``) means the device is free. A + boolean-true literal (``True``/``1``/``yes``/``on``) means it is busy. + + Any other, unparsable value is treated as **busy**: it's safer to avoid + attachment of such device. Such a case is logged as a warning. + """ + if untrusted_busy is None: + return False + if isinstance(untrusted_busy, bytes): + untrusted_busy = untrusted_busy.decode("ascii", errors="replace") + try: + return qbool(untrusted_busy.strip()) + except QubesValueError: + if log is not None: + log.warning( + "Invalid 'busy' marker %r, assuming device is busy", + untrusted_busy, + ) + return True + + class DeviceSerializer: """ Group of method for serialization of device properties. @@ -916,6 +942,7 @@ def __init__( parent: Optional["DeviceInfo"] = None, attachment: Optional[QubesVM] = None, device_id: Optional[str] = None, + busy: Optional[bool] = None, **kwargs, ): super().__init__(port, device_id) @@ -928,6 +955,7 @@ def __init__( self._interfaces = interfaces self._parent = parent self._attachment = attachment + self._busy = busy self.data = kwargs @@ -1082,6 +1110,17 @@ def attachment(self) -> Optional[QubesVM]: """ return self._attachment + @property + def busy(self) -> bool: + """ + Is the device currently busy (unavailable for attachment)? + + True when the device or one of its children is in use: attached + to a VM or used locally in the backend VM. The device remains + visible but cannot be attached until it is freed. + """ + return bool(self._busy) + def serialize(self) -> bytes: """ Serialize an object to be transmitted via Qubes API. @@ -1105,6 +1144,11 @@ def serialize(self) -> bytes: "attachment", self.attachment.name ) + if self.busy: + properties += b" " + DeviceSerializer.pack_property( + "busy", "True" + ) + properties += b" " + DeviceSerializer.pack_property( "interfaces", "".join(repr(ifc) for ifc in self.interfaces) ) @@ -1188,6 +1232,14 @@ def _deserialize( del properties["parent_port_id"] del properties["parent_devclass"] + # A missing property means free; otherwise parse strictly. An + # invalid value raises QubesValueError, which the caller + # (deserialize) turns into an UnknownDevice. + untrusted_busy = properties.pop("busy", None) + properties["busy"] = ( + qbool(untrusted_busy) if untrusted_busy is not None else False + ) + return cls(**properties) @property diff --git a/qubes/ext/block.py b/qubes/ext/block.py index 37f024708..9185ccdec 100644 --- a/qubes/ext/block.py +++ b/qubes/ext/block.py @@ -49,6 +49,10 @@ SYSTEM_DISKS_DOM0_KERNEL = SYSTEM_DISKS + ("xvdd",) +def get_busy_qdb_key(port_id): + return f"/qubes-block-devices/{port_id}/busy" + + class BlockDevice(qubes.device_protocol.DeviceInfo): def __init__(self, port: qubes.device_protocol.Port): @@ -195,7 +199,10 @@ def parent_device(self) -> Optional[qubes.device_protocol.DeviceInfo]: parent_ident, sep, interface_num = self._sanitize( untrusted_parent_info ).partition(":") - devclass = "usb" if sep == ":" else "block" + if sep == ":" or re.match(r"^\d+-", parent_ident): + devclass = "usb" + else: + devclass = "block" if not parent_ident: return None try: @@ -242,6 +249,24 @@ def _is_attached_to(self, vm): return True return False + @property + def busy(self) -> bool: + """ + Is this device busy? A device is considered busy if any of its children + are in use (e.g., a partition of a disk is attached). + """ + if self._busy is None: + if not self.backend_domain or not self.backend_domain.is_running(): + # don't cache this value + return False + untrusted_busy = self.backend_domain.untrusted_qdb.read( + get_busy_qdb_key(self.port_id) + ) + self._busy = qubes.device_protocol.qbool_untrusted_busy( + untrusted_busy, self.backend_domain.log + ) + return self._busy + @property # type: ignore[misc] def device_id(self) -> str: """ @@ -285,6 +310,18 @@ def _sanitize( c if c in set(safe_chars) else "_" for c in untrusted_device_desc ) + def mark_busy(self, busy: bool) -> None: + """Set or clear the busy marker in the backend VM's QDB.""" + key = get_busy_qdb_key(self.port_id) + if not busy: + try: + self.backend_domain.untrusted_qdb.rm(key) + except Exception: # pylint: disable=broad-except + pass + else: + self.backend_domain.untrusted_qdb.write(key, b"True") + self._busy = busy + def _try_get_block_device_info(app, disk): if disk.get("type") != "block": @@ -330,6 +367,19 @@ def on_domain_init_load(self, vm, event): for dev in self.on_device_list_block(vm, None) ) self.devices_cache[vm.name] = current_devices + # Re-establish busy markers for partitions already attached + # before this qubesd instance started (e.g. after qubesd restart). + for port_id, front_vm in device_attachments.items(): + if front_vm is not None: + self._mark_parents_busy( + BlockDevice( + Port( + backend_domain=vm, + port_id=port_id, + devclass="block", + ) + ) + ) else: self.devices_cache[vm.name] = {} @@ -383,7 +433,7 @@ def device_get(vm, port_id): if not untrusted_qubes_device_attrs: return None return BlockDevice( - Port(backend_domain=vm, port_id=port_id, devclass="block") + Port(backend_domain=vm, port_id=port_id, devclass="block"), ) @qubes.ext.handler("device-list:block") @@ -520,6 +570,12 @@ def pre_attachment_internal( ) raise qubes.exc.UnrecognizedDevice() + if device.busy: + raise qubes.exc.QubesValueError( + f"Device {device} is busy: " + "a child partition is currently passed through to a VM" + ) + # validate options for option, value in options.items(): if option == "frontend-dev": @@ -585,6 +641,86 @@ def pre_attachment_internal( vm, options.get("devtype", "disk") ) + self._mark_parents_busy(device) + + def _mark_parents_busy(self, device): + """Set busy for the parent chain in the backend VM's QDB.""" + try: + if not device.parent_device: + return + if not device.backend_domain.is_running(): + # something change in meantime, ignore it + return + self._mark_parent_chain(device, busy=True) + except Exception: # pylint: disable=broad-except + # best effort + pass + + def _unmark_parents_busy(self, device): + """ + Clear busy for the parent chain in the backend VM's QDB. + + Ancestors shared with other still-attached devices (sibling + partitions of the same disk, or sibling block devices of the same + USB device) are kept busy. + """ + backend = device.backend_domain + if not backend.is_running(): + # something change in meantime, ignore it + return + + try: + keep_busy = set() + for dev_id, front_vm in self.devices_cache[backend.name].items(): + if front_vm is None or dev_id == device.port_id: + continue + other = BlockDevice( + Port(backend_domain=backend, port_id=dev_id, + devclass="block") + ) + for ancestor in self._parent_chain(other): + keep_busy.add((ancestor.devclass, ancestor.port_id)) + self._mark_parent_chain(device, busy=False, skip=keep_busy) + except Exception: # pylint: disable=broad-except + # best effort + pass + + @staticmethod + def _parent_chain(device): + """Yield ancestors of the device.""" + current = device + # limit the depth: better to leave some ancestors unmarked than to + # loop forever on a corrupted or malicious parent chain + for _ in range(8): + parent = current.parent_device + if parent is None or isinstance( + parent, qubes.device_protocol.UnknownDevice + ): + break + yield parent + if parent.devclass == "usb": + break # USB is the top of the hierarchy + current = parent + + def _mark_parent_chain(self, device, busy, skip=frozenset()): + """ + Walk the parent_device chain and set busy for each ancestor. + + Ancestors listed in `skip` (as (devclass, port_id) pairs) are left + untouched. Trigger writes are deferred until all busy keys have + been written so that watch callbacks see a consistent QDB state. + """ + pending_triggers = set() + + for parent in self._parent_chain(device): + if (parent.devclass, parent.port_id) in skip: + continue + parent.mark_busy(busy) + pending_triggers.add(f"/qubes-{parent.devclass}-devices") + + for trigger in pending_triggers: + device.backend_domain.untrusted_qdb.write(trigger, b"") + @qubes.ext.handler("domain-start") async def on_domain_start(self, vm, _event, **_kwargs): # pylint: disable=unused-argument @@ -598,6 +734,8 @@ async def on_domain_start(self, vm, _event, **_kwargs): for device in assignment.devices: if isinstance(device, qubes.device_protocol.UnknownDevice): continue + if device.busy: + continue if device.attachment: continue if not assignment.matches(device): @@ -662,6 +800,11 @@ async def on_domain_shutdown(self, vm, event, **_kwargs): "device-detach:block", port=dev.port ) ) + # the frontend is gone: clear the busy markers + self.devices_cache[domain.name][dev_id] = None + self._unmark_parents_busy( + BlockDevice(Port(domain, dev_id, "block")) + ) else: new_cache[domain.name][dev_id] = front_vm self.devices_cache = new_cache.copy() @@ -703,4 +846,5 @@ def on_device_pre_detached_block(self, vm, event, port): device=attached_device, vm=vm, options=options ) ) + self._unmark_parents_busy(attached_device) break diff --git a/qubes/ext/utils.py b/qubes/ext/utils.py index 832d2ba7d..7644b7e54 100644 --- a/qubes/ext/utils.py +++ b/qubes/ext/utils.py @@ -92,6 +92,9 @@ def device_list_change( assignment.matches(device) and device.port_id in added and device.port_id not in attached + # do not auto-attach busy devices: attach would be + # refused anyway, leaving an unhandled task exception + and not device.busy ): frontends = to_attach.get(device.port_id, {}) # make it unique diff --git a/qubes/tests/devices.py b/qubes/tests/devices.py index ab909aa77..4d745478f 100644 --- a/qubes/tests/devices.py +++ b/qubes/tests/devices.py @@ -579,6 +579,52 @@ def test_030_serialize_and_deserialize(self): self.assertEqual(deserialized.interfaces[0], device.interfaces[0]) self.assertEqual(deserialized.interfaces[1], device.interfaces[1]) + def test_040_serialize_busy(self): + device = DeviceInfo( + Port(backend_domain=self.vm, port_id="1-1.1.1", devclass="bus"), + device_id="0000:0000::?******", + busy=True, + ) + self.assertIn(b"busy='True'", device.serialize()) + + def test_041_serialize_not_busy(self): + device = DeviceInfo( + Port(backend_domain=self.vm, port_id="1-1.1.1", devclass="bus"), + device_id="0000:0000::?******", + busy=False, + ) + self.assertNotIn(b"busy=", device.serialize()) + + def test_042_deserialize_busy(self): + serialized = ( + b"1-1.1.1 device_id='0000:0000::?******' port_id='1-1.1.1' " + b"devclass='bus' backend_domain='vm' interfaces='?******' " + b"busy='True'" + ) + actual = DeviceInfo.deserialize(serialized, self.vm) + self.assertNotIsInstance(actual, UnknownDevice) + self.assertTrue(actual.busy) + + def test_043_deserialize_not_busy(self): + serialized = ( + b"1-1.1.1 device_id='0000:0000::?******' port_id='1-1.1.1' " + b"devclass='bus' backend_domain='vm' interfaces='?******'" + ) + actual = DeviceInfo.deserialize(serialized, self.vm) + self.assertNotIsInstance(actual, UnknownDevice) + self.assertFalse(actual.busy) + + def test_044_deserialize_invalid_busy(self): + # strict on the protocol layer: an invalid value makes the whole + # device unrecognized rather than silently guessing + serialized = ( + b"1-1.1.1 device_id='0000:0000::?******' port_id='1-1.1.1' " + b"devclass='bus' backend_domain='vm' interfaces='?******' " + b"busy='garbage'" + ) + actual = DeviceInfo.deserialize(serialized, self.vm) + self.assertIsInstance(actual, UnknownDevice) + class TC_03_DeviceAssignment(qubes.tests.QubesTestCase): def setUp(self): diff --git a/qubes/tests/devices_block.py b/qubes/tests/devices_block.py index dfd2d0d3f..858d460e0 100644 --- a/qubes/tests/devices_block.py +++ b/qubes/tests/devices_block.py @@ -117,6 +117,12 @@ def __init__(self, data): def read(self, key): return self._data.get(key, None) + def write(self, key, value): + self._data[key] = value + + def rm(self, key): + self._data.pop(key, None) + def list(self, prefix): return [key for key in self._data if key.startswith(prefix)] @@ -217,11 +223,54 @@ def get_qdb(mode): return result +class FakeUSBDevice: + """Minimal USB parent device""" + + devclass = "usb" + + def __init__(self, backend_domain, port_id): + self.backend_domain = backend_domain + self.port_id = port_id + self.mark_busy_calls = [] + + def mark_busy(self, busy): + self.mark_busy_calls.append(busy) + key = f"/qubes-usb-devices/{self.port_id}/busy" + if busy: + self.backend_domain.untrusted_qdb.write(key, b"True") + else: + self.backend_domain.untrusted_qdb.rm(key) + + +def get_usb_storage_qdb(): + """ + QubesDB content of a multi-LUN USB storage device: two disks (sda, sdb) + under the same USB device 1-1, sda having two partitions (sda1, sda2). + """ + result = {} + for disk in ("sda", "sdb"): + result[f"/qubes-block-devices/{disk}"] = b"" + result[f"/qubes-block-devices/{disk}/desc"] = b"Test device" + result[f"/qubes-block-devices/{disk}/size"] = b"1024000" + result[f"/qubes-block-devices/{disk}/mode"] = b"w" + result[f"/qubes-block-devices/{disk}/parent"] = b"1-1" + for part in ("sda1", "sda2"): + result[f"/qubes-block-devices/{part}"] = b"" + result[f"/qubes-block-devices/{part}/desc"] = b"Test device" + result[f"/qubes-block-devices/{part}/size"] = b"512000" + result[f"/qubes-block-devices/{part}/mode"] = b"w" + result[f"/qubes-block-devices/{part}/parent"] = b"sda" + return result + + class TC_00_Block(qubes.tests.QubesTestCase): def setUp(self): super().setUp() self.ext = qubes.ext.block.BlockDeviceExtension() + # Extension is a singleton: restore methods that other tests + # replaced with mocks directly on the shared instance + self.ext.__dict__.pop("attach_and_notify", None) def test_000_device_get(self): vm = TestVM( @@ -1278,6 +1327,230 @@ def test_082_on_startup_multiple_assignments_dev(self): self.ext.attach_and_notify.call_args[0][1].options, {"any": "did"} ) + def busy_setup(self): + """ + Backend VM exposing a multi-LUN USB storage device (sda+sda1+sda2, + sdb, all under USB device 1-1) and a front VM to attach to. + """ + back = TestVM( + get_usb_storage_qdb(), + domain_xml=domain_xml_template.format(""), + name="sys-usb", + ) + back.app.vmm.configure_mock(**{"offline_mode": False}) + back.devices["block"] = TestDeviceCollection( + backend_vm=back, devclass="block" + ) + back.devices["usb"] = TestDeviceCollection( + backend_vm=back, devclass="usb" + ) + for dev_name in ("sda", "sda1", "sda2", "sdb"): + back.devices["block"]._exposed.append( + qubes.ext.block.BlockDevice(Port(back, dev_name, "block")) + ) + usb = FakeUSBDevice(back, "1-1") + back.devices["usb"]._exposed.append(usb) + + front = TestVM( + {}, domain_xml=domain_xml_template.format(""), name="front-vm" + ) + dom0 = TestVM( + {}, name="dom0", domain_xml=domain_xml_template.format("") + ) + back.app.domains["sys-usb"] = back + back.app.domains["front-vm"] = front + back.app.domains[0] = dom0 + back.app.domains["dom0"] = dom0 + front.app = back.app + dom0.app = back.app + return back, front, usb + + def test_090_device_get_busy(self): + # boolean-true literals mean busy + for value in (b"True", b"true", b"1", b"yes", b"on"): + qdb = get_qdb(mode="w") + qdb["/qubes-block-devices/sda/busy"] = value + vm = TestVM(qdb) + device_info = self.ext.device_get(vm, "sda") + self.assertTrue(device_info.busy, value) + + def test_091_device_get_not_busy(self): + # no key at all means free + vm = TestVM(get_qdb(mode="w")) + device_info = self.ext.device_get(vm, "sda") + self.assertFalse(device_info.busy) + # boolean-false literals also mean free + for value in (b"False", b"false", b"0", b"no", b"off"): + qdb = get_qdb(mode="w") + qdb["/qubes-block-devices/sda/busy"] = value + vm = TestVM(qdb) + device_info = self.ext.device_get(vm, "sda") + self.assertFalse(device_info.busy, value) + + def test_0911_device_get_invalid_busy_fails_closed(self): + # an unparsable value is treated as busy (fail-closed) and logged + qdb = get_qdb(mode="w") + qdb["/qubes-block-devices/sda/busy"] = b"garbage" + vm = TestVM(qdb) + device_info = self.ext.device_get(vm, "sda") + self.assertTrue(device_info.busy) + vm.log.warning.assert_called_once() + + def test_092_attach_busy_device_refused(self): + qdb = get_qdb(mode="w") + qdb["/qubes-block-devices/sda/busy"] = b"True" + back_vm = TestVM(name="sys-usb", qdb=qdb) + vm = TestVM({}, domain_xml=domain_xml_template.format("")) + dev = qubes.ext.block.BlockDevice(Port(back_vm, "sda", "block")) + with self.assertRaises(qubes.exc.QubesValueError): + self.ext.on_device_pre_attached_block(vm, "", dev, {}) + self.assertFalse(vm.libvirt_domain.attachDevice.called) + + def test_093_attach_partition_marks_parents_busy(self): + back, front, usb = self.busy_setup() + dev = self.ext.device_get(back, "sda1") + self.ext.on_device_pre_attached_block(front, "", dev, {}) + self.assertEqual( + back.untrusted_qdb.read("/qubes-block-devices/sda/busy"), b"True" + ) + self.assertEqual( + back.untrusted_qdb.read("/qubes-usb-devices/1-1/busy"), b"True" + ) + self.assertEqual(usb.mark_busy_calls, [True]) + + def test_094_attach_whole_disk_marks_usb_busy(self): + back, front, usb = self.busy_setup() + dev = self.ext.device_get(back, "sdb") + self.ext.on_device_pre_attached_block(front, "", dev, {}) + self.assertEqual( + back.untrusted_qdb.read("/qubes-usb-devices/1-1/busy"), b"True" + ) + self.assertEqual(usb.mark_busy_calls, [True]) + # the attached device itself is not marked by qubesd + self.assertIsNone( + back.untrusted_qdb.read("/qubes-block-devices/sdb/busy") + ) + + def test_095_detach_unmarks_parents(self): + back, front, usb = self.busy_setup() + back.untrusted_qdb.write("/qubes-block-devices/sda/busy", b"True") + back.untrusted_qdb.write("/qubes-usb-devices/1-1/busy", b"True") + self.ext.devices_cache["sys-usb"] = {"sda1": None} + + dev = self.ext.device_get(back, "sda1") + self.ext._unmark_parents_busy(dev) + self.assertIsNone( + back.untrusted_qdb.read("/qubes-block-devices/sda/busy") + ) + self.assertIsNone( + back.untrusted_qdb.read("/qubes-usb-devices/1-1/busy") + ) + self.assertEqual(usb.mark_busy_calls, [False]) + + def test_096_detach_sibling_disk_attached_keeps_usb_busy(self): + # sda still attached while sdb is being detached + back, front, usb = self.busy_setup() + back.untrusted_qdb.write("/qubes-usb-devices/1-1/busy", b"True") + self.ext.devices_cache["sys-usb"] = {"sda": front, "sdb": None} + + dev = self.ext.device_get(back, "sdb") + self.ext._unmark_parents_busy(dev) + self.assertEqual( + back.untrusted_qdb.read("/qubes-usb-devices/1-1/busy"), b"True" + ) + self.assertEqual(usb.mark_busy_calls, []) + + def test_097_detach_sibling_partition_attached_keeps_parents_busy(self): + # sda2 still attached while sda1 is being detached + back, front, usb = self.busy_setup() + back.untrusted_qdb.write("/qubes-block-devices/sda/busy", b"True") + back.untrusted_qdb.write("/qubes-usb-devices/1-1/busy", b"True") + self.ext.devices_cache["sys-usb"] = {"sda1": None, "sda2": front} + + dev = self.ext.device_get(back, "sda1") + self.ext._unmark_parents_busy(dev) + self.assertEqual( + back.untrusted_qdb.read("/qubes-block-devices/sda/busy"), b"True" + ) + self.assertEqual( + back.untrusted_qdb.read("/qubes-usb-devices/1-1/busy"), b"True" + ) + self.assertEqual(usb.mark_busy_calls, []) + + def test_098_detach_partition_sibling_disk_attached(self): + # sdb still attached while sda1 is being detached: sda can be + # unmarked, but the shared USB ancestor must stay busy + back, front, usb = self.busy_setup() + back.untrusted_qdb.write("/qubes-block-devices/sda/busy", b"True") + back.untrusted_qdb.write("/qubes-usb-devices/1-1/busy", b"True") + self.ext.devices_cache["sys-usb"] = {"sda1": None, "sdb": front} + + dev = self.ext.device_get(back, "sda1") + self.ext._unmark_parents_busy(dev) + self.assertIsNone( + back.untrusted_qdb.read("/qubes-block-devices/sda/busy") + ) + self.assertEqual( + back.untrusted_qdb.read("/qubes-usb-devices/1-1/busy"), b"True" + ) + self.assertEqual(usb.mark_busy_calls, []) + + def test_099_on_startup_busy_not_auto_attached(self): + back, front = self.added_assign_setup() + back.untrusted_qdb.write("/qubes-block-devices/sda/busy", b"True") + + exp_dev = qubes.ext.block.BlockDevice(Port(back, "sda", "block")) + assign = DeviceAssignment( + VirtualDevice(exp_dev.port, exp_dev.device_id), mode="auto-attach" + ) + + front.devices["block"]._assigned.append(assign) + back.devices["block"]._exposed.append(exp_dev) + + self.ext.attach_and_notify = Mock() + loop = asyncio.get_event_loop() + with mock.patch("asyncio.ensure_future"): + loop.run_until_complete(self.ext.on_domain_start(front, None)) + self.ext.attach_and_notify.assert_not_called() + + def test_100_attach_required_busy_refused(self): + # 'required' mode is possible for block devices (unlike usb); + # attaching a required device must still be refused when it is busy + back, front = self.added_assign_setup() + back.untrusted_qdb.write("/qubes-block-devices/sda/busy", b"True") + + exp_dev = qubes.ext.block.BlockDevice(Port(back, "sda", "block")) + assignment = DeviceAssignment(exp_dev, mode="required") + front.devices["block"]._assigned.append(assignment) + back.devices["block"]._exposed.append(exp_dev) + front.fire_event_async = AsyncMock() + + loop = asyncio.get_event_loop() + with self.assertRaises(qubes.exc.QubesValueError): + loop.run_until_complete( + self.ext.attach_and_notify(front, assignment) + ) + self.assertFalse(front.libvirt_domain.attachDevice.called) + front.fire_event_async.assert_not_called() + + def test_101_attach_required_not_busy(self): + # sanity counterpart of test_100: the same required assignment + # attaches once the device is no longer busy + back, front = self.added_assign_setup() + + exp_dev = qubes.ext.block.BlockDevice(Port(back, "sda", "block")) + assignment = DeviceAssignment(exp_dev, mode="required") + front.devices["block"]._assigned.append(assignment) + back.devices["block"]._exposed.append(exp_dev) + front.fire_event_async = AsyncMock() + + loop = asyncio.get_event_loop() + loop.run_until_complete(self.ext.attach_and_notify(front, assignment)) + self.assertTrue(front.libvirt_domain.attachDevice.called) + front.fire_event_async.assert_called_once_with( + "device-attach:block", device=exp_dev, options=assignment.options + ) + def test_083_on_startup_already_attached(self): disk = """ From d0a4e33ea01a29346b254dcb6d92d85d941f449a Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Fri, 10 Jul 2026 16:22:49 +0200 Subject: [PATCH 2/2] q-dev: busyness include internal usage if one of subdevices are locally used (e.g. in sys-usb) the parent is considered busy --- qubes/ext/block.py | 59 ++++++++++++++++++++++++++++++------ qubes/tests/devices_block.py | 27 +++++++++++++++++ 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/qubes/ext/block.py b/qubes/ext/block.py index 9185ccdec..4c6ae4eb1 100644 --- a/qubes/ext/block.py +++ b/qubes/ext/block.py @@ -252,8 +252,9 @@ def _is_attached_to(self, vm): @property def busy(self) -> bool: """ - Is this device busy? A device is considered busy if any of its children - are in use (e.g., a partition of a disk is attached). + Is this device busy? A device is considered busy if it or any of + its children is in use: attached to a VM or used locally in the + backend VM (mounted, part of a device-mapper, enabled swap). """ if self._busy is None: if not self.backend_domain or not self.backend_domain.is_running(): @@ -570,10 +571,10 @@ def pre_attachment_internal( ) raise qubes.exc.UnrecognizedDevice() - if device.busy: + # DeviceAlreadyAttached is checked below + if device.busy and not device.attachment: raise qubes.exc.QubesValueError( - f"Device {device} is busy: " - "a child partition is currently passed through to a VM" + f"Device {device} is busy: it or one of its children is in use." ) # validate options @@ -656,6 +657,39 @@ def _mark_parents_busy(self, device): # best effort pass + async def _refresh_busy_after_detach(self, device): + """ + Ask the backend VM to refresh `busy` for the detached device's subtree + by re-triggering udev change events. + + The udev script rebuilds the state from what only the backend can + see (vbd attachments in sysfs and the local mount table), so this + never clears a marker owed to a local mount. + """ + backend = device.backend_domain + if not backend.is_running(): + return + # vbd teardown is asynchronous: refreshing while the vbd is still + # visible in the backend's sysfs would re-mark the device as busy + # with no later event to clear it. + # TODO: better idea? + await asyncio.sleep(1) + + name = device.port_id.replace("_", "/") + try: + await backend.run_service_for_stdio( + "qubes.RefreshBlockDevices", + user="root", + input=name.encode() + b"\n", + ) + except Exception: # pylint: disable=broad-except + backend.log.warning( + "block refresh failed for %s, " + "freeing parents while ignoring local use.", + device.port_id, + ) + self._unmark_parents_busy(device) + def _unmark_parents_busy(self, device): """ Clear busy for the parent chain in the backend VM's QDB. @@ -663,6 +697,9 @@ def _unmark_parents_busy(self, device): Ancestors shared with other still-attached devices (sibling partitions of the same disk, or sibling block devices of the same USB device) are kept busy. + + Fallback: prefer `_refresh_busy_after_detach`, which take into account + local use. """ backend = device.backend_domain if not backend.is_running(): @@ -801,9 +838,10 @@ async def on_domain_shutdown(self, vm, event, **_kwargs): ) ) # the frontend is gone: clear the busy markers - self.devices_cache[domain.name][dev_id] = None - self._unmark_parents_busy( - BlockDevice(Port(domain, dev_id, "block")) + detached = BlockDevice(Port(domain, dev_id, "block")) + detached.mark_busy(False) + asyncio.ensure_future( + self._refresh_busy_after_detach(detached) ) else: new_cache[domain.name][dev_id] = front_vm @@ -846,5 +884,8 @@ def on_device_pre_detached_block(self, vm, event, port): device=attached_device, vm=vm, options=options ) ) - self._unmark_parents_busy(attached_device) + attached_device.mark_busy(False) + asyncio.ensure_future( + self._refresh_busy_after_detach(attached_device) + ) break diff --git a/qubes/tests/devices_block.py b/qubes/tests/devices_block.py index 858d460e0..28c843a28 100644 --- a/qubes/tests/devices_block.py +++ b/qubes/tests/devices_block.py @@ -1551,6 +1551,33 @@ def test_101_attach_required_not_busy(self): "device-attach:block", device=exp_dev, options=assignment.options ) + def test_102_detach_clears_own_busy_marker(self): + qdb = get_qdb(mode="r") + qdb["/qubes-block-devices/sda/busy"] = b"True" + back_vm = TestVM(name="sys-usb", qdb=qdb) + device_xml = ( + '\n' + ' \n' + ' \n' + ' \n' + " \n" + ' \n' + '