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

Author: marmarek

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 @@ def respond(self, src, meth, dest, arg, *, untrusted_payload):
             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 @@ def __delitem__(self, key):
         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 @@ def on_domain_pre_deleted(self, event, vm):
                         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 @@
 import itertools
 import logging
 import functools
+import string
 import termios
 
 import asyncio
@@ -665,6 +666,13 @@ def _monitor_process(proc, error_message):
             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 @@ def backup_do(self):
         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,14 +717,17 @@ def backup_do(self):
             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:
             # Prepare the backup target (Qubes service call)
             # 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 @@ def backup_do(self):
 
         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)),
-                    send_task))
+                        self.target_vm.name)))
+            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 @@ def backup_do(self):
             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