tests: various fixes for storage tests

- improve TestPool mock - init_volume now return appropriate mock type,
   instead of TestPool
 - improve patching base directory (/var/lib/qubes) - it is stored in
   more than one place...
 - fix inheritance in TC_01_ThinPool class
 - fix expected LVM volume names ('vm-' prefix)
 - fix cleanup after FilePool tests - remove temporary qubes.xml
 - asyncio usage
 - better reporting in integ.storage - include error message in the
   report, not only as a comment in code

qubes/tests/integ/storage.py

@@ -22,6 +22,7 @@
 import asyncio
 import os
 import shutil
+import subprocess
 
 import qubes.storage.lvm
 import qubes.tests
@@ -36,11 +37,11 @@ class StorageTestMixin(object):
         self.vm1 = self.app.add_new_vm(qubes.vm.appvm.AppVM,
             name=self.make_vm_name('vm1'),
             label='red')
-        self.vm1.create_on_disk()
+        self.loop.run_until_complete(self.vm1.create_on_disk())
         self.vm2 = self.app.add_new_vm(qubes.vm.appvm.AppVM,
             name=self.make_vm_name('vm2'),
             label='red')
-        self.vm2.create_on_disk()
+        self.loop.run_until_complete(self.vm2.create_on_disk())
         self.pool = None
         self.init_pool()
         self.app.save()
@@ -63,7 +64,9 @@ class StorageTestMixin(object):
             'rw': True,
         }
         testvol = self.vm1.storage.init_volume('testvol', volume_config)
-        yield from self.vm1.storage.get_pool(testvol).create(testvol)
+        coro_maybe = testvol.create()
+        if asyncio.iscoroutine(coro_maybe):
+            yield from coro_maybe
         self.app.save()
         yield from (self.vm1.start())
 
@@ -93,9 +96,10 @@ class StorageTestMixin(object):
             'save_on_stop': True,
             'rw': True,
         }
-        testvol = yield from self.vm1.storage.init_volume(
+        testvol = self.vm1.storage.init_volume('testvol', volume_config)
-            'testvol', volume_config)
+        coro_maybe = testvol.create()
-        yield from self.vm1.storage.get_pool(testvol).create(testvol)
+        if asyncio.iscoroutine(coro_maybe):
+            yield from coro_maybe
         self.app.save()
         yield from self.vm1.start()
         # non-volatile image not clean
@@ -128,7 +132,9 @@ class StorageTestMixin(object):
             'rw': False,
         }
         testvol = self.vm1.storage.init_volume('testvol', volume_config)
-        yield from self.vm1.storage.get_pool(testvol).create(testvol)
+        coro_maybe = testvol.create()
+        if asyncio.iscoroutine(coro_maybe):
+            yield from coro_maybe
         self.app.save()
         yield from self.vm1.start()
         # non-volatile image not clean
@@ -158,7 +164,9 @@ class StorageTestMixin(object):
             'rw': True,
         }
         testvol = self.vm1.storage.init_volume('testvol', volume_config)
-        yield from self.vm1.storage.get_pool(testvol).create(testvol)
+        coro_maybe = testvol.create()
+        if asyncio.iscoroutine(coro_maybe):
+            yield from coro_maybe
         volume_config = {
             'pool': self.pool.name,
             'size': size,
@@ -167,44 +175,61 @@ class StorageTestMixin(object):
             'rw': True,
         }
         testvol_snap = self.vm2.storage.init_volume('testvol', volume_config)
-        yield from self.vm2.storage.get_pool(testvol_snap).create(testvol_snap)
+        coro_maybe = testvol_snap.create()
+        if asyncio.iscoroutine(coro_maybe):
+            yield from coro_maybe
         self.app.save()
         yield from self.vm1.start()
         yield from self.vm2.start()
-        # origin image not clean
-        yield from self.vm1.run_for_stdio(
-            'head -c {} /dev/zero 2>&1 | diff -q /dev/xvde - 2>&1'.format(size),
-            user='root')
 
-        # snapshot image not clean
+        try:
+            yield from self.vm1.run_for_stdio(
+                'head -c {} /dev/zero 2>&1 | diff -q /dev/xvde - 2>&1'.
+                    format(size),
+                user='root')
+        except subprocess.CalledProcessError:
+            self.fail('origin image not clean')
+
+        try:
             yield from self.vm2.run_for_stdio(
                 'head -c {} /dev/zero | diff -q /dev/xvde -'.format(size),
                 user='root')
+        except subprocess.CalledProcessError:
+            self.fail('snapshot image not clean')
 
-        # Write to read-write volume failed
+        try:
-        yield from self.vm1.run_for_stdio('echo test123 > /dev/xvde && sync',
+            yield from self.vm1.run_for_stdio(
+                'echo test123 > /dev/xvde && sync',
                 user='root')
-        # origin changes propagated to snapshot too early
+        except subprocess.CalledProcessError:
+            self.fail('Write to read-write volume failed')
+        try:
             yield from self.vm2.run_for_stdio(
-            'head -c {} /dev/zero 2>&1 | diff -q /dev/xvde - 2>&1'.format(size),
+                'head -c {} /dev/zero 2>&1 | diff -q /dev/xvde - 2>&1'.
+                    format(size),
                 user='root')
+        except subprocess.CalledProcessError:
+            self.fail('origin changes propagated to snapshot too early')
         yield from self.vm1.shutdown(wait=True)
 
         # after origin shutdown there should be still no change
 
-        # origin changes propagated to snapshot too early2
+        try:
             yield from self.vm2.run_for_stdio(
-            'head -c {} /dev/zero 2>&1 | diff -q /dev/xvde - 2>&1'.format(size),
+                'head -c {} /dev/zero 2>&1 | diff -q /dev/xvde - 2>&1'.
+                    format(size),
                 user='root')
+        except subprocess.CalledProcessError:
+            self.fail('origin changes propagated to snapshot too early2')
 
         yield from self.vm2.shutdown(wait=True)
         yield from self.vm2.start()
 
         # only after target VM restart changes should be visible
 
-        # origin changes not visible in snapshot
+        with self.assertRaises(subprocess.CalledProcessError,
-        with self.assertRaises(subprocess.CalledProcessError):
+                msg='origin changes not visible in snapshot'):
-            yield from self.vm2.run(
+            yield from self.vm2.run_for_stdio(
                 'head -c {} /dev/zero 2>&1 | diff -q /dev/xvde - 2>&1'.format(
                     size),
                 user='root')
@@ -224,7 +249,9 @@ class StorageTestMixin(object):
             'rw': True,
         }
         testvol = self.vm1.storage.init_volume('testvol', volume_config)
-        yield from self.vm1.storage.get_pool(testvol).create(testvol)
+        coro_maybe = testvol.create()
+        if asyncio.iscoroutine(coro_maybe):
+            yield from coro_maybe
         volume_config = {
             'pool': self.pool.name,
             'size': size,
@@ -233,7 +260,9 @@ class StorageTestMixin(object):
             'rw': True,
         }
         testvol_snap = self.vm2.storage.init_volume('testvol', volume_config)
-        yield from self.vm2.storage.get_pool(testvol_snap).create(testvol_snap)
+        coro_maybe = testvol_snap.create()
+        if asyncio.iscoroutine(coro_maybe):
+            yield from coro_maybe
         self.app.save()
         yield from self.vm2.start()
 

qubes/tests/storage.py

@@ -19,6 +19,7 @@
 #
 import unittest.mock
 import qubes.log
+import qubes.storage
 from qubes.exc import QubesException
 from qubes.storage import pool_drivers
 from qubes.storage.file import FilePool
@@ -30,10 +31,21 @@ from qubes.tests import SystemTestCase
 class TestPool(unittest.mock.Mock):
     def __init__(self, *args, **kwargs):
         super(TestPool, self).__init__(*args, spec=qubes.storage.Pool, **kwargs)
+        try:
+            self.name = kwargs['name']
+        except KeyError:
+            pass
 
     def __str__(self):
         return 'test'
 
+    def init_volume(self, vm, volume_config):
+        vol = unittest.mock.Mock(spec=qubes.storage.Volume)
+        vol.configure_mock(**volume_config)
+        vol.pool = self
+        vol.import_data.return_value = '/tmp/test-' + vm.name
+        return vol
+
 
 class TestVM(object):
     def __init__(self, test, template=None):

qubes/tests/storage_file.py

@@ -24,6 +24,7 @@ import os
 import shutil
 
 import asyncio
+import unittest.mock
 
 import qubes.storage
 import qubes.tests.storage
@@ -50,6 +51,8 @@ class TestApp(qubes.Qubes):
     def cleanup(self):
         ''' Remove temporary directories '''
         shutil.rmtree(self.pools['linux-kernel'].dir_path)
+        if os.path.exists(self.store):
+            os.unlink(self.store)
 
     def create_dummy_template(self):
         ''' Initalizes a dummy TemplateVM as the `default_template` '''
@@ -306,21 +309,32 @@ class TC_03_FilePool(qubes.tests.QubesTestCase):
     def setUp(self):
         """ Add a test file based storage pool """
         super(TC_03_FilePool, self).setUp()
-        self._orig_qubes_base_dir = qubes.config.qubes_base_dir
+        self.test_base_dir = '/tmp/qubes-test-dir'
-        qubes.config.qubes_base_dir = '/tmp/qubes-test'
+        self.base_dir_patch = unittest.mock.patch.dict(qubes.config.system_path,
+            {'qubes_base_dir': self.test_base_dir})
+        self.base_dir_patch2 = unittest.mock.patch(
+            'qubes.config.qubes_base_dir', self.test_base_dir)
+        self.base_dir_patch3 = unittest.mock.patch.dict(
+            qubes.config.defaults['pool_configs']['varlibqubes'],
+            {'dir_path': self.test_base_dir})
+        self.base_dir_patch.start()
+        self.base_dir_patch2.start()
+        self.base_dir_patch3.start()
         self.app = TestApp()
-        self.app.create_dummy_template()
         self.app.add_pool(**self.POOL_CONFIG)
+        self.app.create_dummy_template()
 
     def tearDown(self):
         """ Remove the file based storage pool after testing """
         self.app.remove_pool("test-pool")
         self.app.cleanup()
+        self.base_dir_patch3.stop()
+        self.base_dir_patch2.stop()
+        self.base_dir_patch.stop()
         super(TC_03_FilePool, self).tearDown()
         shutil.rmtree(self.POOL_DIR, ignore_errors=True)
-        if os.path.exists('/tmp/qubes-test'):
+        if os.path.exists('/tmp/qubes-test-dir'):
-            shutil.rmtree('/tmp/qubes-test')
+            shutil.rmtree('/tmp/qubes-test-dir')
-        qubes.config.qubes_base_dir = self._orig_qubes_base_dir
 
     def test_001_pool_exists(self):
         """ Check if the storage pool was added to the storage pool config """

qubes/tests/storage_lvm.py

@@ -160,7 +160,7 @@ class TC_00_ThinPool(ThinPoolBase):
         volume.remove()
 
 @skipUnlessLvmPoolExists
-class TC_01_ThinPool(qubes.tests.SystemTestCase, ThinPoolBase):
+class TC_01_ThinPool(ThinPoolBase, qubes.tests.SystemTestCase):
     ''' Sanity tests for :py:class:`qubes.storage.lvm.ThinPool` '''
 
     def setUp(self):
@@ -176,7 +176,7 @@ class TC_01_ThinPool(qubes.tests.SystemTestCase, ThinPoolBase):
         vm.clone_disk_files(template_vm, pool='test-lvm')
         for v_name, volume in vm.volumes.items():
             if volume.save_on_stop:
-                expected = "/dev/{!s}/{!s}-{!s}".format(
+                expected = "/dev/{!s}/vm-{!s}-{!s}".format(
                     DEFAULT_LVM_POOL.split('/')[0], vm.name, v_name)
                 self.assertEqual(volume.path, expected)
         with self.assertNotRaises(qubes.exc.QubesException):
@@ -188,7 +188,7 @@ class TC_01_ThinPool(qubes.tests.SystemTestCase, ThinPoolBase):
         vm.create_on_disk(pool='test-lvm')
         for v_name, volume in vm.volumes.items():
             if volume.save_on_stop:
-                expected = "/dev/{!s}/{!s}-{!s}".format(
+                expected = "/dev/{!s}/vm-{!s}-{!s}".format(
                     DEFAULT_LVM_POOL.split('/')[0], vm.name, v_name)
                 self.assertEqual(volume.path, expected)
         with self.assertNotRaises(qubes.exc.QubesException):