Merge branch 'fixes-20170929'

* fixes-20170929:
  vm: do not start QubesDB watch instance multiple times
  vm: report storage.stop() errors to log
  vm: move comment
  storage: fix method name in LinuxModules volume
  Prevent removing domain that is referenced from anywhere
  vm: add vm.klass property
  Move QubesVM.{name,qid,uuid,label} to BaseVM
  vm: do not allow deleting template property from AppVM and DispVM
  vm/qubesvm: emit event on failed startup
  vm/qubesvm: remove duplicated qmemman_client.close()
  vm/dispvm: cleanup DispVM also on failed startup
  vm/dispvm: fix error message
  ext/block: properly list devtype=cdrom option
  block: fix handling non-existing devices
  block: improve handling device name and description

qubes/app.py

@@ -35,6 +35,7 @@ import time
 import traceback
 import uuid
 
+import itertools
 import lxml.etree
 
 import jinja2
@@ -857,6 +858,8 @@ class Qubes(qubes.PropertyHolder):
 
         return element
 
+    def __str__(self):
+        return type(self).__name__
 
     def save(self, lock=True):
         '''Save all data to qubes.xml
@@ -1186,14 +1189,17 @@ class Qubes(qubes.PropertyHolder):
     @qubes.events.handler('domain-pre-delete')
     def on_domain_pre_deleted(self, event, vm):
         # pylint: disable=unused-argument
-        if isinstance(vm, qubes.vm.templatevm.TemplateVM):
-            appvms = self.domains.get_vms_based_on(vm)
-            if appvms:
-                raise qubes.exc.QubesException(
-                    'Cannot remove template that has dependent AppVMs. '
-                    'Affected are: {}'.format(', '.join(
-                        appvm.name for appvm in sorted(appvms))))
-
+        for obj in itertools.chain(self.domains, (self,)):
+            for prop in obj.property_list():
+                try:
+                    if isinstance(prop, qubes.vm.VMProperty) and \
+                            getattr(obj, prop.__name__) == vm:
+                        self.log.error(
+                            'Cannot remove %s, used by %s.%s',
+                            vm, obj, prop.__name__)
+                        raise qubes.exc.QubesVMInUseError(vm)
+                except AttributeError:
+                    pass
 
     @qubes.events.handler('domain-delete')
     def on_domain_deleted(self, event, vm):

qubes/exc.py

@@ -42,6 +42,11 @@ class QubesVMError(QubesException):
         super(QubesVMError, self).__init__(msg)
         self.vm = vm
 
+class QubesVMInUseError(QubesVMError):
+    '''VM is in use, cannot remove.'''
+    def __init__(self, vm, msg=None):
+        super(QubesVMInUseError, self).__init__(vm,
+            msg or 'Domain is in use: {!r}'.format(vm.name))
 
 class QubesVMNotStartedError(QubesVMError):
     '''Domain is not started.

qubes/ext/block.py

@@ -58,6 +58,8 @@ class BlockDevice(qubes.devices.DeviceInfo):
                 string.ascii_letters + string.digits + '()+,-.:=_/ '}
             untrusted_desc = self.backend_domain.untrusted_qdb.read(
                 '/qubes-block-devices/{}/desc'.format(self.ident))
+            if not untrusted_desc:
+                return ''
             desc = ''.join((chr(c) if c in safe_set else '_')
                 for c in untrusted_desc)
             self._description = desc
@@ -136,14 +138,13 @@ class BlockDeviceExtension(qubes.ext.Extension):
     def on_device_list_block(self, vm, event):
         # pylint: disable=unused-argument,no-self-use
 
-        safe_set = string.ascii_letters + string.digits
         if not vm.is_running():
             return
         untrusted_qubes_devices = vm.untrusted_qdb.list('/qubes-block-devices/')
         untrusted_idents = set(untrusted_path.split('/', 3)[2]
             for untrusted_path in untrusted_qubes_devices)
         for untrusted_ident in untrusted_idents:
-            if not all(c in safe_set for c in untrusted_ident):
+            if not name_re.match(untrusted_ident):
                 msg = ("%s vm's device path name contains unsafe characters. "
                        "Skipping it.")
                 vm.log.warning(msg % vm.name)
@@ -161,7 +162,9 @@ class BlockDeviceExtension(qubes.ext.Extension):
         if not vm.is_running():
             return
         if not vm.app.vmm.offline_mode:
-            yield self.device_get(vm, ident)
+            device_info = self.device_get(vm, ident)
+            if device_info:
+                yield device_info
 
     @qubes.ext.handler('device-list-attached:block')
     def on_device_list_attached(self, vm, event, **kwargs):
@@ -202,6 +205,8 @@ class BlockDeviceExtension(qubes.ext.Extension):
             else:
                 options['read-only'] = 'no'
             options['frontend-dev'] = frontend_dev
+            if disk.get('device') != 'disk':
+                options['devtype'] = disk.get('device')
 
             if dev_path.startswith('/dev/'):
                 ident = dev_path[len('/dev/'):]

qubes/storage/kernels.py

@@ -83,8 +83,8 @@ class LinuxModules(Volume):
     def is_dirty(self):
         return False
 
-    def clone(self, source):
-        if isinstance(source, LinuxModules):
+    def import_volume(self, src_volume):
+        if isinstance(src_volume, LinuxModules):
             # do nothing
             return self
         raise StoragePoolException('clone of LinuxModules volume from '

qubes/tests/app.py

@@ -271,13 +271,13 @@ class TC_30_VMCollection(qubes.tests.QubesTestCase):
 #       pass
 
 
-class TC_90_Qubes(qubes.tests.QubesTestCase):
+class TC_89_QubesEmpty(qubes.tests.QubesTestCase):
     def tearDown(self):
         try:
             os.unlink('/tmp/qubestest.xml')
         except:
             pass
-        super(TC_90_Qubes, self).tearDown()
+        super().tearDown()
 
     @qubes.tests.skipUnlessDom0
     def test_000_init_empty(self):
@@ -288,25 +288,105 @@ class TC_90_Qubes(qubes.tests.QubesTestCase):
             pass
         qubes.Qubes.create_empty_store('/tmp/qubestest.xml').close()
 
-    def test_100_clockvm(self):
-        app = qubes.Qubes('/tmp/qubestest.xml', load=False, offline_mode=True)
-        app.load_initial_values()
 
-        template = app.add_new_vm('TemplateVM', name='test-template',
+class TC_90_Qubes(qubes.tests.QubesTestCase):
+    def tearDown(self):
+        try:
+            os.unlink('/tmp/qubestest.xml')
+        except:
+            pass
+        super().tearDown()
+
+    def setUp(self):
+        super(TC_90_Qubes, self).setUp()
+        self.app = qubes.Qubes('/tmp/qubestest.xml', load=False,
+            offline_mode=True)
+        self.addCleanup(self.cleanup_qubes)
+        self.app.load_initial_values()
+        self.template = self.app.add_new_vm('TemplateVM', name='test-template',
             label='green')
-        appvm = app.add_new_vm('AppVM', name='test-vm', template=template,
+
+    def cleanup_qubes(self):
+        self.app.close()
+        del self.app
+        try:
+            del self.template
+        except AttributeError:
+            pass
+
+    def test_100_clockvm(self):
+        appvm = self.app.add_new_vm('AppVM', name='test-vm', template=self.template,
             label='red')
-        self.assertIsNone(app.clockvm)
+        self.assertIsNone(self.app.clockvm)
         self.assertNotIn('service.clocksync', appvm.features)
-        self.assertNotIn('service.clocksync', template.features)
-        app.clockvm = appvm
+        self.assertNotIn('service.clocksync', self.template.features)
+        self.app.clockvm = appvm
         self.assertIn('service.clocksync', appvm.features)
         self.assertTrue(appvm.features['service.clocksync'])
-        app.clockvm = template
+        self.app.clockvm = self.template
         self.assertNotIn('service.clocksync', appvm.features)
-        self.assertIn('service.clocksync', template.features)
-        self.assertTrue(template.features['service.clocksync'])
-        app.close()
+        self.assertIn('service.clocksync', self.template.features)
+        self.assertTrue(self.template.features['service.clocksync'])
+
+    def test_200_remove_template(self):
+        appvm = self.app.add_new_vm('AppVM', name='test-vm',
+            template=self.template,
+            label='red')
+        with mock.patch.object(self.app, 'vmm'):
+            with self.assertRaises(qubes.exc.QubesException):
+                del self.app.domains[self.template]
+
+    def test_201_remove_netvm(self):
+        netvm = self.app.add_new_vm('AppVM', name='test-netvm',
+            template=self.template, provides_network=True,
+            label='red')
+        appvm = self.app.add_new_vm('AppVM', name='test-vm',
+            template=self.template,
+            label='red')
+        appvm.netvm = netvm
+        with mock.patch.object(self.app, 'vmm'):
+            with self.assertRaises(qubes.exc.QubesVMInUseError):
+                del self.app.domains[netvm]
+
+    def test_202_remove_default_netvm(self):
+        netvm = self.app.add_new_vm('AppVM', name='test-netvm',
+            template=self.template, provides_network=True,
+            label='red')
+        self.app.default_netvm = netvm
+        with mock.patch.object(self.app, 'vmm'):
+            with self.assertRaises(qubes.exc.QubesVMInUseError):
+                del self.app.domains[netvm]
+
+    def test_203_remove_default_dispvm(self):
+        appvm = self.app.add_new_vm('AppVM', name='test-appvm',
+            template=self.template,
+            label='red')
+        self.app.default_dispvm = appvm
+        with mock.patch.object(self.app, 'vmm'):
+            with self.assertRaises(qubes.exc.QubesVMInUseError):
+                del self.app.domains[appvm]
+
+    def test_204_remove_appvm_dispvm(self):
+        dispvm = self.app.add_new_vm('AppVM', name='test-appvm',
+            template=self.template,
+            label='red')
+        appvm = self.app.add_new_vm('AppVM', name='test-appvm2',
+            template=self.template, default_dispvm=dispvm,
+            label='red')
+        with mock.patch.object(self.app, 'vmm'):
+            with self.assertRaises(qubes.exc.QubesVMInUseError):
+                del self.app.domains[dispvm]
+
+    def test_205_remove_appvm_dispvm(self):
+        appvm = self.app.add_new_vm('AppVM', name='test-appvm',
+            template=self.template, template_for_dispvms=True,
+            label='red')
+        dispvm = self.app.add_new_vm('DispVM', name='test-dispvm',
+            template=appvm,
+            label='red')
+        with mock.patch.object(self.app, 'vmm'):
+            with self.assertRaises(qubes.exc.QubesVMInUseError):
+                del self.app.domains[appvm]
 
     @qubes.tests.skipUnlessGit
     def test_900_example_xml_in_doc(self):

qubes/tests/devices_block.py

@@ -328,6 +328,29 @@ class TC_00_Block(qubes.tests.QubesTestCase):
         self.assertEqual(options['frontend-dev'], 'xvdi')
         self.assertEqual(options['read-only'], 'no')
 
+    def test_033_list_attached_cdrom(self):
+        disk = '''
+        <disk type="block" device="cdrom">
+            <driver name="phy" />
+            <source dev="/dev/sr0" />
+            <target dev="xvdi" />
+            <readonly />
+            <backenddomain name="sys-usb" />
+        </disk>
+        '''
+        vm = TestVM({}, domain_xml=domain_xml_template.format(disk))
+        vm.app.domains['test-vm'] = vm
+        vm.app.domains['sys-usb'] = TestVM({}, name='sys-usb')
+        devices = sorted(list(self.ext.on_device_list_attached(vm, '')))
+        self.assertEqual(len(devices), 1)
+        dev = devices[0][0]
+        options = devices[0][1]
+        self.assertEqual(dev.backend_domain, vm.app.domains['sys-usb'])
+        self.assertEqual(dev.ident, 'sr0')
+        self.assertEqual(options['frontend-dev'], 'xvdi')
+        self.assertEqual(options['read-only'], 'yes')
+        self.assertEqual(options['devtype'], 'cdrom')
+
     def test_040_attach(self):
         back_vm = TestVM(name='sys-usb', qdb={
             '/qubes-block-devices/sda': b'',

qubes/tests/vm/adminvm.py

@@ -38,7 +38,7 @@ class TC_00_AdminVM(qubes.tests.QubesTestCase):
                     qubes.vm.adminvm.AdminVM, 'start_qdb_watch') as mock_qdb:
                 self.vm = qubes.vm.adminvm.AdminVM(self.app,
                     xml=None)
-                mock_qdb.assert_called_once_with('dom0')
+                mock_qdb.assert_called_once_with()
                 self.addCleanup(self.cleanup_adminvm)
         except:  # pylint: disable=bare-except
             if self.id().endswith('.test_000_init'):

qubes/tests/vm/appvm.py

@@ -130,6 +130,11 @@ class TC_90_AppVM(qubes.tests.vm.qubesvm.QubesVMTestsMixin,
             with self.assertRaises(qubes.exc.QubesVMNotHaltedError):
                 vm.template = self.template
 
+    def test_004_template_reset(self):
+        vm = self.get_vm()
+        with self.assertRaises(qubes.exc.QubesValueError):
+            vm.template = qubes.property.DEFAULT
+
     def test_500_property_migrate_template_for_dispvms(self):
         xml_template = '''
         <domain class="AppVM" id="domain-1">

qubes/tests/vm/dispvm.py

@@ -103,6 +103,24 @@ class TC_00_DispVM(qubes.tests.QubesTestCase):
             dispvm = self.loop.run_until_complete(
                 qubes.vm.dispvm.DispVM.from_appvm(self.appvm))
 
+    def test_002_template_change(self):
+        self.appvm.template_for_dispvms = True
+        orig_getitem = self.app.domains.__getitem__
+        with mock.patch.object(self.app, 'domains', wraps=self.app.domains) \
+                as mock_domains:
+            mock_domains.configure_mock(**{
+                'get_new_unused_dispid': mock.Mock(return_value=42),
+                '__getitem__.side_effect': orig_getitem
+            })
+            dispvm = self.app.add_new_vm(qubes.vm.dispvm.DispVM,
+                name='test-dispvm', template=self.appvm)
+
+            with self.assertRaises(qubes.exc.QubesValueError):
+                dispvm.template = self.appvm
+            with self.assertRaises(qubes.exc.QubesValueError):
+                dispvm.template = qubes.property.DEFAULT
+
+
     def test_010_create_direct(self):
         self.appvm.template_for_dispvms = True
         orig_getitem = self.app.domains.__getitem__

qubes/tests/vm/qubesvm.py

@@ -71,15 +71,15 @@ class TC_00_setters(qubes.tests.QubesTestCase):
 
     def test_000_setter_qid(self):
         self.assertEqual(
-            qubes.vm.qubesvm._setter_qid(self.vm, self.prop, 5), 5)
+            qubes.vm._setter_qid(self.vm, self.prop, 5), 5)
 
     def test_001_setter_qid_lt_0(self):
         with self.assertRaises(ValueError):
-            qubes.vm.qubesvm._setter_qid(self.vm, self.prop, -1)
+            qubes.vm._setter_qid(self.vm, self.prop, -1)
 
     def test_002_setter_qid_gt_max(self):
         with self.assertRaises(ValueError):
-            qubes.vm.qubesvm._setter_qid(self.vm,
+            qubes.vm._setter_qid(self.vm,
                 self.prop, qubes.config.max_qid + 5)
 
     @unittest.skip('test not implemented')

qubes/vm/__init__.py

@@ -27,6 +27,7 @@
 import asyncio
 import re
 import string
+import uuid
 
 import lxml.etree
 
@@ -64,6 +65,26 @@ def validate_name(holder, prop, value):
         raise qubes.exc.QubesValueError(
             'VM name cannot be \'none\' nor \'default\'')
 
+def setter_label(self, prop, value):
+    ''' Helper for setting the domain label '''
+    # pylint: disable=unused-argument
+    if isinstance(value, qubes.Label):
+        return value
+    if isinstance(value, str) and value.startswith('label-'):
+        return self.app.labels[int(value.split('-', 1)[1])]
+
+    return self.app.get_label(value)
+
+
+def _setter_qid(self, prop, value):
+    ''' Helper for setting the domain qid '''
+    # pylint: disable=unused-argument
+    value = int(value)
+    if not 0 <= value <= qubes.config.max_qid:
+        raise ValueError(
+            '{} value must be between 0 and qubes.config.max_qid'.format(
+                prop.__name__))
+    return value
 
 class Features(dict):
     '''Manager of the features.
@@ -262,15 +283,35 @@ class BaseVM(qubes.PropertyHolder):
     '''
     # pylint: disable=no-member
 
+    uuid = qubes.property('uuid', type=uuid.UUID, write_once=True,
+        clone=False,
+        doc='UUID from libvirt.')
+
+    name = qubes.property('name', type=str, write_once=True,
+        clone=False,
+        doc='User-specified name of the domain.')
+
+    qid = qubes.property('qid', type=int, write_once=True,
+        setter=_setter_qid,
+        clone=False,
+        doc='''Internal, persistent identificator of particular domain. Note
+            this is different from Xen domid.''')
+
+    label = qubes.property('label',
+        setter=setter_label,
+        doc='''Colourful label assigned to VM. This is where the colour of the
+            padlock is set.''')
+
     def __init__(self, app, xml, features=None, devices=None, tags=None,
             **kwargs):
         # pylint: disable=redefined-outer-name
 
+        self._qdb_watch_paths = set()
+        self._qdb_connection_watch = None
+
         # self.app must be set before super().__init__, because some property
         # setters need working .app attribute
         #: mother :py:class:`qubes.Qubes` object
-        self._qdb_watch_paths = set()
-        self._qdb_connection_watch = None
         self.app = app
 
         super(BaseVM, self).__init__(xml, **kwargs)
@@ -445,14 +486,20 @@ class BaseVM(qubes.PropertyHolder):
             self._qdb_connection_watch.close()
             self._qdb_connection_watch = None
 
-    def start_qdb_watch(self, name, loop=None):
+    def start_qdb_watch(self, loop=None):
         '''Start watching QubesDB
 
         Calling this method in appropriate time is responsibility of child
         class.
         '''
+        # cleanup old watch connection first, if any
+        if self._qdb_connection_watch is not None:
+            asyncio.get_event_loop().remove_reader(
+                self._qdb_connection_watch.watch_fd())
+            self._qdb_connection_watch.close()
+
         import qubesdb  # pylint: disable=import-error
-        self._qdb_connection_watch = qubesdb.QubesDB(name)
+        self._qdb_connection_watch = qubesdb.QubesDB(self.name)
         if loop is None:
             loop = asyncio.get_event_loop()
         loop.add_reader(self._qdb_connection_watch.watch_fd(),
@@ -460,6 +507,10 @@ class BaseVM(qubes.PropertyHolder):
         for path in self._qdb_watch_paths:
             self._qdb_connection_watch.watch(path)
 
+    @qubes.stateless_property
+    def klass(self):
+        '''Domain class name'''
+        return type(self).__name__
 
 class VMProperty(qubes.property):
     '''Property that is referring to a VM
@@ -531,14 +582,3 @@ class VMProperty(qubes.property):
             return untrusted_vmname
         validate_name(None, self, untrusted_vmname)
         return untrusted_vmname
-
-
-def setter_label(self, prop, value):
-    ''' Helper for setting the domain label '''
-    # pylint: disable=unused-argument
-    if isinstance(value, qubes.Label):
-        return value
-    if isinstance(value, str) and value.startswith('label-'):
-        return self.app.labels[int(value.split('-', 1)[1])]
-
-    return self.app.get_label(value)

qubes/vm/adminvm.py

@@ -24,10 +24,12 @@
 ''' This module contains the AdminVM implementation '''
 
 import libvirt
+
 import qubes
 import qubes.exc
 import qubes.vm
 
+
 class AdminVM(qubes.vm.BaseVM):
     '''Dom0'''
 
@@ -36,12 +38,6 @@ class AdminVM(qubes.vm.BaseVM):
     name = qubes.property('name',
         default='dom0', setter=qubes.property.forbidden)
 
-    label = qubes.property('label',
-        setter=qubes.vm.setter_label,
-        saver=(lambda self, prop, value: 'label-{}'.format(value.index)),
-        doc='''Colourful label assigned to VM. This is where the colour of the
-            padlock is set.''')
-
     qid = qubes.property('qid',
         default=0, setter=qubes.property.forbidden)
 
@@ -62,7 +58,7 @@ class AdminVM(qubes.vm.BaseVM):
         self._libvirt_domain = None
 
         if not self.app.vmm.offline_mode:
-            self.start_qdb_watch('dom0')
+            self.start_qdb_watch()
 
     def __str__(self):
         return self.name

qubes/vm/appvm.py

@@ -104,6 +104,12 @@ class AppVM(qubes.vm.qubesvm.QubesVM):
         '''  # pylint: disable=unused-argument
         assert self.template
 
+    @qubes.events.handler('property-pre-del:template')
+    def on_property_pre_del_template(self, event, name, oldvalue=None):
+        '''Forbid deleting template of running VM
+        '''  # pylint: disable=unused-argument,no-self-use
+        raise qubes.exc.QubesValueError('Cannot unset template')
+
     @qubes.events.handler('property-pre-set:template')
     def on_property_pre_set_template(self, event, name, newvalue,
             oldvalue=None):

qubes/vm/dispvm.py

@@ -121,8 +121,9 @@ class DispVM(qubes.vm.qubesvm.QubesVM):
         '''  # pylint: disable=unused-argument
         assert self.template
 
-    @qubes.events.handler('property-pre-set:template')
-    def on_property_pre_set_template(self, event, name, newvalue,
+    @qubes.events.handler('property-pre-set:template',
+        'property-pre-del:template')
+    def on_property_pre_set_template(self, event, name, newvalue=None,
             oldvalue=None):
         ''' Disposable VM cannot have template changed '''
         # pylint: disable=unused-argument
@@ -137,7 +138,7 @@ class DispVM(qubes.vm.qubesvm.QubesVM):
         '''
         with (yield from self.startup_lock):
             yield from self.storage.stop()
-            if self.auto_cleanup:
+            if self.auto_cleanup and self in self.app.domains:
                 yield from self.remove_from_disk()
                 del self.app.domains[self]
                 self.app.save()
@@ -164,7 +165,7 @@ class DispVM(qubes.vm.qubesvm.QubesVM):
         if not appvm.template_for_dispvms:
             raise qubes.exc.QubesException(
                 'Refusing to create DispVM out of this AppVM, because '
-                'template_for_appvms=False')
+                'template_for_dispvms=False')
         app = appvm.app
         dispvm = app.add_new_vm(
             cls,
@@ -197,10 +198,19 @@ class DispVM(qubes.vm.qubesvm.QubesVM):
     def start(self, **kwargs):
         # pylint: disable=arguments-differ
 
-        # sanity check, if template_for_dispvm got changed in the meantime
-        if not self.template.template_for_dispvms:
-            raise qubes.exc.QubesException(
-                'template for DispVM ({}) needs to have '
-                'template_for_dispvms=True'.format(self.template.name))
+        try:
+            # sanity check, if template_for_dispvm got changed in the meantime
+            if not self.template.template_for_dispvms:
+                raise qubes.exc.QubesException(
+                    'template for DispVM ({}) needs to have '
+                    'template_for_dispvms=True'.format(self.template.name))
 
-        yield from super(DispVM, self).start(**kwargs)
+            yield from super(DispVM, self).start(**kwargs)
+        except:
+            # cleanup also on failed startup; there is potential race with
+            # self.on_domain_shutdown_coro, so check if wasn't already removed
+            if self.auto_cleanup and self in self.app.domains:
+                yield from self.remove_from_disk()
+                del self.app.domains[self]
+                self.app.save()
+            raise

qubes/vm/qubesvm.py

@@ -59,17 +59,6 @@ MEM_OVERHEAD_BASE = (3 + 1) * 1024 * 1024
 MEM_OVERHEAD_PER_VCPU = 3 * 1024 * 1024 / 2
 
 
-def _setter_qid(self, prop, value):
-    ''' Helper for setting the domain qid '''
-    # pylint: disable=unused-argument
-    value = int(value)
-    if not 0 <= value <= qubes.config.max_qid:
-        raise ValueError(
-            '{} value must be between 0 and qubes.config.max_qid'.format(
-                prop.__name__))
-    return value
-
-
 def _setter_kernel(self, prop, value):
     ''' Helper for setting the domain kernel and running sanity checks on it.
     '''  # pylint: disable=unused-argument
@@ -369,30 +358,6 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
     # properties loaded from XML
     #
 
-    label = qubes.property('label',
-        setter=qubes.vm.setter_label,
-        saver=(lambda self, prop, value: 'label-{}'.format(value.index)),
-        doc='''Colourful label assigned to VM. This is where the colour of the
-            padlock is set.''')
-
-#   provides_network = qubes.property('provides_network',
-#       type=bool, setter=qubes.property.bool,
-#       doc='`True` if it is NetVM or ProxyVM, false otherwise.')
-
-    qid = qubes.property('qid', type=int, write_once=True,
-        setter=_setter_qid,
-        clone=False,
-        doc='''Internal, persistent identificator of particular domain. Note
-            this is different from Xen domid.''')
-
-    name = qubes.property('name', type=str, write_once=True,
-        clone=False,
-        doc='User-specified name of the domain.')
-
-    uuid = qubes.property('uuid', type=uuid.UUID, write_once=True,
-        clone=False,
-        doc='UUID from libvirt.')
-
     virt_mode = qubes.property('virt_mode',
         type=str, setter=_setter_virt_mode,
         default='hvm',
@@ -747,7 +712,7 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
             self.storage = qubes.storage.Storage(self)
 
         if not self.app.vmm.offline_mode and self.is_running():
-            self.start_qdb_watch(self.name)
+            self.start_qdb_watch()
 
     @qubes.events.handler('property-set:label')
     def on_property_set_label(self, event, name, newvalue, oldvalue=None):
@@ -839,17 +804,24 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
                 pre_event=True,
                 start_guid=start_guid, mem_required=mem_required)
 
-            yield from self.storage.verify()
-
-            if self.netvm is not None:
-                # pylint: disable = no-member
-                if self.netvm.qid != 0:
-                    if not self.netvm.is_running():
-                        yield from self.netvm.start(start_guid=start_guid,
-                            notify_function=notify_function)
-
-            qmemman_client = yield from asyncio.get_event_loop().\
-                run_in_executor(None, self.request_memory, mem_required)
+            try:
+                yield from self.storage.verify()
+
+                if self.netvm is not None:
+                    # pylint: disable = no-member
+                    if self.netvm.qid != 0:
+                        if not self.netvm.is_running():
+                            yield from self.netvm.start(start_guid=start_guid,
+                                notify_function=notify_function)
+
+                qmemman_client = yield from asyncio.get_event_loop().\
+                    run_in_executor(None, self.request_memory, mem_required)
+
+            except Exception as exc:
+                # let anyone receiving domain-pre-start know that startup failed
+                yield from self.fire_event_async('domain-start-failed',
+                    reason=str(exc))
+                raise
 
             try:
                 yield from self.storage.start()
@@ -857,6 +829,13 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
 
                 self.libvirt_domain.createWithFlags(
                     libvirt.VIR_DOMAIN_START_PAUSED)
+
+            except Exception as exc:
+                # let anyone receiving domain-pre-start know that startup failed
+                yield from self.fire_event_async('domain-start-failed',
+                    reason=str(exc))
+                raise
+
             finally:
                 if qmemman_client:
                     qmemman_client.close()
@@ -868,34 +847,27 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
                 self.log.info('Setting Qubes DB info for the VM')
                 yield from self.start_qubesdb()
                 self.create_qdb_entries()
+                self.start_qdb_watch()
 
                 self.log.warning('Activating the {} VM'.format(self.name))
                 self.libvirt_domain.resume()
 
-                # close() is not really needed, because the descriptor is
-                # close-on-exec anyway, the reason to postpone close() is that
-                # possibly xl is not done constructing the domain after its main
-                # process exits so we close() when we know the domain is up the
-                # successful unpause is some indicator of it
-                if qmemman_client:
-                    qmemman_client.close()
-                    qmemman_client = None
-
                 yield from self.start_qrexec_daemon()
 
                 yield from self.fire_event_async('domain-start',
                     start_guid=start_guid)
 
-            except:  # pylint: disable=bare-except
+            except Exception as exc:  # pylint: disable=bare-except
                 if self.is_running() or self.is_paused():
                     # This avoids losing the exception if an exception is
                     # raised in self.force_shutdown(), because the vm is not
                     # running or paused
                     yield from self.kill()  # pylint: disable=not-an-iterable
+
+                # let anyone receiving domain-pre-start know that startup failed
+                yield from self.fire_event_async('domain-start-failed',
+                    reason=str(exc))
                 raise
-            finally:
-                if qmemman_client:
-                    qmemman_client.close()
 
         return self
 
@@ -905,7 +877,11 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
         Do not allow domain to be started again until this finishes.
         '''
         with (yield from self.startup_lock):
-            yield from self.storage.stop()
+            try:
+                yield from self.storage.stop()
+            except qubes.storage.StoragePoolException:
+                self.log.exception('Failed to stop storage for domain %s',
+                    self.name)
 
     @qubes.events.handler('domain-shutdown')
     def on_domain_shutdown(self, _event, **_kwargs):
@@ -1767,8 +1743,6 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
 
         self.fire_event('domain-qdb-create')
 
-        self.start_qdb_watch(self.name)
-
     # TODO async; update this in constructor
     def _update_libvirt_domain(self):
         '''Re-initialise :py:attr:`libvirt_domain`.'''