q-dev: fix and unify device detach on frontend shutdown

qubes/ext/block.py

@@ -643,28 +643,17 @@ async def on_domain_shutdown(self, vm, event, **_kwargs):
         """
         # pylint: disable=unused-argument
 
-        new_cache = {}
-        for domain in vm.app.domains.values():
-            new_cache[domain.name] = {}
-            if domain == vm:
-                for dev_id, front_vm in self.devices_cache[domain.name].items():
-                    dev = BlockDevice(Port(vm, dev_id, "block"))
-                    vm.fire_event("device-removed:block", port=dev.port)
-                    if front_vm is None:
-                        continue
-                    await self.detach_and_notify(front_vm, dev.port)
-                continue
-            for dev_id, front_vm in self.devices_cache[domain.name].items():
-                if front_vm == vm:
-                    dev = BlockDevice(Port(vm, dev_id, "block"))
-                    asyncio.ensure_future(
-                        front_vm.fire_event_async(
-                            "device-detach:block", port=dev.port
-                        )
-                    )
-                else:
-                    new_cache[domain.name][dev_id] = front_vm
-        self.devices_cache = new_cache.copy()
+        # devices exposed by the shutting-down backend vm:
+        # notify that they are gone and detach them from frontend vms
+        for dev_id, front_vm in self.devices_cache[vm.name].items():
+            dev = BlockDevice(Port(vm, dev_id, "block"))
+            vm.fire_event("device-removed:block", port=dev.port)
+            if front_vm is not None:
+                await self.detach_and_notify(front_vm, dev.port)
+        self.devices_cache[vm.name] = {}
+
+        # devices attached to shutting-down frontend vm: notify detach
+        utils.detach_attached_devices_on_shutdown(self, vm, BlockDevice)
 
     def ensure_detach(self, vm, port):
         """

qubes/ext/utils.py

@@ -109,6 +109,47 @@ def device_list_change(
     asyncio.ensure_future(resolve_conflicts_and_attach(ext, to_attach))
 
 
+def detach_attached_devices_on_shutdown(
+    ext: qubes.ext.Extension,
+    vm,
+    device_class: Type[qubes.device_protocol.DeviceInfo],
+):
+    """
+    Notify about devices attached to a shutting-down frontend VM.
+
+    For every device attached to ``vm`` fire a ``device-detach`` event with the
+    proper backend :py:class:`Port` and mark it as no longer attached in the
+    cache. The backend keeps exposing the device, so the cache entry is reset
+    to ``None``.
+    """
+    devclass = device_class.__name__[: -len("Device")].lower()
+
+    for backend_name, ports in ext.devices_cache.items():
+        if backend_name == vm.name:
+            # devices exposed by the shutting-down vm are handled by the caller
+            continue
+        try:
+            backend = vm.app.domains[backend_name]
+        except KeyError:
+            continue
+        for port_id, front_vm in ports.items():
+            if front_vm != vm:
+                continue
+            device = device_class(
+                Port(
+                    backend_domain=backend,
+                    port_id=port_id,
+                    devclass=devclass,
+                )
+            )
+            ports[port_id] = None
+            asyncio.ensure_future(
+                vm.fire_event_async(
+                    f"device-detach:{devclass}", port=device.port
+                )
+            )
+
+
 async def resolve_conflicts_and_attach(ext, to_attach):
     for _, frontends in to_attach.items():
         if len(frontends) > 1:

qubes/tests/devices_block.py

@@ -1303,3 +1303,49 @@ def test_083_on_startup_already_attached(self):
         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_090_on_domain_shutdown_frontend(self):
+        # a frontend that has a device attached is shutting down;
+        # the detach event is expected
+        back, front = self.added_assign_setup()
+
+        exp_dev = qubes.ext.block.BlockDevice(Port(back, "sda", "block"))
+        self.ext.devices_cache = {"sys-usb": {"sda": front}, "front-vm": {}}
+
+        loop = asyncio.get_event_loop()
+        with mock.patch("asyncio.ensure_future"):
+            loop.run_until_complete(self.ext.on_domain_shutdown(front, None))
+
+        front.fire_event_async.assert_called_with(
+            "device-detach:block", port=exp_dev.port
+        )
+        self.assertEqual(
+            front.fire_event_async.call_args.kwargs["port"].backend_domain,
+            back,
+        )
+        self.assertEqual(
+            self.ext.devices_cache,
+            {"sys-usb": {"sda": None}, "front-vm": {}},
+        )
+
+    def test_091_on_domain_shutdown_backend(self):
+        # a backend exposing an attached device is shutting down:
+        # the device is removed and detached from its frontend
+        back, front = self.added_assign_setup()
+
+        exp_dev = qubes.ext.block.BlockDevice(Port(back, "sda", "block"))
+        self.ext.devices_cache = {"sys-usb": {"sda": front}}
+        self.ext.detach_and_notify = AsyncMock()
+
+        loop = asyncio.get_event_loop()
+        with mock.patch("asyncio.ensure_future"):
+            loop.run_until_complete(self.ext.on_domain_shutdown(back, None))
+
+        self.assertEqual(
+            back.fired_events[
+                ("device-removed:block", frozenset({("port", exp_dev.port)}))
+            ],
+            1,
+        )
+        self.ext.detach_and_notify.assert_called_once_with(front, exp_dev.port)
+        self.assertEqual(self.ext.devices_cache, {"sys-usb": {}})