Merge branch 'cdrom-boot'

* cdrom-boot:
  devices: fix error reporting
  api/admin: implement admin.vm.device....Set.persistent
  devices: implement DeviceCollection.update_persistent()
  devices: move DeviceInfo definition earlier
  api: do not fail events when listener is cancelled in the meantime

Makefile

@@ -55,14 +55,17 @@ ADMIN_API_METHODS_SIMPLE = \
 	admin.vm.device.pci.Available \
 	admin.vm.device.pci.Detach \
 	admin.vm.device.pci.List \
+	admin.vm.device.pci.Set.persistent \
 	admin.vm.device.block.Attach \
 	admin.vm.device.block.Available \
 	admin.vm.device.block.Detach \
 	admin.vm.device.block.List \
+	admin.vm.device.block.Set.persistent \
 	admin.vm.device.mic.Attach \
 	admin.vm.device.mic.Available \
 	admin.vm.device.mic.Detach \
 	admin.vm.device.mic.List \
+	admin.vm.device.mic.Set.persistent \
 	admin.vm.feature.CheckWithTemplate \
 	admin.vm.feature.Get \
 	admin.vm.feature.List \
@@ -188,6 +191,7 @@ endif
 		          admin.vm.device.testclass.Attach \
 				  admin.vm.device.testclass.Detach \
 				  admin.vm.device.testclass.List \
+				  admin.vm.device.testclass.Set.persistent \
 				  admin.vm.device.testclass.Available
 	# sanity check
 	for method in $(DESTDIR)/etc/qubes-rpc/policy/admin.*; do \

qubes/api/__init__.py

@@ -325,6 +325,8 @@ class QubesDaemonProtocol(asyncio.Protocol):
             self.transport.write(content.encode('utf-8'))
 
     def send_event(self, subject, event, **kwargs):
+        if self.transport is None:
+            return
         self.event_sent = True
         self.send_header(0x31)
 

qubes/api/admin.py

@@ -1093,6 +1093,36 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
         yield from self.dest.devices[devclass].detach(assignment)
         self.app.save()
 
+    # Attach/Detach action can both modify persistent state (with
+    # persistent=True) and volatile state of running VM (with persistent=False).
+    # For this reason, write=True + execute=True
+    @qubes.api.method('admin.vm.device.{endpoint}.Set.persistent',
+        endpoints=(ep.name
+            for ep in pkg_resources.iter_entry_points('qubes.devices')),
+        scope='local', write=True, execute=True)
+    @asyncio.coroutine
+    def vm_device_set_persistent(self, endpoint, untrusted_payload):
+        devclass = endpoint
+
+        assert untrusted_payload in (b'True', b'False')
+        persistent = untrusted_payload == b'True'
+        del untrusted_payload
+
+        # qrexec already verified that no strange characters are in self.arg
+        backend_domain, ident = self.arg.split('+', 1)
+        # device must be already attached
+        matching_devices = [dev for dev
+            in self.dest.devices[devclass].attached()
+            if dev.backend_domain.name == backend_domain and dev.ident == ident]
+        assert len(matching_devices) == 1
+        dev = matching_devices[0]
+
+        self.fire_event_for_permission(device=dev,
+            persistent=persistent)
+
+        self.dest.devices[devclass].update_persistent(dev, persistent)
+        self.app.save()
+
     @qubes.api.method('admin.vm.firewall.Get', no_payload=True,
             scope='local', read=True)
     @asyncio.coroutine

qubes/devices.py

@@ -68,6 +68,55 @@ class DeviceAlreadyAttached(qubes.exc.QubesException, KeyError):
     '''Trying to attach already attached device'''
     pass
 
+class DeviceInfo(object):
+    ''' Holds all information about a device '''
+    # pylint: disable=too-few-public-methods
+    def __init__(self, backend_domain, ident, description=None,
+                 frontend_domain=None):
+        #: domain providing this device
+        self.backend_domain = backend_domain
+        #: device identifier (unique for given domain and device type)
+        self.ident = ident
+        # allow redefining those as dynamic properties in subclasses
+        try:
+            #: human readable description/name of the device
+            self.description = description
+        except AttributeError:
+            pass
+        try:
+            #: (running) domain to which device is currently attached
+            self.frontend_domain = frontend_domain
+        except AttributeError:
+            pass
+
+        if hasattr(self, 'regex'):
+            # pylint: disable=no-member
+            dev_match = self.regex.match(ident)
+            if not dev_match:
+                raise ValueError('Invalid device identifier: {!r}'.format(
+                    ident))
+
+            for group in self.regex.groupindex:
+                setattr(self, group, dev_match.group(group))
+
+    def __hash__(self):
+        return hash((self.backend_domain, self.ident))
+
+    def __eq__(self, other):
+        return (
+            self.backend_domain == other.backend_domain and
+            self.ident == other.ident
+        )
+
+    def __lt__(self, other):
+        if isinstance(other, DeviceInfo):
+            return (self.backend_domain, self.ident) < \
+                   (other.backend_domain, other.ident)
+        return NotImplemented
+
+    def __str__(self):
+        return '{!s}:{!s}'.format(self.backend_domain, self.ident)
+
 
 class DeviceAssignment(object): # pylint: disable=too-few-public-methods
     ''' Maps a device to a frontend_domain. '''
@@ -221,6 +270,27 @@ class DeviceCollection(object):
         device_assignment.bus = self._bus
         self._set.add(device_assignment)
 
+    def update_persistent(self, device: DeviceInfo, persistent: bool):
+        '''Update `persistent` flag of already attached device.
+        '''
+
+        if self._vm.is_halted():
+            raise qubes.exc.QubesVMNotStartedError(self._vm,
+                'VM must be running to modify device persistence flag')
+        assignments = [a for a in self.assignments() if a.device == device]
+        if not assignments:
+            raise qubes.exc.QubesValueError('Device not assigned')
+        assert len(assignments) == 1
+        assignment = assignments[0]
+
+        # be careful to use already present assignment, not the provided one
+        # - to not change options as a side effect
+        if persistent and device not in self._set:
+            assignment.persistent = True
+            self._set.add(assignment)
+        elif not persistent and device in self._set:
+            self._set.discard(assignment)
+
     @asyncio.coroutine
     def detach(self, device_assignment: DeviceAssignment):
         '''Detach (remove) device from domain.
@@ -282,8 +352,8 @@ class DeviceCollection(object):
         try:
             devices = self._vm.fire_event('device-list-attached:' + self._bus,
                 persistent=persistent)
-        except Exception as e:  # pylint: disable=broad-except
-            self._vm.log.exception(e, 'Failed to list {} devices'.format(
+        except Exception:  # pylint: disable=broad-except
+            self._vm.log.exception('Failed to list {} devices'.format(
                 self._bus))
             if persistent is True:
                 # don't break app.save()
@@ -352,56 +422,6 @@ class DeviceManager(dict):
         return self[key]
 
 
-class DeviceInfo(object):
-    ''' Holds all information about a device '''
-    # pylint: disable=too-few-public-methods
-    def __init__(self, backend_domain, ident, description=None,
-                 frontend_domain=None):
-        #: domain providing this device
-        self.backend_domain = backend_domain
-        #: device identifier (unique for given domain and device type)
-        self.ident = ident
-        # allow redefining those as dynamic properties in subclasses
-        try:
-            #: human readable description/name of the device
-            self.description = description
-        except AttributeError:
-            pass
-        try:
-            #: (running) domain to which device is currently attached
-            self.frontend_domain = frontend_domain
-        except AttributeError:
-            pass
-
-        if hasattr(self, 'regex'):
-            # pylint: disable=no-member
-            dev_match = self.regex.match(ident)
-            if not dev_match:
-                raise ValueError('Invalid device identifier: {!r}'.format(
-                    ident))
-
-            for group in self.regex.groupindex:
-                setattr(self, group, dev_match.group(group))
-
-    def __hash__(self):
-        return hash((self.backend_domain, self.ident))
-
-    def __eq__(self, other):
-        return (
-            self.backend_domain == other.backend_domain and
-            self.ident == other.ident
-        )
-
-    def __lt__(self, other):
-        if isinstance(other, DeviceInfo):
-            return (self.backend_domain, self.ident) < \
-                   (other.backend_domain, other.ident)
-        return NotImplemented
-
-    def __str__(self):
-        return '{!s}:{!s}'.format(self.backend_domain, self.ident)
-
-
 class UnknownDevice(DeviceInfo):
     # pylint: disable=too-few-public-methods
     '''Unknown device - for example exposed by domain not running currently'''

qubes/tests/api_admin.py

@@ -2131,6 +2131,102 @@ class TC_00_VMs(AdminAPITestCase):
                 b'test-vm1')
         self.assertFalse(self.app.save.called)
 
+    def test_650_vm_device_set_persistent_true(self):
+        self.vm.add_handler('device-list:testclass',
+            self.device_list_testclass)
+        self.vm.add_handler('device-list-attached:testclass',
+            self.device_list_attached_testclass)
+        with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
+                'is_halted', lambda _: False):
+            value = self.call_mgmt_func(
+                b'admin.vm.device.testclass.Set.persistent',
+                b'test-vm1', b'test-vm1+1234', b'True')
+        self.assertIsNone(value)
+        dev = qubes.devices.DeviceInfo(self.vm, '1234')
+        self.assertIn(dev, self.vm.devices['testclass'].persistent())
+        self.app.save.assert_called_once_with()
+
+    def test_651_vm_device_set_persistent_false_unchanged(self):
+        self.vm.add_handler('device-list:testclass',
+            self.device_list_testclass)
+        self.vm.add_handler('device-list-attached:testclass',
+            self.device_list_attached_testclass)
+        with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
+                'is_halted', lambda _: False):
+            value = self.call_mgmt_func(
+                b'admin.vm.device.testclass.Set.persistent',
+                b'test-vm1', b'test-vm1+1234', b'False')
+        self.assertIsNone(value)
+        dev = qubes.devices.DeviceInfo(self.vm, '1234')
+        self.assertNotIn(dev, self.vm.devices['testclass'].persistent())
+        self.app.save.assert_called_once_with()
+
+    def test_652_vm_device_set_persistent_false(self):
+        self.vm.add_handler('device-list:testclass',
+            self.device_list_testclass)
+        assignment = qubes.devices.DeviceAssignment(self.vm, '1234', {},
+            True)
+        self.loop.run_until_complete(
+            self.vm.devices['testclass'].attach(assignment))
+        self.vm.add_handler('device-list-attached:testclass',
+            self.device_list_attached_testclass)
+        dev = qubes.devices.DeviceInfo(self.vm, '1234')
+        self.assertIn(dev, self.vm.devices['testclass'].persistent())
+        with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
+                'is_halted', lambda _: False):
+            value = self.call_mgmt_func(
+                b'admin.vm.device.testclass.Set.persistent',
+                b'test-vm1', b'test-vm1+1234', b'False')
+        self.assertIsNone(value)
+        self.assertNotIn(dev, self.vm.devices['testclass'].persistent())
+        self.assertIn(dev, self.vm.devices['testclass'].attached())
+        self.app.save.assert_called_once_with()
+
+    def test_653_vm_device_set_persistent_true_unchanged(self):
+        self.vm.add_handler('device-list:testclass',
+            self.device_list_testclass)
+        assignment = qubes.devices.DeviceAssignment(self.vm, '1234', {},
+            True)
+        self.loop.run_until_complete(
+            self.vm.devices['testclass'].attach(assignment))
+        self.vm.add_handler('device-list-attached:testclass',
+            self.device_list_attached_testclass)
+        with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
+                'is_halted', lambda _: False):
+            value = self.call_mgmt_func(
+                b'admin.vm.device.testclass.Set.persistent',
+                b'test-vm1', b'test-vm1+1234', b'True')
+        self.assertIsNone(value)
+        dev = qubes.devices.DeviceInfo(self.vm, '1234')
+        self.assertIn(dev, self.vm.devices['testclass'].persistent())
+        self.assertIn(dev, self.vm.devices['testclass'].attached())
+        self.app.save.assert_called_once_with()
+
+    def test_654_vm_device_set_persistent_not_attached(self):
+        self.vm.add_handler('device-list:testclass',
+            self.device_list_testclass)
+        with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
+                'is_halted', lambda _: False):
+            with self.assertRaises(AssertionError):
+                self.call_mgmt_func(
+                    b'admin.vm.device.testclass.Set.persistent',
+                    b'test-vm1', b'test-vm1+1234', b'True')
+        dev = qubes.devices.DeviceInfo(self.vm, '1234')
+        self.assertNotIn(dev, self.vm.devices['testclass'].persistent())
+        self.assertFalse(self.app.save.called)
+
+    def test_655_vm_device_set_persistent_invalid_value(self):
+        self.vm.add_handler('device-list:testclass',
+            self.device_list_testclass)
+        with unittest.mock.patch.object(qubes.vm.qubesvm.QubesVM,
+                'is_halted', lambda _: False):
+            with self.assertRaises(AssertionError):
+                self.call_mgmt_func(
+                    b'admin.vm.device.testclass.Set.persistent',
+                    b'test-vm1', b'test-vm1+1234', b'maybe')
+        dev = qubes.devices.DeviceInfo(self.vm, '1234')
+        self.assertNotIn(dev, self.vm.devices['testclass'].persistent())
+        self.assertFalse(self.app.save.called)
 
     def test_990_vm_unexpected_payload(self):
         methods_with_no_payload = [

qubes/tests/devices.py

@@ -71,6 +71,9 @@ class TestVM(qubes.tests.TestEmitter):
     def is_halted(self):
         return not self.running
 
+    def is_running(self):
+        return self.running
+
 
 
 class TC_00_DeviceCollection(qubes.tests.QubesTestCase):
@@ -130,8 +133,6 @@ class TC_00_DeviceCollection(qubes.tests.QubesTestCase):
         self.loop.run_until_complete(self.collection.attach(self.assignment))
         self.assertEventFired(self.emitter, 'device-list-attached:testclass')
         self.assertEqual({self.device}, set(self.collection.persistent()))
-        self.assertEqual({self.device},
-            set(self.collection.persistent()))
         self.assertEqual(set([]),
             set(self.collection.attached()))
 
@@ -153,6 +154,52 @@ class TC_00_DeviceCollection(qubes.tests.QubesTestCase):
         self.assertEqual({self.device}, set(self.collection))
         self.assertEventFired(self.emitter, 'device-list:testclass')
 
+    def test_020_update_persistent_to_false(self):
+        self.emitter.running = True
+        self.assertEqual(set([]), set(self.collection.persistent()))
+        self.loop.run_until_complete(self.collection.attach(self.assignment))
+        # device-attach event not implemented, so manipulate object manually
+        self.device.frontend_domain = self.emitter
+        self.assertEqual({self.device}, set(self.collection.persistent()))
+        self.assertEqual({self.device}, set(self.collection.attached()))
+        self.assertEqual({self.device}, set(self.collection.persistent()))
+        self.assertEqual({self.device}, set(self.collection.attached()))
+        self.collection.update_persistent(self.device, False)
+        self.assertEqual(set(), set(self.collection.persistent()))
+        self.assertEqual({self.device}, set(self.collection.attached()))
+
+    def test_021_update_persistent_to_true(self):
+        self.assignment.persistent = False
+        self.emitter.running = True
+        self.assertEqual(set([]), set(self.collection.persistent()))
+        self.loop.run_until_complete(self.collection.attach(self.assignment))
+        # device-attach event not implemented, so manipulate object manually
+        self.device.frontend_domain = self.emitter
+        self.assertEqual(set(), set(self.collection.persistent()))
+        self.assertEqual({self.device}, set(self.collection.attached()))
+        self.assertEqual(set(), set(self.collection.persistent()))
+        self.assertEqual({self.device}, set(self.collection.attached()))
+        self.collection.update_persistent(self.device, True)
+        self.assertEqual({self.device}, set(self.collection.persistent()))
+        self.assertEqual({self.device}, set(self.collection.attached()))
+
+    def test_022_update_persistent_reject_not_running(self):
+        self.assertEqual(set([]), set(self.collection.persistent()))
+        self.loop.run_until_complete(self.collection.attach(self.assignment))
+        self.assertEqual({self.device}, set(self.collection.persistent()))
+        self.assertEqual(set(), set(self.collection.attached()))
+        with self.assertRaises(qubes.exc.QubesVMNotStartedError):
+            self.collection.update_persistent(self.device, False)
+
+    def test_023_update_persistent_reject_not_attached(self):
+        self.assertEqual(set(), set(self.collection.persistent()))
+        self.assertEqual(set(), set(self.collection.attached()))
+        self.emitter.running = True
+        with self.assertRaises(qubes.exc.QubesValueError):
+            self.collection.update_persistent(self.device, True)
+        with self.assertRaises(qubes.exc.QubesValueError):
+            self.collection.update_persistent(self.device, False)
+
 
 class TC_01_DeviceManager(qubes.tests.QubesTestCase):
     def setUp(self):