q-dev: allow an unassignment of required device + typos

The unassignment of a required device does not break anything, and it's required to detach the required device. Detaching required device can break things but should be possible.

qubes/api/admin.py

@@ -1304,9 +1304,9 @@ async def vm_device_unassign(self, endpoint):
 
         # qrexec already verified that no strange characters are in self.arg
         backend_domain, ident = self.arg.split('+', 1)
-        # may raise KeyError; if device isn't found, it will be UnknownDevice
-        #  instance - but allow it, otherwise it will be impossible to detach
-        #  already removed device
+        # may raise KeyError; if a device isn't found, it will be UnknownDevice
+        #  instance - but allow it, otherwise it will be impossible to unassign
+        #  an already removed device
         dev = self.app.domains[backend_domain].devices[devclass][ident]
 
         self.fire_event_for_permission(device=dev, devclass=devclass)

qubes/devices.py

@@ -304,7 +304,7 @@ async def detach(self, device: Device):
                 "Can not detach a required device from a non halted qube. "
                 "You need to unassign device first.")
 
-        # use local object
+        # use the local object
         device = assignment.device
         await self._vm.fire_event_async(
             'device-pre-detach:' + self._bus, pre_event=True, device=device)
@@ -326,12 +326,6 @@ async def unassign(self, device_assignment: DeviceAssignment):
                 f'device {device_assignment.ident!s} of class {self._bus} not '
                 f'assigned to {self._vm!s}')
 
-        if not self._vm.is_halted() and assignment.required:
-            raise qubes.exc.QubesVMNotHaltedError(
-                self._vm,
-                "Can not remove an required assignment from "
-                "a non halted qube.")
-
         self._set.discard(assignment)
 
         device = device_assignment.device

qubes/tests/api_admin.py

@@ -1994,7 +1994,8 @@ def test_490_vm_device_unassign_from_running(self):
             value = self.call_mgmt_func(b'admin.vm.device.testclass.Unassign',
                 b'test-vm1', b'test-vm1+1234')
         self.assertIsNone(value)
-        mock_action.assert_called_once_with(self.vm, 'device-unassign:testclass',
+        mock_action.assert_called_once_with(
+            self.vm, 'device-unassign:testclass',
             device=self.vm.devices['testclass']['1234'])
         self.app.save.assert_called_once_with()
 
@@ -2010,12 +2011,15 @@ def test_491_vm_device_unassign_required_from_running(self):
         self.vm.add_handler('device-unassign:testclass', mock_action)
         with unittest.mock.patch.object(
                 qubes.vm.qubesvm.QubesVM, 'is_halted', lambda _: False):
-            with self.assertRaises(qubes.exc.QubesVMNotHaltedError):
-                self.call_mgmt_func(
+            value = self.call_mgmt_func(
                     b'admin.vm.device.testclass.Unassign',
                     b'test-vm1', b'test-vm1+1234')
-        self.assertFalse(mock_action.called)
-        self.assertFalse(self.app.save.called)
+        self.assertIsNone(value)
+        mock_action.assert_called_once_with(
+            self.vm, 'device-unassign:testclass',
+            device=self.vm.devices['testclass']['1234'])
+        self.app.save.assert_called_once_with()
+
     def test_492_vm_device_unassign_from_halted(self):
         assignment = qubes.device_protocol.DeviceAssignment(
             self.vm, '1234', attach_automatically=True, required=False,

qubes/tests/devices.py

@@ -312,11 +312,11 @@ def test_041_detach_required_from_halted(self):
                 self.collection.detach(self.assignment))
 
     def test_042_unassign_required(self):
-        self.loop.run_until_complete(self.collection.assign(self.assignment))
         self.emitter.running = True
-        with self.assertRaises(qubes.exc.QubesVMNotHaltedError):
-            self.loop.run_until_complete(
-                self.collection.unassign(self.assignment))
+        self.loop.run_until_complete(self.collection.assign(self.assignment))
+        self.loop.run_until_complete(self.collection.unassign(self.assignment))
+        self.assertEventFired(self.emitter, 'device-assign:testclass')
+        self.assertEventFired(self.emitter, 'device-unassign:testclass')
 
     def test_043_detach_assigned(self):
         self.assignment.required = False