q-dev: fix block auto-attach

Author: piotrbartman

qubes/api/admin.py

@@ -1227,7 +1227,7 @@ async def vm_device_available(self, endpoint):
         devices = self.fire_event_for_filter(devices, devclass=devclass)
         dev_info = {f'{dev.port_id}:{dev.device_id}':
                         dev.serialize().decode() for dev in devices}
-        return ''.join('{} {}\n'.format(port_id, dev_info[port_id])
+        return ''.join(f'{port_id} {dev_info[port_id]}\n'
                        for port_id in sorted(dev_info))
 
     @qubes.api.method('admin.vm.device.{endpoint}.Assigned', endpoints=(ep.name

qubes/device_protocol.py

@@ -895,7 +895,7 @@ def serialize(self) -> bytes:
                 if getattr(self, key) != getattr(default, key)))
 
         if self.attachment:
-            properties = DeviceSerializer.pack_property(
+            properties += b' ' + DeviceSerializer.pack_property(
                 'attachment', self.attachment.name)
 
         properties += b' ' + DeviceSerializer.pack_property(
@@ -1122,6 +1122,8 @@ def devices(self) -> List[DeviceInfo]:
             # could return UnknownDevice
             return [self.backend_domain.devices[self.devclass][self.port_id]]
         result = []
+        if self.device_id == "0000:0000::?******":
+            return result
         for dev in self.backend_domain.devices[self.devclass]:
             if dev.device_id == self.device_id:
                 result.append(dev)

qubes/ext/block.py

@@ -541,34 +541,41 @@ def pre_attachment_internal(
     @qubes.ext.handler('domain-start')
     async def on_domain_start(self, vm, _event, **_kwargs):
         # pylint: disable=unused-argument
-        for assignment in vm.devices['block'].get_assigned_devices():
-            self.notify_auto_attached(vm, assignment)
-
-    def notify_auto_attached(self, vm, assignment):
-        for device in assignment.devices:
-            if not assignment.matches(device):
-                print("Unrecognized identity, skipping attachment of device "
-                      f"from the port {assignment}", file=sys.stderr)
+        to_attach = {}
+        assignments = vm.devices['block'].get_assigned_devices()
+        # the most specific assignments first
+        for assignment in reversed(sorted(assignments)):
+            if assignment.required:
+                # already attached
                 continue
-
-            if assignment.mode.value == "ask-to-attach":
-                if vm.name != confirm_device_attachment(device,
-                                                        {vm: assignment}):
+                # TODO: notify?
+            for device in assignment.devices:
+                if isinstance(device, qubes.device_protocol.UnknownDevice):
                     continue
-
-            self.pre_attachment_internal(
-                vm, device, assignment.options, expected_attachment=vm)
-
-            asyncio.ensure_future(vm.fire_event_async(
-                'device-attach:block',
-                device=device,
-                options=assignment.options,
-            ))
+                if not assignment.matches(device):
+                    print(
+                        "Unrecognized identity, skipping attachment of device "
+                        f"from the port {assignment}", file=sys.stderr)
+                    continue
+                # chose first assignment (the most specific) and ignore rest
+                if device not in to_attach:
+                    # make it unique
+                    to_attach[device] = assignment.clone(
+                        device=qubes.device_protocol.VirtualDevice(
+                            device.port, device.device_id))
+        for assignment in to_attach.values():
+            await self.attach_and_notify(vm, assignment)
 
     async def attach_and_notify(self, vm, assignment):
         # bypass DeviceCollection logic preventing double attach
-        # we expected that these devices are already attached to this vm
-        self.notify_auto_attached(vm, assignment)
+        device = assignment.device
+        if assignment.mode.value == "ask-to-attach":
+            if vm.name != confirm_device_attachment(device, {vm: assignment}):
+                return
+        self.on_device_pre_attached_block(
+            vm, 'device-pre-attach:block', device, assignment.options)
+        await vm.fire_event_async(
+            'device-attach:block', device=str(device), options=assignment.options)
 
     @qubes.ext.handler('domain-shutdown')
     async def on_domain_shutdown(self, vm, event, **_kwargs):

qubes/ext/utils.py

@@ -43,15 +43,15 @@ def device_list_change(
         compare_device_cache(vm, ext.devices_cache, current_devices))
 
     # send events about devices detached/attached outside by themselves
-    for dev_id, front_vm in detached.items():
-        dev = device_class(vm, dev_id)
+    for port_id, front_vm in detached.items():
+        dev = device_class(vm, port_id)
         asyncio.ensure_future(front_vm.fire_event_async(
             f'device-detach:{devclass}', port=dev.port))
-    for dev_id in removed:
-        device = device_class(vm, dev_id)
-        vm.fire_event(f'device-removed:{devclass}', device=device)
-    for dev_id in added:
-        device = device_class(vm, dev_id)
+    for port_id in removed:
+        device = device_class(vm, port_id)
+        vm.fire_event(f'device-removed:{devclass}', port=device.port)
+    for port_id in added:
+        device = device_class(vm, port_id)
         vm.fire_event(f'device-added:{devclass}', device=device)
     for dev_ident, front_vm in attached.items():
         dev = device_class(vm, dev_ident)
@@ -73,8 +73,12 @@ def device_list_change(
                 ):
                     frontends = to_attach.get(device.port_id, {})
                     # make it unique
-                    frontends[front_vm] = assignment.clone(
+                    ass = assignment.clone(
                         device=VirtualDevice(device.port, device.device_id))
+                    curr = frontends.get(front_vm, None)
+                    if curr is None or curr < ass:
+                        # chose the most specific assignment
+                        frontends[front_vm] = ass
                     to_attach[device.port_id] = frontends
 
     for port_id, frontends in to_attach.items():
@@ -137,7 +141,6 @@ def compare_device_cache(vm, devices_cache, current_devices):
 
 def confirm_device_attachment(device, frontends) -> str:
     guivm = 'dom0'  # TODO
-    # TODO: guivm rpc?
 
     try:
         proc = subprocess.Popen(

qubes/tests/devices_block.py

@@ -668,7 +668,7 @@ def test_060_on_qdb_change_added(self):
             '/qubes-block-devices/sda/size': b'1024000',
             '/qubes-block-devices/sda/mode': b'r',
         }, domain_xml=domain_xml_template.format(""))
-        exp_dev =  qubes.ext.block.BlockDevice(back_vm, 'sda')
+        exp_dev = qubes.ext.block.BlockDevice(back_vm, 'sda')
 
         self.ext.on_qdb_change(back_vm, None, None)
 
@@ -713,6 +713,7 @@ def test_061_on_qdb_change_auto_attached(self):
 
         # In the case of block devices it is the same,
         # but notify_auto_attached is synchronous
+        # TODO!
         self.ext.attach_and_notify = self.ext.notify_auto_attached
         with mock.patch('asyncio.ensure_future'):
             self.ext.on_qdb_change(back_vm, None, None)