Merge branch 'devel-4'

* devel-4:
  tools/qvm-start-gui: multiple fixes
  vm: raise CalledProcessError instead of QubesVMError on failed service call
  events: improve handling qubesd restart

qubesadmin/events/__init__.py

@@ -118,7 +118,8 @@ class EventsDispatcher(object):
         while True:
             try:
                 yield from self._listen_for_events(vm)
-            except ConnectionRefusedError:
+            except (ConnectionRefusedError, ConnectionResetError,
+                    FileNotFoundError):
                 pass
             if not reconnect:
                 break

qubesadmin/tests/tools/qvm_start_gui.py

@@ -142,18 +142,30 @@ class TC_00_qvm_start_gui(qubesadmin.tests.QubesTestCase):
 
     @unittest.mock.patch('asyncio.create_subprocess_exec')
     def test_020_start_gui_for_vm(self, proc_mock):
+        loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(loop)
+        self.addCleanup(loop.close)
+
         self.app.expected_calls[
             ('dom0', 'admin.vm.List', None, None)] = \
             b'0\x00test-vm class=AppVM state=Running\n'
+        self.app.expected_calls[
+            ('test-vm', 'admin.vm.List', None, None)] = \
+            b'0\x00test-vm class=AppVM state=Running\n'
         self.app.expected_calls[
             ('test-vm', 'admin.vm.property.Get', 'xid', None)] = \
                 b'0\x00default=False type=int 3000'
         self.app.expected_calls[
             ('test-vm', 'admin.vm.property.Get', 'hvm', None)] = \
                 b'0\x00default=False type=bool False'
+        self.app.expected_calls[
+            ('test-vm', 'admin.vm.feature.CheckWithTemplate',
+            'no-monitor-layout', None)] = \
+            b'2\x00QubesFeatureNotFoundError\x00\x00Feature not set\x00'
         with unittest.mock.patch.object(self.launcher,
                 'common_guid_args', lambda vm: []):
-            self.launcher.start_gui_for_vm(self.app.domains['test-vm'])
+            loop.run_until_complete(self.launcher.start_gui_for_vm(
+                self.app.domains['test-vm']))
             # common arguments dropped for simplicity
             proc_mock.assert_called_once_with('-d', '3000')
 
@@ -161,9 +173,16 @@ class TC_00_qvm_start_gui(qubesadmin.tests.QubesTestCase):
 
     @unittest.mock.patch('asyncio.create_subprocess_exec')
     def test_021_start_gui_for_vm_hvm(self, proc_mock):
+        loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(loop)
+        self.addCleanup(loop.close)
+
         self.app.expected_calls[
             ('dom0', 'admin.vm.List', None, None)] = \
             b'0\x00test-vm class=AppVM state=Running\n'
+        self.app.expected_calls[
+            ('test-vm', 'admin.vm.List', None, None)] = \
+            b'0\x00test-vm class=AppVM state=Running\n'
         self.app.expected_calls[
             ('test-vm', 'admin.vm.property.Get', 'xid', None)] = \
                 b'0\x00default=False type=int 3000'
@@ -180,18 +199,30 @@ class TC_00_qvm_start_gui(qubesadmin.tests.QubesTestCase):
             ('test-vm', 'admin.vm.feature.CheckWithTemplate', 'rpc-clipboard',
             None)] = \
                 b'0\x00True'
+        self.app.expected_calls[
+            ('test-vm', 'admin.vm.feature.CheckWithTemplate',
+            'no-monitor-layout', None)] = \
+            b'2\x00QubesFeatureNotFoundError\x00\x00Feature not set\x00'
         with unittest.mock.patch.object(self.launcher,
                 'common_guid_args', lambda vm: []):
-            self.launcher.start_gui_for_vm(self.app.domains['test-vm'])
+            loop.run_until_complete(self.launcher.start_gui_for_vm(
+                self.app.domains['test-vm']))
             # common arguments dropped for simplicity
             proc_mock.assert_called_once_with('-d', '3000', '-n', '-Q')
 
         self.assertAllCalled()
 
     def test_022_start_gui_for_vm_hvm_stubdom(self):
+        loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(loop)
+        self.addCleanup(loop.close)
+
         self.app.expected_calls[
             ('dom0', 'admin.vm.List', None, None)] = \
             b'0\x00test-vm class=AppVM state=Running\n'
+        self.app.expected_calls[
+            ('test-vm', 'admin.vm.List', None, None)] = \
+            b'0\x00test-vm class=AppVM state=Running\n'
         self.app.expected_calls[
             ('test-vm', 'admin.vm.property.Get', 'xid', None)] = \
                 b'0\x00default=False type=int 3000'
@@ -208,6 +239,10 @@ class TC_00_qvm_start_gui(qubesadmin.tests.QubesTestCase):
             ('test-vm', 'admin.vm.feature.CheckWithTemplate', 'rpc-clipboard',
             None)] = \
                 b'0\x00True'
+        self.app.expected_calls[
+            ('test-vm', 'admin.vm.feature.CheckWithTemplate',
+            'no-monitor-layout', None)] = \
+            b'2\x00QubesFeatureNotFoundError\x00\x00Feature not set\x00'
         pidfile = tempfile.NamedTemporaryFile()
         pidfile.write(b'1234\n')
         pidfile.flush()
@@ -222,7 +257,8 @@ class TC_00_qvm_start_gui(qubesadmin.tests.QubesTestCase):
             mock_proc = patch_proc.start()
             patch_args.start()
             patch_pidfile.start()
-            self.launcher.start_gui_for_vm(self.app.domains['test-vm'])
+            loop.run_until_complete(self.launcher.start_gui_for_vm(
+                self.app.domains['test-vm']))
             # common arguments dropped for simplicity
             mock_proc.assert_called_once_with(
                 '-d', '3000', '-n', '-Q', '-K', '1234')
@@ -252,8 +288,8 @@ class TC_00_qvm_start_gui(qubesadmin.tests.QubesTestCase):
         self.assertAllCalled()
 
     @asyncio.coroutine
-    def mock_coroutine(self, mock, *args):
+    def mock_coroutine(self, mock, *args, **kwargs):
-        mock(*args)
+        mock(*args, **kwargs)
 
     def test_040_start_gui(self):
         loop = asyncio.new_event_loop()
@@ -287,8 +323,8 @@ class TC_00_qvm_start_gui(qubesadmin.tests.QubesTestCase):
         mock_start_vm = unittest.mock.Mock()
         mock_start_stubdomain = unittest.mock.Mock()
         patch_start_vm = unittest.mock.patch.object(
-            self.launcher, 'start_gui_for_vm', lambda vm_:
+            self.launcher, 'start_gui_for_vm', functools.partial(
-            self.mock_coroutine(mock_start_vm, vm_))
+                self.mock_coroutine, mock_start_vm))
         patch_start_stubdomain = unittest.mock.patch.object(
             self.launcher, 'start_gui_for_stubdomain', lambda vm_:
             self.mock_coroutine(mock_start_stubdomain, vm_))
@@ -296,7 +332,7 @@ class TC_00_qvm_start_gui(qubesadmin.tests.QubesTestCase):
             patch_start_vm.start()
             patch_start_stubdomain.start()
             loop.run_until_complete(self.launcher.start_gui(vm))
-            mock_start_vm.assert_called_once_with(vm)
+            mock_start_vm.assert_called_once_with(vm, monitor_layout=None)
             mock_start_stubdomain.assert_called_once_with(vm)
         finally:
             unittest.mock.patch.stopall()

qubesadmin/tools/qvm_start_gui.py

@@ -174,10 +174,15 @@ class GUILauncher(object):
         '''Helper function to construct a pidfile path'''
         return '/var/run/qubes/guid-running.{}'.format(xid)
 
-    def start_gui_for_vm(self, vm):
+    @asyncio.coroutine
+    def start_gui_for_vm(self, vm, monitor_layout=None):
         '''Start GUI daemon (qubes-guid) connected directly to a VM
 
         This function is a coroutine.
+
+        :param vm: VM for which start GUI daemon
+        :param monitor_layout: monitor layout to send; if None, fetch it from
+        local X server.
         '''
         guid_cmd = self.common_guid_args(vm)
         guid_cmd.extend(['-d', str(vm.xid)])
@@ -195,13 +200,19 @@ class GUILauncher(object):
                     stubdom_guid_pid = pidfile.read().strip()
                 guid_cmd += ['-K', stubdom_guid_pid]
 
-        return asyncio.create_subprocess_exec(*guid_cmd)
+        vm.log.info('Starting GUI')
+
+        yield from asyncio.create_subprocess_exec(*guid_cmd)
+
+        yield from self.send_monitor_layout(vm, layout=monitor_layout,
+            startup=True)
 
     def start_gui_for_stubdomain(self, vm):
         '''Start GUI daemon (qubes-guid) connected to a stubdomain
 
         This function is a coroutine.
         '''
+        vm.log.info('Starting GUI (stubdomain)')
         guid_cmd = self.common_guid_args(vm)
         guid_cmd.extend(['-d', str(vm.stubdom_xid), '-t', str(vm.xid)])
 
@@ -220,17 +231,13 @@ class GUILauncher(object):
         if not vm.features.check_with_template('gui', True):
             return
 
-        vm.log.info('Starting GUI')
         if vm.hvm:
             if force_stubdom or not os.path.exists(self.guid_pidfile(vm.xid)):
                 if not os.path.exists(self.guid_pidfile(vm.stubdom_xid)):
                     yield from self.start_gui_for_stubdomain(vm)
 
         if not os.path.exists(self.guid_pidfile(vm.xid)):
-            yield from self.start_gui_for_vm(vm)
+            yield from self.start_gui_for_vm(vm, monitor_layout=monitor_layout)
-
-        yield from self.send_monitor_layout(vm, layout=monitor_layout,
-            startup=True)
 
     @asyncio.coroutine
     def send_monitor_layout(self, vm, layout=None, startup=False):
@@ -267,9 +274,12 @@ class GUILauncher(object):
             except FileNotFoundError:
                 pass
 
+        try:
             yield from asyncio.get_event_loop().run_in_executor(None,
                 vm.run_service_for_stdio, 'qubes.SetMonitorLayout',
                     ''.join(layout).encode())
+        except subprocess.CalledProcessError as e:
+            vm.log.warning('Failed to send monitor layout: %s', e.stderr)
 
     def send_monitor_layout_all(self):
         '''Send monitor layout to all (running) VMs'''
@@ -302,12 +312,21 @@ class GUILauncher(object):
         daemon for domains started before this tool. '''
 
         monitor_layout = get_monitor_layout()
+        self.app.domains.clear_cache()
         for vm in self.app.domains:
             if isinstance(vm, qubesadmin.vm.AdminVM):
                 continue
-            if vm.is_running():
+            if not vm.features.check_with_template('gui', True):
+                continue
+            power_state = vm.get_power_state()
+            if power_state == 'Running':
                 asyncio.ensure_future(self.start_gui(vm,
                     monitor_layout=monitor_layout))
+            elif power_state == 'Transient':
+                # it is still starting, we'll get 'domain-start' event when
+                # fully started
+                if vm.hvm:
+                    asyncio.ensure_future(self.start_gui_for_stubdomain(vm))
 
     def register_events(self, events):
         '''Register domain startup events in app.events dispatcher'''

qubesadmin/vm/__init__.py

@@ -21,6 +21,9 @@
 '''Qubes VM objects.'''
 
 import logging
+
+import subprocess
+
 import qubesadmin.base
 import qubesadmin.exc
 import qubesadmin.storage
@@ -277,10 +280,10 @@ class QubesVM(qubesadmin.base.PropertyHolder):
         stdouterr = p.communicate(input=input)
 
         if p.returncode:
-            raise qubesadmin.exc.QubesVMError(
+            exc = subprocess.CalledProcessError(p.returncode, service)
-                'VM {}: service {!r} failed with retcode {!r}; '
+            # Python < 3.5 didn't have those
-                'stdout={!r} stderr={!r}'.format(self,
+            exc.output, exc.stderr = stdouterr
-                    service, p.returncode, *stdouterr))
+            raise exc
 
         return stdouterr
 
@@ -297,8 +300,13 @@ class QubesVM(qubesadmin.base.PropertyHolder):
         '''Run a shell command inside the domain using qubes.VMShell qrexec.
 
         '''  # pylint: disable=redefined-builtin
+        try:
             return self.run_service_for_stdio('qubes.VMShell',
                 input=self.prepare_input_for_vmshell(command, input), **kwargs)
+        except subprocess.CalledProcessError as e:
+            e.cmd = command
+            raise e
+
 
 # pylint: disable=abstract-method
 class AdminVM(QubesVM):