Merge branch 'tests-fixes-1'

* tests-fixes-1:
  api: extract function to make pylint happy
  tests/vm: simplify AppVM storage test
  storage: do not use deepcopy on volume configs
  api: cleanup already started servers when some later failed
  tests: fix block devices tests when running on real system
  tests: fix some FD leaks

qubes/api/__init__.py

@@ -345,6 +345,26 @@ class QubesDaemonProtocol(asyncio.Protocol):
         self.transport.write(str(exc).encode('utf-8') + b'\0')
 
 
+def cleanup_socket(sockpath, force):
+    '''Remove socket if stale, or force=True
+    :param sockpath: path to a socket
+    :param force: should remove even if still used
+    '''
+    if force:
+        os.unlink(sockpath)
+    else:
+        sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+        try:
+            sock.connect(sockpath)
+        except ConnectionRefusedError:
+            # dead socket, remove it anyway
+            os.unlink(sockpath)
+        else:
+            # woops, someone is listening
+            sock.close()
+            raise FileExistsError(errno.EEXIST,
+                'socket already exists: {!r}'.format(sockpath))
+
 @asyncio.coroutine
 def create_servers(*args, force=False, loop=None, **kwargs):
     '''Create multiple Qubes API servers
@@ -375,20 +395,7 @@ def create_servers(*args, force=False, loop=None, **kwargs):
                     type(handler).__name__)
 
             if os.path.exists(sockpath):
-                if force:
-                    os.unlink(sockpath)
-                else:
-                    sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
-                    try:
-                        sock.connect(sockpath)
-                    except ConnectionRefusedError:
-                        # dead socket, remove it anyway
-                        os.unlink(sockpath)
-                    else:
-                        # woops, someone is listening
-                        sock.close()
-                        raise FileExistsError(errno.EEXIST,
-                            'socket already exists: {!r}'.format(sockpath))
+                cleanup_socket(sockpath, force)
 
             server = yield from loop.create_unix_server(
                 functools.partial(QubesDaemonProtocol, handler, **kwargs),
@@ -398,7 +405,18 @@ def create_servers(*args, force=False, loop=None, **kwargs):
                 shutil.chown(sock.getsockname(), group='qubes')
 
             servers.append(server)
-
+    except:
+        for server in servers:
+            for sock in server.sockets:
+                try:
+                    os.unlink(sock.getsockname())
+                except FileNotFoundError:
+                    pass
+            server.close()
+        if servers:
+            yield from asyncio.wait([
+                server.wait_closed() for server in servers])
+        raise
     finally:
         os.umask(old_umask)
 

qubes/storage/__init__.py

@@ -349,6 +349,7 @@ class Storage(object):
 
         if hasattr(vm, 'volume_config'):
             for name, conf in self.vm.volume_config.items():
+                conf = conf.copy()
                 if 'source' in conf:
                     template = getattr(vm, 'template', None)
                     if template:

qubes/tests/__init__.py

@@ -692,7 +692,7 @@ class SystemTestsMixin(object):
 
         try:
             vm.remove_from_disk()
-        except: # pylint: disable=bare-except
+        except:  # pylint: disable=bare-except
             pass
 
         del app.domains[vm.qid]
@@ -706,10 +706,11 @@ class SystemTestsMixin(object):
         try:
             conn = libvirt.open(qubes.config.defaults['libvirt_uri'])
             dom = conn.lookupByName(vmname)
-        except: # pylint: disable=bare-except
+        except:  # pylint: disable=bare-except
             pass
         else:
             cls._remove_vm_libvirt(dom)
+            conn.close()
 
         cls._remove_vm_disk(vmname)
 
@@ -753,7 +754,7 @@ class SystemTestsMixin(object):
             subprocess.check_call(['sudo', 'lvremove', '-f'] +
                 [vol.strip() for vol in volumes.splitlines()
                     if ('/' + prefix) in vol],
-                stdout=open(os.devnull, 'w'))
+                stdout=subprocess.DEVNULL)
         except subprocess.CalledProcessError:
             pass
 
@@ -827,7 +828,7 @@ class SystemTestsMixin(object):
 
         wait_count = 0
         while subprocess.call(['xdotool', 'search', '--name', title],
-                stdout=open(os.path.devnull, 'w'), stderr=subprocess.STDOUT) \
+                stdout=subprocess.DEVNULL, stderr=subprocess.STDOUT) \
                     == int(show):
             wait_count += 1
             if wait_count > timeout*10:
@@ -873,7 +874,7 @@ class SystemTestsMixin(object):
         # create a single partition
         p = subprocess.Popen(['sfdisk', '-q', '-L', vm.storage.root_img],
             stdin=subprocess.PIPE,
-            stdout=open(os.devnull, 'w'),
+            stdout=subprocess.DEVNULL,
             stderr=subprocess.STDOUT)
         p.communicate('2048,\n')
         assert p.returncode == 0, 'sfdisk failed'
@@ -892,7 +893,7 @@ class SystemTestsMixin(object):
                 '--target', 'i386-pc',
                 '--modules', 'part_msdos ext2',
                 '--boot-directory', mountpoint, loopdev],
-                stderr=open(os.devnull, 'w')
+                stderr=subprocess.DEVNULL
             )
             grub_cfg = '{}/grub2/grub.cfg'.format(mountpoint)
             subprocess.check_call(
@@ -927,7 +928,7 @@ class SystemTestsMixin(object):
             subprocess.check_call(
                 ['dracut'] + dracut_args + [os.path.join(mountpoint,
                     'initrd')],
-                stderr=open(os.devnull, 'w')
+                stderr=subprocess.DEVNULL
             )
         finally:
             subprocess.check_call(['sudo', 'umount', mountpoint])

qubes/tests/devices_block.py

@@ -110,6 +110,8 @@ class TestApp(object):
         self.env = jinja2.Environment(
             loader=jinja2.FileSystemLoader([
                 'templates',
+                '/etc/qubes/templates',
+                '/usr/share/qubes/templates',
             ]),
             undefined=jinja2.StrictUndefined)
         self.domains = {}

qubes/tests/vm/appvm.py

@@ -20,6 +20,7 @@
 
 from unittest import mock
 
+import qubes.storage
 import qubes.tests
 import qubes.tests.vm.qubesvm
 import qubes.vm.appvm
@@ -48,12 +49,18 @@ class TestVM(object):
     def is_running(self):
         return self.running
 
+class TestPool(qubes.storage.Pool):
+    def init_volume(self, vm, volume_config):
+        vid = '{}/{}'.format(vm.name, volume_config['name'])
+        assert volume_config.pop('pool', None) == self
+        return qubes.storage.Volume(vid=vid, pool=self, **volume_config)
+
+
 class TC_90_AppVM(qubes.tests.vm.qubesvm.QubesVMTestsMixin,
         qubes.tests.QubesTestCase):
     def setUp(self):
         super().setUp()
-        self.app.pools['default'] = mock.Mock(**{
-            'init_volume.return_value.pool': 'default'})
+        self.app.pools['default'] = TestPool('default')
         self.app.pools['linux-kernel'] = mock.Mock(**{
             'init_volume.return_value.pool': 'linux-kernel'})
         self.template = qubes.vm.templatevm.TemplateVM(self.app, None,

qubes/vm/appvm.py

@@ -93,7 +93,7 @@ class AppVM(qubes.vm.qubesvm.QubesVM):
                 # in case the template vm has more volumes add them to own
                 # config
                 if name not in self.volume_config:
-                    self.volume_config[name] = copy.deepcopy(config)
+                    self.volume_config[name] = config.copy()
                     if 'vid' in self.volume_config[name]:
                         del self.volume_config[name]['vid']
 
@@ -123,7 +123,7 @@ class AppVM(qubes.vm.qubesvm.QubesVM):
         for volume_name, conf in self.default_volume_config.items():
             if conf.get('snap_on_start', False) and \
                     conf.get('source', None) is None:
-                config = copy.deepcopy(conf)
+                config = conf.copy()
                 template_volume = newvalue.volumes[volume_name]
                 self.volume_config[volume_name] = \
                     self.config_volume_from_source(

qubes/vm/dispvm.py

@@ -21,8 +21,6 @@
 
 ''' A disposable vm implementation '''
 
-import copy
-
 import qubes.vm.qubesvm
 import qubes.vm.appvm
 import qubes.config
@@ -89,7 +87,7 @@ class DispVM(qubes.vm.qubesvm.QubesVM):
                 # in case the template vm has more volumes add them to own
                 # config
                 if name not in self.volume_config:
-                    self.volume_config[name] = copy.deepcopy(config)
+                    self.volume_config[name] = config.copy()
                     if 'vid' in self.volume_config[name]:
                         del self.volume_config[name]['vid']
 

qubes/vm/qubesvm.py

@@ -24,7 +24,6 @@
 from __future__ import absolute_import
 
 import asyncio
-import copy
 import base64
 import datetime
 import os
@@ -1799,8 +1798,7 @@ def _clean_volume_config(config):
     common_attributes = ['name', 'pool', 'size',
                          'revisions_to_keep', 'rw', 'snap_on_start',
                          'save_on_stop', 'source']
-    config_copy = copy.deepcopy(config)
-    return {k: v for k, v in config_copy.items() if k in common_attributes}
+    return {k: v for k, v in config.items() if k in common_attributes}
 
 
 def _patch_pool_config(config, pool=None, pools=None):