Merge branch '20171107-tests-backup-api-misc'

* 20171107-tests-backup-api-misc:
  test: make race condition on xterm close less likely
  tests/backupcompatibility: fix handling 'internal' property
  backup: fix handling target write error (like no disk space)
  tests/backupcompatibility: drop R1 format tests
  backup: use offline_mode for backup collection
  qubespolicy: fix handling '$adminvm' target with ask action
  app: drop reference to libvirt object after undefining it
  vm: always log startup fail
  api: do not log handled errors sent to a client
  tests/backups: convert to new restore handling - using qubesadmin module
  app: clarify error message on failed domain remove (used somewhere)
  Fix qubes-core.service ordering

linux/systemd/qubes-core.service

@@ -1,6 +1,6 @@
 [Unit]
 Description=Qubes Dom0 startup setup
-After=qubes-db-dom0.service libvirtd.service xenconsoled.service
+After=qubes-db-dom0.service libvirtd.service xenconsoled.service qubesd.service qubes-qmemman.service
 # Cover legacy init.d script
 
 [Service]

qubes/api/__init__.py

@@ -285,9 +285,6 @@ class QubesDaemonProtocol(asyncio.Protocol):
             if self.debug:
                 self.app.log.exception(msg,
                     err, src, meth, dest, arg, len(untrusted_payload))
-            else:
-                self.app.log.info(msg,
-                    err, src, meth, dest, arg, len(untrusted_payload))
             if self.transport is not None:
                 self.send_exception(err)
                 self.transport.write_eof()

qubes/app.py

@@ -484,6 +484,8 @@ class VMCollection(object):
         self.app.fire_event('domain-pre-delete', pre_event=True, vm=vm)
         try:
             vm.libvirt_domain.undefine()
+            # pylint: disable=protected-access
+            vm._libvirt_domain = None
         except libvirt.libvirtError as e:
             if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
                 # already undefined
@@ -1196,7 +1198,9 @@ class Qubes(qubes.PropertyHolder):
                         self.log.error(
                             'Cannot remove %s, used by %s.%s',
                             vm, obj, prop.__name__)
-                        raise qubes.exc.QubesVMInUseError(vm)
+                        raise qubes.exc.QubesVMInUseError(vm,
+                            'Domain is in use: {!r}; details in system log'
+                                .format(vm.name))
                 except AttributeError:
                     pass
 

qubes/backup.py

@@ -23,6 +23,7 @@ from __future__ import unicode_literals
 import itertools
 import logging
 import functools
+import string
 import termios
 
 import asyncio
@@ -665,6 +666,13 @@ class Backup(object):
             raise
 
         if proc.returncode:
+            if proc.stderr is not None:
+                proc_stderr = (yield from proc.stderr.read())
+                proc_stderr = proc_stderr.decode('ascii', errors='ignore')
+                proc_stderr = ''.join(
+                    c for c in proc_stderr if c in string.printable and
+                                              c not in '\r\n%{}')
+                error_message += ': ' + proc_stderr
             raise qubes.exc.QubesException(error_message)
 
     @staticmethod
@@ -694,7 +702,7 @@ class Backup(object):
         self.tmpdir = tempfile.mkdtemp()
         shutil.copy(qubes_xml, os.path.join(self.tmpdir, 'qubes.xml'))
         qubes_xml = os.path.join(self.tmpdir, 'qubes.xml')
-        backup_app = qubes.Qubes(qubes_xml)
+        backup_app = qubes.Qubes(qubes_xml, offline_mode=True)
         backup_app.events_enabled = False
 
         files_to_backup = self._files_to_backup
@@ -709,6 +717,7 @@ class Backup(object):
             backup_app.domains[qid].features['backup-path'] = vm_info.subdir
             backup_app.domains[qid].features['backup-size'] = vm_info.size
         backup_app.save()
+        del backup_app
 
         vmproc = None
         if self.target_vm is not None:
@@ -716,7 +725,9 @@ class Backup(object):
             # If APPVM, STDOUT is a PIPE
             read_fd, write_fd = os.pipe()
             vmproc = yield from self.target_vm.run_service('qubes.Backup',
-                stdin=read_fd, stderr=subprocess.PIPE)
+                stdin=read_fd,
+                stderr=subprocess.PIPE,
+                stdout=subprocess.DEVNULL)
             os.close(read_fd)
             os.write(write_fd, (self.target_dir.
                 replace("\r", "").replace("\n", "") + "\n").encode())
@@ -754,11 +765,12 @@ class Backup(object):
 
         vmproc_task = None
         if vmproc is not None:
-            vmproc_task = asyncio.ensure_future(self._cancel_on_error(
+            vmproc_task = asyncio.ensure_future(
                 self._monitor_process(vmproc,
                     'Writing backup to VM {} failed'.format(
-                        self.target_vm.name)),
+                        self.target_vm.name)))
-                    send_task))
+            asyncio.ensure_future(self._cancel_on_error(
+                vmproc_task, send_task))
 
         for file_name in header_files:
             yield from to_send.put(file_name)
@@ -782,10 +794,12 @@ class Backup(object):
             except:
                 yield from to_send.put(QUEUE_ERROR)
                 # in fact we may be handling CancelledError, induced by
-                # exception in send_task (and propagated by
+                # exception in send_task or vmproc_task (and propagated by
                 # self._cancel_on_error call above); in such a case this
                 # yield from will raise exception, covering CancelledError -
                 # this is intended behaviour
+                if vmproc_task:
+                    yield from vmproc_task
                 yield from send_task
                 raise
 

qubes/tests/integ/backup.py

@@ -27,6 +27,9 @@ import os
 import shutil
 
 import sys
+
+import asyncio
+
 import qubes
 import qubes.backup
 import qubes.exc
@@ -38,6 +41,14 @@ import qubes.vm.appvm
 import qubes.vm.templatevm
 import qubes.vm.qubesvm
 
+try:
+    import qubesadmin.backup.restore
+    import qubesadmin.exc
+    restore_available = True
+except ImportError:
+    restore_available = False
+
+
 # noinspection PyAttributeOutsideInit
 class BackupTestsMixin(object):
     class BackupErrorHandler(logging.Handler):
@@ -65,15 +76,15 @@ class BackupTestsMixin(object):
 
         self.error_handler = self.BackupErrorHandler(self.error_detected,
             level=logging.WARNING)
-        backup_log = logging.getLogger('qubes.backup')
+        backup_log = logging.getLogger('qubesadmin.backup')
         backup_log.addHandler(self.error_handler)
 
     def tearDown(self):
-        super(BackupTestsMixin, self).tearDown()
         shutil.rmtree(self.backupdir)
 
         backup_log = logging.getLogger('qubes.backup')
         backup_log.removeHandler(self.error_handler)
+        super(BackupTestsMixin, self).tearDown()
 
     def fill_image(self, path, size=None, sparse=False):
         block_size = 4096
@@ -112,7 +123,6 @@ class BackupTestsMixin(object):
         self.log.debug("Creating %s" % vmname)
         testvm1 = self.app.add_new_vm(qubes.vm.appvm.AppVM,
             name=vmname, template=template, label='red')
-        testvm1.uses_default_netvm = False
         testvm1.netvm = testnet
         self.loop.run_until_complete(
             testvm1.create_on_disk(pool=pool))
@@ -175,10 +185,18 @@ class BackupTestsMixin(object):
             else:
                 raise
 
+    def remove_vms(self, vms):
+        vms = list(vms)
+        for vm in vms:
+            vm.netvm = None
+            vm.default_dispvm = None
+        super(BackupTestsMixin, self).remove_vms(vms)
+
     def restore_backup(self, source=None, appvm=None, options=None,
                        expect_errors=None, manipulate_restore_info=None,
                        passphrase='qubes'):
-        self.skipTest('Test not converted to Qubes 4.0')
+        if not restore_available:
+            self.skipTest('qubesadmin module not available')
 
         if source is None:
             backupfile = os.path.join(self.backupdir,
@@ -186,19 +204,30 @@ class BackupTestsMixin(object):
         else:
             backupfile = source
 
-        with self.assertNotRaises(qubes.exc.QubesException):
+        client_app = qubesadmin.Qubes()
-            restore_op = qubes.backup.BackupRestore(
+        if appvm:
-                self.app, backupfile, appvm, passphrase)
+            appvm = self.loop.run_until_complete(
+                self.loop.run_in_executor(None,
+                    client_app.domains.__getitem__, appvm.name))
+        with self.assertNotRaises(qubesadmin.exc.QubesException):
+            restore_op = self.loop.run_until_complete(
+                self.loop.run_in_executor(None,
+                    qubesadmin.backup.restore.BackupRestore,
+                    client_app, backupfile, appvm, passphrase))
             if options:
                 for key, value in options.items():
                     setattr(restore_op.options, key, value)
-            restore_info = restore_op.get_restore_info()
+            restore_info = self.loop.run_until_complete(
+                self.loop.run_in_executor(None,
+                    restore_op.get_restore_info))
         if callable(manipulate_restore_info):
             restore_info = manipulate_restore_info(restore_info)
         self.log.debug(restore_op.get_restore_summary(restore_info))
 
-        with self.assertNotRaises(qubes.exc.QubesException):
+        with self.assertNotRaises(qubesadmin.exc.QubesException):
-            restore_op.restore_do(restore_info)
+            self.loop.run_until_complete(
+                self.loop.run_in_executor(None,
+                    restore_op.restore_do, restore_info))
 
         errors = []
         if expect_errors is None:
@@ -230,7 +259,7 @@ class BackupTestsMixin(object):
             for name, volume in vm.volumes.items():
                 if not volume.rw or not volume.save_on_stop:
                     continue
-                vol_path = vm.storage.get_pool(volume).export(volume)
+                vol_path = volume.export()
                 hasher = hashlib.sha1()
                 with open(vol_path, 'rb') as afile:
                     for buf in iter(lambda: afile.read(4096000), b''):
@@ -238,7 +267,37 @@ class BackupTestsMixin(object):
                 hashes[vm.name][name] = hasher.hexdigest()
         return hashes
 
-    def assertCorrectlyRestored(self, orig_vms, orig_hashes):
+    def get_vms_info(self, vms):
+        ''' Get VM metadata, for comparing VM later without holding actual
+        reference to the old object.'''
+
+        vms_info = {}
+        for vm in vms:
+            vm_info = {
+                'properties': {},
+                'default': {},
+                'devices': {},
+            }
+            for prop in ('name', 'kernel',
+                    'memory', 'maxmem', 'kernelopts',
+                    'services', 'vcpus', 'features'
+                    'include_in_backups', 'default_user', 'qrexec_timeout',
+                    'autostart', 'pci_strictreset', 'debug',
+                    'internal', 'netvm', 'template', 'label'):
+                if not hasattr(vm, prop):
+                    continue
+                vm_info['properties'][prop] = str(getattr(vm, prop))
+                vm_info['default'][prop] = vm.property_is_default(prop)
+            for dev_class in vm.devices.keys():
+                vm_info['devices'][dev_class] = {}
+                for dev_ass in vm.devices[dev_class].assignments():
+                    vm_info['devices'][dev_class][str(dev_ass.device)] = \
+                        dev_ass.options
+            vms_info[vm.name] = vm_info
+
+        return vms_info
+
+    def assertCorrectlyRestored(self, vms_info, orig_hashes):
         ''' Verify if restored VMs are identical to those before backup.
 
         :param orig_vms: collection of original QubesVM objects
@@ -246,114 +305,108 @@ class BackupTestsMixin(object):
             before backup
         :return:
         '''
-        for vm in orig_vms:
+        for vm_name in vms_info:
-            self.assertIn(vm.name, self.app.domains)
+            vm_info = vms_info[vm_name]
-            restored_vm = self.app.domains[vm.name]
+            self.assertIn(vm_name, self.app.domains)
-            for prop in ('name', 'kernel',
+            restored_vm = self.app.domains[vm_name]
-                    'memory', 'maxmem', 'kernelopts',
+            for prop in vm_info['properties']:
-                    'services', 'vcpus', 'features'
-                    'include_in_backups', 'default_user', 'qrexec_timeout',
-                    'autostart', 'pci_strictreset', 'debug',
-                    'internal'):
-                if not hasattr(vm, prop):
-                    continue
                 self.assertEqual(
-                    getattr(vm, prop), getattr(restored_vm, prop),
+                    vm_info['properties'][prop],
+                    str(getattr(restored_vm, prop)),
                     "VM {} - property {} not properly restored".format(
-                        vm.name, prop))
+                        vm_name, prop))
-            for prop in ('netvm', 'template', 'label'):
+                self.assertEqual(
-                if not hasattr(vm, prop):
+                    vm_info['default'][prop],
-                    continue
+                    restored_vm.property_is_default(prop),
-                orig_value = getattr(vm, prop)
+                    "VM {} - property {} differs in being default".format(
-                restored_value = getattr(restored_vm, prop)
+                        vm_name, prop))
-                if orig_value and restored_value:
+            for dev_class in vm_info['devices']:
-                    self.assertEqual(orig_value.name, restored_value.name,
+                for dev in vm_info['devices'][dev_class]:
-                        "VM {} - property {} not properly restored".format(
+                    found = False
-                            vm.name, prop))
+                    for restored_dev_ass in restored_vm.devices[
-                else:
+                            dev_class].assignments():
-                    self.assertEqual(orig_value, restored_value,
+                        if str(restored_dev_ass.device) == dev:
-                        "VM {} - property {} not properly restored".format(
+                            found = True
-                            vm.name, prop))
+                            self.assertEqual(vm_info['devices'][dev_class][dev],
-            for dev_class in vm.devices.keys():
+                                restored_dev_ass.options,
-                for dev in vm.devices[dev_class]:
+                                'VM {} - {} device {} options mismatch'.format(
-                    self.assertIn(dev, restored_vm.devices[dev_class],
+                                    vm_name, dev_class, str(dev)))
-                        "VM {} - {} device not restored".format(
+                    self.assertTrue(found,
-                            vm.name, dev_class))
+                        'VM {} - {} device {} not restored'.format(
+                            vm_name, dev_class, dev))
 
             if orig_hashes:
                 hashes = self.vm_checksum([restored_vm])[restored_vm.name]
-                self.assertEqual(orig_hashes[vm.name], hashes,
+                self.assertEqual(orig_hashes[vm_name], hashes,
                     "VM {} - disk images are not properly restored".format(
-                        vm.name))
+                        vm_name))
 
 
 class TC_00_Backup(BackupTestsMixin, qubes.tests.SystemTestCase):
     def test_000_basic_backup(self):
         vms = self.create_backup_vms()
-        orig_hashes = self.vm_checksum(vms)
+        try:
-        self.make_backup(vms)
+            orig_hashes = self.vm_checksum(vms)
-        self.remove_vms(reversed(vms))
+            vms_info = self.get_vms_info(vms)
+            self.make_backup(vms)
+            self.remove_vms(reversed(vms))
+        finally:
+            del vms
         self.restore_backup()
-        self.assertCorrectlyRestored(vms, orig_hashes)
+        self.assertCorrectlyRestored(vms_info, orig_hashes)
-        self.remove_vms(reversed(vms))
 
     def test_001_compressed_backup(self):
         vms = self.create_backup_vms()
-        orig_hashes = self.vm_checksum(vms)
+        try:
-        self.make_backup(vms, compressed=True)
+            orig_hashes = self.vm_checksum(vms)
-        self.remove_vms(reversed(vms))
+            vms_info = self.get_vms_info(vms)
+            self.make_backup(vms, compressed=True)
+            self.remove_vms(reversed(vms))
+        finally:
+            del vms
         self.restore_backup()
-        self.assertCorrectlyRestored(vms, orig_hashes)
+        self.assertCorrectlyRestored(vms_info, orig_hashes)
-
-    def test_002_encrypted_backup(self):
-        vms = self.create_backup_vms()
-        orig_hashes = self.vm_checksum(vms)
-        self.make_backup(vms, encrypted=True)
-        self.remove_vms(reversed(vms))
-        self.restore_backup()
-        self.assertCorrectlyRestored(vms, orig_hashes)
-
-    def test_003_compressed_encrypted_backup(self):
-        vms = self.create_backup_vms()
-        orig_hashes = self.vm_checksum(vms)
-        self.make_backup(vms, compressed=True, encrypted=True)
-        self.remove_vms(reversed(vms))
-        self.restore_backup()
-        self.assertCorrectlyRestored(vms, orig_hashes)
 
     def test_004_sparse_multipart(self):
         vms = []
+        try:
+            vmname = self.make_vm_name('testhvm2')
+            self.log.debug("Creating %s" % vmname)
 
-        vmname = self.make_vm_name('testhvm2')
+            self.hvmtemplate = self.app.add_new_vm(
-        self.log.debug("Creating %s" % vmname)
+                qubes.vm.templatevm.TemplateVM, name=vmname, virt_mode='hvm', label='red')
+            self.loop.run_until_complete(self.hvmtemplate.create_on_disk())
+            self.fill_image(
+                os.path.join(self.hvmtemplate.dir_path, '00file'),
+                195 * 1024 * 1024 - 4096 * 3)
+            self.fill_image(self.hvmtemplate.storage.export('private'),
+                            195 * 1024 * 1024 - 4096 * 3)
+            self.fill_image(self.hvmtemplate.storage.export('root'), 1024 * 1024 * 1024,
+                            sparse=True)
+            vms.append(self.hvmtemplate)
+            self.app.save()
+            orig_hashes = self.vm_checksum(vms)
+            vms_info = self.get_vms_info(vms)
 
-        hvmtemplate = self.app.add_new_vm(
+            self.make_backup(vms)
-            qubes.vm.templatevm.TemplateVM, name=vmname, virt_mode='hvm', label='red')
+            self.remove_vms(reversed(vms))
-        hvmtemplate.create_on_disk()
+            self.restore_backup()
-        self.fill_image(
+            self.assertCorrectlyRestored(vms_info, orig_hashes)
-            os.path.join(hvmtemplate.dir_path, '00file'),
+            # TODO check vm.backup_timestamp
-            195 * 1024 * 1024 - 4096 * 3)
+        finally:
-        self.fill_image(hvmtemplate.storage.export('private'),
+            del vms
-                        195 * 1024 * 1024 - 4096 * 3)
-        self.fill_image(hvmtemplate.storage.export('root'), 1024 * 1024 * 1024,
-                        sparse=True)
-        vms.append(hvmtemplate)
-        self.app.save()
-        orig_hashes = self.vm_checksum(vms)
-
-        self.make_backup(vms)
-        self.remove_vms(reversed(vms))
-        self.restore_backup()
-        self.assertCorrectlyRestored(vms, orig_hashes)
-        # TODO check vm.backup_timestamp
 
     def test_005_compressed_custom(self):
         vms = self.create_backup_vms()
-        orig_hashes = self.vm_checksum(vms)
+        try:
-        self.make_backup(vms, compression_filter="bzip2")
+            orig_hashes = self.vm_checksum(vms)
-        self.remove_vms(reversed(vms))
+            vms_info = self.get_vms_info(vms)
-        self.restore_backup()
+            self.make_backup(vms, compression_filter="bzip2")
-        self.assertCorrectlyRestored(vms, orig_hashes)
+            self.remove_vms(reversed(vms))
+            self.restore_backup()
+            self.assertCorrectlyRestored(vms_info, orig_hashes)
+        finally:
+            del vms
 
     def test_010_selective_restore(self):
         # create backup with internal dependencies (template, netvm etc)
@@ -368,27 +421,39 @@ class TC_00_Backup(BackupTestsMixin, qubes.tests.SystemTestCase):
                 restore_info.pop(name)
             return restore_info
         vms = self.create_backup_vms()
-        orig_hashes = self.vm_checksum(vms)
+        try:
-        self.make_backup(vms, compression_filter="bzip2")
+            orig_hashes = self.vm_checksum(vms)
-        self.remove_vms(reversed(vms))
+            vms_info = self.get_vms_info(vms)
-        self.restore_backup(manipulate_restore_info=exclude_some)
+            self.make_backup(vms, compression_filter="bzip2")
-        for vm in vms:
+            self.remove_vms(reversed(vms))
-            if vm.name == self.make_vm_name('test1'):
+            self.restore_backup(manipulate_restore_info=exclude_some)
-                # netvm was set to 'test-inst-test-net' - excluded
+            for vm_name in vms_info:
-                vm.netvm = qubes.property.DEFAULT
+                if vm_name == self.make_vm_name('test1'):
-            elif vm.name == self.make_vm_name('custom'):
+                    # netvm was set to 'test-inst-test-net' - excluded
-                # template was set to 'test-inst-template' - excluded
+                    vms_info[vm_name]['properties']['netvm'] = \
-                vm.template = self.app.default_template
+                        str(self.app.default_netvm)
-        vms = [vm for vm in vms if vm.name not in exclude]
+                    vms_info[vm_name]['default']['netvm'] = True
-        self.assertCorrectlyRestored(vms, orig_hashes)
+                elif vm_name == self.make_vm_name('custom'):
+                    # template was set to 'test-inst-template' - excluded
+                    vms_info[vm_name]['properties']['template'] = \
+                        str(self.app.default_template)
+            for excluded in exclude:
+                vms_info.pop(excluded, None)
+            self.assertCorrectlyRestored(vms_info, orig_hashes)
+        finally:
+            del vms
 
     def test_020_encrypted_backup_non_ascii(self):
         vms = self.create_backup_vms()
-        orig_hashes = self.vm_checksum(vms)
+        try:
-        self.make_backup(vms, encrypted=True, passphrase=u'zażółć gęślą jaźń')
+            orig_hashes = self.vm_checksum(vms)
-        self.remove_vms(reversed(vms))
+            vms_info = self.get_vms_info(vms)
-        self.restore_backup(passphrase=u'zażółć gęślą jaźń')
+            self.make_backup(vms, passphrase=u'zażółć gęślą jaźń')
-        self.assertCorrectlyRestored(vms, orig_hashes)
+            self.remove_vms(reversed(vms))
+            self.restore_backup(passphrase=u'zażółć gęślą jaźń')
+            self.assertCorrectlyRestored(vms_info, orig_hashes)
+        finally:
+            del vms
 
     def test_100_backup_dom0_no_restore(self):
         # do not write it into dom0 home itself...
@@ -403,19 +468,28 @@ class TC_00_Backup(BackupTestsMixin, qubes.tests.SystemTestCase):
         :return:
         """
         vms = self.create_backup_vms()
-        orig_hashes = self.vm_checksum(vms)
+        try:
-        self.make_backup(vms)
+            orig_hashes = self.vm_checksum(vms)
-        self.remove_vms(reversed(vms))
+            vms_info = self.get_vms_info(vms)
-        test_dir = vms[0].dir_path
+            self.make_backup(vms)
-        os.mkdir(test_dir)
+            test_dir = vms[0].dir_path
-        with open(os.path.join(test_dir, 'some-file.txt'), 'w') as f:
+            self.remove_vms(reversed(vms))
-            f.write('test file\n')
+            os.mkdir(test_dir)
-        self.restore_backup(
+            with open(os.path.join(test_dir, 'some-file.txt'), 'w') as f:
-            expect_errors=[
+                f.write('test file\n')
-                '*** Directory {} already exists! It has been moved'.format(
+            self.restore_backup(expect_errors=[
-                    test_dir)
+                'Error restoring VM test-inst-test-net, skipping: Got empty '
+                'response from qubesd. See journalctl in dom0 for details.',
+                'Error setting test-inst-test1.netvm to test-inst-test-net: '
+                '\'"No such domain: \\\'test-inst-test-net\\\'"\'',
             ])
-        self.assertCorrectlyRestored(vms, orig_hashes)
+            del vms_info['test-inst-test-net']
+            vms_info['test-inst-test1']['properties']['netvm'] = \
+                str(self.app.default_netvm)
+            vms_info['test-inst-test1']['default']['netvm'] = True
+            self.assertCorrectlyRestored(vms_info, orig_hashes)
+        finally:
+            del vms
 
     def test_210_auto_rename(self):
         """
@@ -423,16 +497,22 @@ class TC_00_Backup(BackupTestsMixin, qubes.tests.SystemTestCase):
         :return:
         """
         vms = self.create_backup_vms()
-        self.make_backup(vms)
+        vms_info = self.get_vms_info(vms)
-        self.restore_backup(options={
+        try:
-            'rename_conflicting': True
+            self.make_backup(vms)
-        })
+            self.restore_backup(options={
-        for vm in vms:
+                'rename_conflicting': True
-            with self.assertNotRaises(
+            })
-                    (qubes.exc.QubesVMNotFoundError, KeyError)):
+            for vm_name in vms_info:
-                restored_vm = self.app.domains[vm.name + '1']
+                with self.assertNotRaises(
-            if vm.netvm and not vm.property_is_default('netvm'):
+                        (qubes.exc.QubesVMNotFoundError, KeyError)):
-                self.assertEqual(restored_vm.netvm.name, vm.netvm.name + '1')
+                    restored_vm = self.app.domains[vm_name + '1']
+                if vms_info[vm_name]['properties']['netvm'] and \
+                        not vms_info[vm_name]['default']['netvm']:
+                    self.assertEqual(restored_vm.netvm.name,
+                        vms_info[vm_name]['properties']['netvm'] + '1')
+        finally:
+            del vms
 
     def _find_pool(self, volume_group, thin_pool):
         ''' Returns the pool matching the specified ``volume_group`` &
@@ -456,12 +536,15 @@ class TC_00_Backup(BackupTestsMixin, qubes.tests.SystemTestCase):
                 **qubes.tests.storage_lvm.POOL_CONF)
             self.created_pool = True
         vms = self.create_backup_vms(pool=self.pool)
-        orig_hashes = self.vm_checksum(vms)
+        try:
-        self.make_backup(vms)
+            orig_hashes = self.vm_checksum(vms)
-        self.remove_vms(reversed(vms))
+            vms_info = self.get_vms_info(vms)
-        self.restore_backup()
+            self.make_backup(vms)
-        self.assertCorrectlyRestored(vms, orig_hashes)
+            self.remove_vms(reversed(vms))
-        self.remove_vms(reversed(vms))
+            self.restore_backup()
+            self.assertCorrectlyRestored(vms_info, orig_hashes)
+        finally:
+            del vms
 
     @qubes.tests.storage_lvm.skipUnlessLvmPoolExists
     def test_301_restore_to_lvm(self):
@@ -473,17 +556,20 @@ class TC_00_Backup(BackupTestsMixin, qubes.tests.SystemTestCase):
                 **qubes.tests.storage_lvm.POOL_CONF)
             self.created_pool = True
         vms = self.create_backup_vms()
-        orig_hashes = self.vm_checksum(vms)
+        try:
-        self.make_backup(vms)
+            orig_hashes = self.vm_checksum(vms)
-        self.remove_vms(reversed(vms))
+            vms_info = self.get_vms_info(vms)
-        self.restore_backup(options={'override_pool': self.pool.name})
+            self.make_backup(vms)
-        self.assertCorrectlyRestored(vms, orig_hashes)
+            self.remove_vms(reversed(vms))
-        for vm in vms:
+            self.restore_backup(options={'override_pool': self.pool.name})
-            vm = self.app.domains[vm.name]
+            self.assertCorrectlyRestored(vms_info, orig_hashes)
-            for volume in vm.volumes.values():
+            for vm_name in vms_info:
-                if volume.save_on_stop:
+                vm = self.app.domains[vm_name]
-                    self.assertEqual(volume.pool, self.pool.name)
+                for volume in vm.volumes.values():
-        self.remove_vms(reversed(vms))
+                    if volume.save_on_stop:
+                        self.assertEqual(volume.pool, self.pool.name)
+        finally:
+            del vms
 
 
 class TC_10_BackupVMMixin(BackupTestsMixin):
@@ -499,28 +585,40 @@ class TC_10_BackupVMMixin(BackupTestsMixin):
 
     def test_100_send_to_vm_file_with_spaces(self):
         vms = self.create_backup_vms()
-        self.loop.run_until_complete(self.backupvm.start())
+        orig_hashes = self.vm_checksum(vms)
-        self.loop.run_until_complete(self.backupvm.run_for_stdio(
+        vms_info = self.get_vms_info(vms)
-            "mkdir '/var/tmp/backup directory'"))
+        try:
-        self.make_backup(vms, target_vm=self.backupvm,
+            self.loop.run_until_complete(self.backupvm.start())
-            compressed=True, encrypted=True,
+            self.loop.run_until_complete(self.backupvm.run_for_stdio(
-            target='/var/tmp/backup directory')
+                "mkdir '/var/tmp/backup directory'"))
-        self.remove_vms(reversed(vms))
+            self.make_backup(vms, target_vm=self.backupvm,
-        (backup_path, _) = self.loop.run_until_complete(
+                compressed=True,
-            self.backupvm.run_for_stdio("ls /var/tmp/backup*/qubes-backup*"))
+                target='/var/tmp/backup directory')
-        backup_path = backup_path.decode().strip()
+            self.remove_vms(reversed(vms))
-        self.restore_backup(source=backup_path,
+            (backup_path, _) = self.loop.run_until_complete(
-                            appvm=self.backupvm)
+                self.backupvm.run_for_stdio("ls /var/tmp/backup*/qubes-backup*"))
+            backup_path = backup_path.decode().strip()
+            self.restore_backup(source=backup_path,
+                                appvm=self.backupvm)
+            self.assertCorrectlyRestored(vms_info, orig_hashes)
+        finally:
+            del vms
 
     def test_110_send_to_vm_command(self):
         vms = self.create_backup_vms()
-        self.loop.run_until_complete(self.backupvm.start())
+        orig_hashes = self.vm_checksum(vms)
-        self.make_backup(vms, target_vm=self.backupvm,
+        vms_info = self.get_vms_info(vms)
-            compressed=True, encrypted=True,
+        try:
-            target='dd of=/var/tmp/backup-test')
+            self.loop.run_until_complete(self.backupvm.start())
-        self.remove_vms(reversed(vms))
+            self.make_backup(vms, target_vm=self.backupvm,
-        self.restore_backup(source='dd if=/var/tmp/backup-test',
+                compressed=True,
-                            appvm=self.backupvm)
+                target='dd of=/var/tmp/backup-test')
+            self.remove_vms(reversed(vms))
+            self.restore_backup(source='dd if=/var/tmp/backup-test',
+                                appvm=self.backupvm)
+            self.assertCorrectlyRestored(vms_info, orig_hashes)
+        finally:
+            del vms
 
     def test_110_send_to_vm_no_space(self):
         """
@@ -529,21 +627,24 @@ class TC_10_BackupVMMixin(BackupTestsMixin):
         :return:
         """
         vms = self.create_backup_vms()
-        self.loop.run_until_complete(self.backupvm.start())
+        try:
-        self.loop.run_until_complete(self.backupvm.run_for_stdio(
+            self.loop.run_until_complete(self.backupvm.start())
-            # Debian 7 has too old losetup to handle loop-control device
+            self.loop.run_until_complete(self.backupvm.run_for_stdio(
-            "mknod /dev/loop0 b 7 0;"
+                # Debian 7 has too old losetup to handle loop-control device
-            "truncate -s 50M /home/user/backup.img && "
+                "mknod /dev/loop0 b 7 0;"
-            "mkfs.ext4 -F /home/user/backup.img && "
+                "truncate -s 50M /home/user/backup.img && "
-            "mkdir /home/user/backup && "
+                "mkfs.ext4 -F /home/user/backup.img && "
-            "mount /home/user/backup.img /home/user/backup -o loop &&"
+                "mkdir /home/user/backup && "
-            "chmod 777 /home/user/backup",
+                "mount /home/user/backup.img /home/user/backup -o loop &&"
-            user="root"))
+                "chmod 777 /home/user/backup",
-        with self.assertRaises(qubes.exc.QubesException):
+                user="root"))
-            self.make_backup(vms, target_vm=self.backupvm,
+            with self.assertRaises(qubes.exc.QubesException):
-                compressed=False, encrypted=True,
+                self.make_backup(vms, target_vm=self.backupvm,
-                target='/home/user/backup',
+                    compressed=False,
-                expect_failure=True)
+                    target='/home/user/backup',
+                    expect_failure=True)
+        finally:
+            del vms
 
 
 def load_tests(loader, tests, pattern):
@@ -551,7 +652,7 @@ def load_tests(loader, tests, pattern):
         tests.addTests(loader.loadTestsFromTestCase(
             type(
                 'TC_10_BackupVM_' + template,
-                (TC_10_BackupVMMixin, qubes.tests.QubesTestCase),
+                (TC_10_BackupVMMixin, qubes.tests.SystemTestCase),
                 {'template': template})))
 
     return tests

qubes/tests/integ/dispvm.py

@@ -147,8 +147,11 @@ class TC_20_DispVMMixin(object):
             self.assertTrue(dispvm.is_running())
             try:
                 window_title = 'user@%s' % (dispvm.name,)
+                # close xterm on Return, but after short delay, to allow
+                # xdotool to send also keyup event
                 p.stdin.write("xterm -e "
-                    "\"sh -c 'echo \\\"\033]0;{}\007\\\";read x;'\"\n".
+                    "\"sh -c 'echo \\\"\033]0;{}\007\\\";read x;"
+                              "sleep 0.1;'\"\n".
                     format(window_title).encode())
                 self.loop.run_until_complete(p.stdin.drain())
                 self.wait_for_window(window_title)

qubes/vm/qubesvm.py

@@ -897,6 +897,7 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
                     libvirt.VIR_DOMAIN_START_PAUSED)
 
             except Exception as exc:
+                self.log.error('Start failed: %s', str(exc))
                 # let anyone receiving domain-pre-start know that startup failed
                 yield from self.fire_event_async('domain-start-failed',
                     reason=str(exc))
@@ -928,6 +929,7 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
                     start_guid=start_guid)
 
             except Exception as exc:  # pylint: disable=bare-except
+                self.log.error('Start failed: %s', str(exc))
                 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

qubespolicy/__init__.py

@@ -617,6 +617,11 @@ class Policy(object):
                 if verify_target_value(system_info, dispvm):
                     targets.add(dispvm)
 
+        # expand other keywords
+        if '$adminvm' in targets:
+            targets.remove('$adminvm')
+            targets.add('dom0')
+
         return targets
 
     def evaluate(self, system_info, source, target):

qubespolicy/tests/__init__.py

@@ -685,6 +685,7 @@ class TC_20_Policy(qubes.tests.QubesTestCase):
             f.write('$tag:tag1 $type:AppVM allow\n')
             f.write('test-no-dvm $dispvm allow\n')
             f.write('test-standalone $dispvm allow\n')
+            f.write('test-standalone $adminvm allow\n')
         policy = qubespolicy.Policy('test.service', tmp_policy_dir)
         self.assertCountEqual(policy.collect_targets_for_ask(system_info,
             'test-vm1'), ['test-vm1', 'test-vm2', 'test-vm3',
@@ -698,7 +699,7 @@ class TC_20_Policy(qubes.tests.QubesTestCase):
         self.assertCountEqual(policy.collect_targets_for_ask(system_info,
             'test-standalone'), ['test-vm1', 'test-vm2', 'test-vm3',
             'default-dvm', 'test-no-dvm', 'test-invalid-dvm',
-            '$dispvm:default-dvm'])
+            '$dispvm:default-dvm', 'dom0'])
         self.assertCountEqual(policy.collect_targets_for_ask(system_info,
             'test-no-dvm'), [])