diff --git a/doc/qubes-storage.rst b/doc/qubes-storage.rst index 3d9fafa6..d2abdeab 100644 --- a/doc/qubes-storage.rst +++ b/doc/qubes-storage.rst @@ -99,6 +99,10 @@ Methods and properties required to be implemented by the volume class: - :py:meth:`~qubes.storage.Volume.export` - return a path to be read to extract volume data; for complex formats, this can be a pipe (connected to some data-extracting process) + - :py:meth:`~qubes.storage.Volume.export_end` - cleanup after exporting the + data; this function is called when the path returned by + :py:meth:`~qubes.storage.Volume.export` is not used anymore. This method + optional - some storage drivers may not implement it if not needed. - :py:meth:`~qubes.storage.Volume.import_data` - return a path the data should be written to, to import volume data; for complex formats, this can be pipe (connected to some data-importing process) diff --git a/qubes/backup.py b/qubes/backup.py index 27e511e1..6cb95b87 100644 --- a/qubes/backup.py +++ b/qubes/backup.py @@ -252,12 +252,30 @@ class Backup: # pylint: disable=too-many-instance-attributes class FileToBackup: # pylint: disable=too-few-public-methods - def __init__(self, file_path, subdir=None, name=None, size=None): + def __init__(self, file_path_or_func, subdir=None, name=None, size=None, + cleanup_func=None): + """Store a single file to backup + + :param file_path_or_func: path to the file or a function + returning one; in case of function, it can be a coroutine; + if a function is given, *name*, *subdir* and *size* needs to be + given too + :param subdir: directory in a backup archive to place file in + :param name: name of the file in the backup archive + :param size: size + :param cleanup_func: function to call after processing the file; + the function will get the file path as an argument + """ + if callable(file_path_or_func): + assert subdir is not None \ + and name is not None \ + and size is not None + if size is None: - size = qubes.storage.file.get_disk_usage(file_path) + size = qubes.storage.file.get_disk_usage(file_path_or_func) if subdir is None: - abs_file_path = os.path.abspath(file_path) + abs_file_path = os.path.abspath(file_path_or_func) abs_base_dir = os.path.abspath( qubes.config.system_path["qubes_base_dir"]) + '/' abs_file_dir = os.path.dirname(abs_file_path) + '/' @@ -269,16 +287,19 @@ class Backup: if subdir and not subdir.endswith('/'): subdir += '/' - #: real path to the file - self.path = file_path + if name is None: + name = os.path.basename(file_path_or_func) + + #: real path to the file (or callable to get one) + self.path = file_path_or_func #: size of the file self.size = size #: directory in backup archive where file should be placed self.subdir = subdir #: use this name in the archive (aka rename) - self.name = os.path.basename(file_path) - if name is not None: - self.name = name + self.name = name + #: function to call after processing the file + self.cleanup_func = cleanup_func class VMToBackup: # pylint: disable=too-few-public-methods @@ -370,10 +391,11 @@ class Backup: if not volume.save_on_stop: continue vm_files.append(self.FileToBackup( - volume.export(), + volume.export, subdir, name + '.img', - volume.usage)) + volume.usage, + cleanup_func=volume.export_end)) vm_files.extend(self.FileToBackup(i, subdir) for i in vm.fire_event('backup-get-files')) @@ -514,10 +536,9 @@ class Backup: if retcode: raise qubes.exc.QubesException( "Failed to compute hmac of header file: " - + scrypt.stderr.read()) + + (yield from scrypt.stderr.read()).decode()) return HEADER_FILENAME, HEADER_FILENAME + ".hmac" - def _send_progress_update(self): if not self.total_backup_bytes: return @@ -610,18 +631,21 @@ class Backup: # Files will be verified before untaring this. # Prefix the path in archive with filename["subdir"] to have it # verified during untar + path = file_info.path + if callable(path): + path = yield from qubes.utils.coro_maybe(path()) tar_cmdline = (["tar", "-Pc", '--sparse', - '-C', os.path.dirname(file_info.path)] + + '-C', os.path.dirname(path)] + (['--dereference'] if file_info.subdir != "dom0-home/" else []) + ['--xform=s:^%s:%s\\0:' % ( - os.path.basename(file_info.path), + os.path.basename(path), file_info.subdir), - os.path.basename(file_info.path) + os.path.basename(path) ]) - file_stat = os.stat(file_info.path) + file_stat = os.stat(path) if stat.S_ISBLK(file_stat.st_mode) or \ - file_info.name != os.path.basename(file_info.path): + file_info.name != os.path.basename(path): # tar doesn't handle content of block device, use our # writer # also use our tar writer when renaming file @@ -631,7 +655,7 @@ class Backup: '--override-name=%s' % ( os.path.join(file_info.subdir, os.path.basename( file_info.name))), - file_info.path] + path] if self.compressed: tar_cmdline.insert(-2, "--use-compress-program=%s" % self.compression_filter) @@ -655,6 +679,10 @@ class Backup: except ProcessLookupError: pass raise + finally: + if file_info.cleanup_func is not None: + yield from qubes.utils.coro_maybe( + file_info.cleanup_func(path)) yield from tar_sparse.wait() if tar_sparse.returncode: diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index 03af6312..5624ccb3 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -203,9 +203,24 @@ class Volume: volume data. If extracting volume data require something more than just reading from file (for example connecting to some other domain, or decompressing the data), the returned path may be a pipe. + + This can be implemented as a coroutine. + ''' raise self._not_implemented("export") + def export_end(self, path): + """ Cleanup after exporting data. + + This method is called after exporting the volume data (using + :py:meth:`export`), when the *path* is not needed anymore. + + This can be implemented as a coroutine. + + :param path: path to cleanup, returned by :py:meth:`export` + """ + # do nothing by default (optional method) + def import_data(self, size): ''' Returns a path to overwrite volume data. @@ -423,7 +438,7 @@ class Storage: if 'internal' in volume_config: # migrate old config del volume_config['internal'] - volume = pool.init_volume(self.vm, volume_config) + volume = pool.init_volume(self.vm, volume_config.copy()) self.vm.volumes[name] = volume return volume @@ -634,14 +649,27 @@ class Storage: for target in parsed_xml.xpath( "//domain/devices/disk/target")} + @asyncio.coroutine def export(self, volume): ''' Helper function to export volume (pool.export(volume))''' assert isinstance(volume, (Volume, str)), \ "You need to pass a Volume or pool name as str" - if isinstance(volume, Volume): - return volume.export() + if not isinstance(volume, Volume): + volume = self.vm.volumes[volume] + return (yield from qubes.utils.coro_maybe(volume.export())) - return self.vm.volumes[volume].export() + @asyncio.coroutine + def export_end(self, volume, export_path): + """ Cleanup after exporting data from the volume + + :param volume: volume that was exported + :param export_path: path returned by the export() call + """ + assert isinstance(volume, (Volume, str)), \ + "You need to pass a Volume or pool name as str" + if not isinstance(volume, Volume): + volume = self.vm.volumes[volume] + yield from qubes.utils.coro_maybe(volume.export_end(export_path)) @asyncio.coroutine def import_data(self, volume, size): diff --git a/qubes/storage/file.py b/qubes/storage/file.py index ee1b2738..2c5e9e22 100644 --- a/qubes/storage/file.py +++ b/qubes/storage/file.py @@ -21,7 +21,7 @@ # ''' This module contains pool implementations backed by file images''' - +import asyncio import os import os.path import re @@ -29,6 +29,7 @@ import subprocess from contextlib import suppress import qubes.storage +import qubes.utils BLKSIZE = 512 @@ -268,6 +269,7 @@ class FileVolume(qubes.storage.Volume): # if domain is running return self.path + @asyncio.coroutine def import_volume(self, src_volume): if src_volume.snap_on_start: raise qubes.storage.StoragePoolException( @@ -275,7 +277,11 @@ class FileVolume(qubes.storage.Volume): src_volume, self)) if self.save_on_stop: _remove_if_exists(self.path) - copy_file(src_volume.export(), self.path) + path = yield from qubes.utils.coro_maybe(src_volume.export()) + try: + copy_file(path, self.path) + finally: + yield from qubes.utils.coro_maybe(src_volume.export_end(path)) return self def import_data(self, size): diff --git a/qubes/storage/lvm.py b/qubes/storage/lvm.py index 61c9887e..aab81667 100644 --- a/qubes/storage/lvm.py +++ b/qubes/storage/lvm.py @@ -518,15 +518,19 @@ class ThinVolume(qubes.storage.Volume): self._vid_import.split('/')[1], str(src_volume.size)] yield from qubes_lvm_coro(cmd, self.log) - src_path = src_volume.export() - cmd = ['dd', 'if=' + src_path, 'of=/dev/' + self._vid_import, - 'conv=sparse', 'status=none', 'bs=128K'] - if not os.access('/dev/' + self._vid_import, os.W_OK) or \ - not os.access(src_path, os.R_OK): - cmd.insert(0, 'sudo') + src_path = yield from qubes.utils.coro_maybe(src_volume.export()) + try: + cmd = ['dd', 'if=' + src_path, 'of=/dev/' + self._vid_import, + 'conv=sparse', 'status=none', 'bs=128K'] + if not os.access('/dev/' + self._vid_import, os.W_OK) or \ + not os.access(src_path, os.R_OK): + cmd.insert(0, 'sudo') - p = yield from asyncio.create_subprocess_exec(*cmd) - yield from p.wait() + p = yield from asyncio.create_subprocess_exec(*cmd) + yield from p.wait() + finally: + yield from qubes.utils.coro_maybe( + src_volume.export_end(src_path)) if p.returncode != 0: cmd = ['remove', self._vid_import] yield from qubes_lvm_coro(cmd, self.log) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 7e3d6b22..8b9bfe0d 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -35,6 +35,7 @@ import tempfile from contextlib import contextmanager, suppress import qubes.storage +import qubes.utils FICLONE = 1074041865 # defined in , assuming sizeof(int)==4 LOOP_SET_CAPACITY = 0x4C07 # defined in @@ -297,15 +298,22 @@ class ReflinkVolume(qubes.storage.Volume): _import_data_end)) @qubes.storage.Volume.locked - @_coroutinized + @asyncio.coroutine def import_volume(self, src_volume): if self.save_on_stop: try: success = False - _copy_file(src_volume.export(), self._path_import) + src_path = yield from qubes.utils.coro_maybe( + src_volume.export()) + try: + yield from _coroutinized(_copy_file)( + src_path, self._path_import) + finally: + yield from qubes.utils.coro_maybe( + src_volume.export_end(src_path)) success = True finally: - self._import_data_end(success) + yield from _coroutinized(self._import_data_end)(success) return self def _path_revision(self, number, timestamp=None): diff --git a/qubes/tests/__init__.py b/qubes/tests/__init__.py index ac8d66f0..07e78ef1 100644 --- a/qubes/tests/__init__.py +++ b/qubes/tests/__init__.py @@ -251,6 +251,7 @@ def wait_on_fail(func): lambda: asyncio.StreamReaderProtocol(reader), sys.stdin)) self.loop.run_until_complete(reader.readline()) + transport.close() raise return wrapper diff --git a/qubes/tests/integ/backup.py b/qubes/tests/integ/backup.py index 3b3e78d3..0b597b55 100644 --- a/qubes/tests/integ/backup.py +++ b/qubes/tests/integ/backup.py @@ -36,6 +36,7 @@ import qubes.exc import qubes.storage.lvm import qubes.tests import qubes.tests.storage_lvm +import qubes.utils import qubes.vm import qubes.vm.appvm import qubes.vm.templatevm @@ -106,6 +107,13 @@ class BackupTestsMixin(object): f.close() + def fill_image_vm(self, vm, volume, size=None, sparse=False): + path = self.loop.run_until_complete(vm.storage.export(volume)) + try: + self.fill_image(path, size=size, sparse=sparse) + finally: + self.loop.run_until_complete(vm.storage.export_end(volume, path)) + # NOTE: this was create_basic_vms def create_backup_vms(self, pool=None): template = self.app.default_template @@ -120,7 +128,7 @@ class BackupTestsMixin(object): testnet.create_on_disk(pool=pool)) testnet.features['service.ntpd'] = True vms.append(testnet) - self.fill_image(testnet.storage.export('private'), 20*1024*1024) + self.fill_image_vm(testnet, 'private', 20*1024*1024) vmname = self.make_vm_name('test1') self.log.debug("Creating %s" % vmname) @@ -130,7 +138,7 @@ class BackupTestsMixin(object): self.loop.run_until_complete( testvm1.create_on_disk(pool=pool)) vms.append(testvm1) - self.fill_image(testvm1.storage.export('private'), 100 * 1024 * 1024) + self.fill_image_vm(testvm1, 'private', 100 * 1024 * 1024) vmname = self.make_vm_name('testhvm1') self.log.debug("Creating %s" % vmname) @@ -140,8 +148,7 @@ class BackupTestsMixin(object): label='red') self.loop.run_until_complete( testvm2.create_on_disk(pool=pool)) - self.fill_image(testvm2.storage.export('root'), 1024 * 1024 * 1024, \ - True) + self.fill_image_vm(testvm2, 'root', 1024 * 1024 * 1024, True) vms.append(testvm2) vmname = self.make_vm_name('template') @@ -150,7 +157,7 @@ class BackupTestsMixin(object): name=vmname, label='red') self.loop.run_until_complete( testvm3.create_on_disk(pool=pool)) - self.fill_image(testvm3.storage.export('root'), 100 * 1024 * 1024, True) + self.fill_image_vm(testvm3, 'root', 100 * 1024 * 1024, True) vms.append(testvm3) vmname = self.make_vm_name('custom') @@ -262,11 +269,14 @@ class BackupTestsMixin(object): for name, volume in vm.volumes.items(): if not volume.rw or not volume.save_on_stop: continue - vol_path = volume.export() + vol_path = self.loop.run_until_complete( + qubes.utils.coro_maybe(volume.export())) hasher = hashlib.sha1() with open(vol_path, 'rb') as afile: for buf in iter(lambda: afile.read(4096000), b''): hasher.update(buf) + self.loop.run_until_complete( + qubes.utils.coro_maybe(volume.export_end(vol_path))) hashes[vm.name][name] = hasher.hexdigest() return hashes @@ -382,9 +392,9 @@ class TC_00_Backup(BackupTestsMixin, qubes.tests.SystemTestCase): self.fill_image( os.path.join(self.hvmtemplate.dir_path, '00file'), 195 * 1024 * 1024 - 4096 * 3) - self.fill_image(self.hvmtemplate.storage.export('private'), + self.fill_image_vm(self.hvmtemplate, 'private', 195 * 1024 * 1024 - 4096 * 3) - self.fill_image(self.hvmtemplate.storage.export('root'), 1024 * 1024 * 1024, + self.fill_image_vm(self.hvmtemplate, 'root', 1024 * 1024 * 1024, sparse=True) vms.append(self.hvmtemplate) self.app.save() diff --git a/qubes/tests/integ/basic.py b/qubes/tests/integ/basic.py index 9a870b33..39fb9b61 100644 --- a/qubes/tests/integ/basic.py +++ b/qubes/tests/integ/basic.py @@ -518,9 +518,14 @@ class TC_03_QvmRevertTemplateChanges(qubes.tests.SystemTestCase): self.app.save() def get_rootimg_checksum(self): - return subprocess.check_output( - ['sha1sum', self.test_template.volumes['root'].export()]).\ - decode().split(' ')[0] + path = self.loop.run_until_complete( + self.test_template.storage.export('root')) + try: + return subprocess.check_output(['sha1sum', path]).\ + decode().split(' ')[0] + finally: + self.loop.run_until_complete( + self.test_template.storage.export_end('root', path)) def _do_test(self): checksum_before = self.get_rootimg_checksum() diff --git a/qubes/tests/integ/qrexec.py b/qubes/tests/integ/qrexec.py index 9d0ee996..b357f5f6 100644 --- a/qubes/tests/integ/qrexec.py +++ b/qubes/tests/integ/qrexec.py @@ -20,6 +20,7 @@ # import asyncio +import contextlib import os import subprocess import sys @@ -585,6 +586,10 @@ class TC_00_QrexecMixin(object): except asyncio.TimeoutError: self.fail( "service timeout, probably EOF wasn't transferred from the VM process") + finally: + with contextlib.suppress(ProcessLookupError): + p.terminate() + self.loop.run_until_complete(p.wait()) self.assertEqual(stdout, b'test\n', 'Received data differs from what was expected') @@ -633,6 +638,10 @@ class TC_00_QrexecMixin(object): except asyncio.TimeoutError: self.fail( "service timeout, probably EOF wasn't transferred from the VM process") + finally: + with contextlib.suppress(ProcessLookupError): + p.terminate() + self.loop.run_until_complete(p.wait()) service_descriptor = b'test.Socket+ test-inst-vm1 keyword adminvm\0' self.assertEqual(service_stdout, service_descriptor + b'test1test2', diff --git a/qubes/tests/integ/vm_qrexec_gui.py b/qubes/tests/integ/vm_qrexec_gui.py index 5ed61f91..dba1fccb 100644 --- a/qubes/tests/integ/vm_qrexec_gui.py +++ b/qubes/tests/integ/vm_qrexec_gui.py @@ -101,6 +101,8 @@ class TC_00_AppVMMixin(object): self.skipTest("Minimal template doesn't have 'gnome-terminal'") if 'whonix' in self.template: self.skipTest("Whonix template doesn't have 'gnome-terminal'") + if 'xfce' in self.template: + self.skipTest("Xfce template doesn't have 'gnome-terminal'") self.loop.run_until_complete(self.testvm1.start()) self.assertEqual(self.testvm1.get_power_state(), "Running") self.loop.run_until_complete(self.wait_for_session(self.testvm1)) diff --git a/qubes/tests/storage_lvm.py b/qubes/tests/storage_lvm.py index 8d297c6f..bef958e9 100644 --- a/qubes/tests/storage_lvm.py +++ b/qubes/tests/storage_lvm.py @@ -40,7 +40,7 @@ from qubes.storage.lvm import ThinPool, ThinVolume, qubes_lvm if 'DEFAULT_LVM_POOL' in os.environ.keys(): DEFAULT_LVM_POOL = os.environ['DEFAULT_LVM_POOL'] else: - DEFAULT_LVM_POOL = 'qubes_dom0/pool00' + DEFAULT_LVM_POOL = 'qubes_dom0/vm-pool' def lvm_pool_exists(volume_group, thin_pool): diff --git a/qubes/tests/vm/appvm.py b/qubes/tests/vm/appvm.py index 956b3fc5..88256b2d 100644 --- a/qubes/tests/vm/appvm.py +++ b/qubes/tests/vm/appvm.py @@ -51,11 +51,24 @@ class TestVM(object): def is_running(self): return self.running +class TestVolume(qubes.storage.Volume): + def create(self): + pass + class TestPool(qubes.storage.Pool): + def __init__(self, *args, **kwargs): + super(TestPool, self).__init__(*args, **kwargs) + self._volumes = {} + 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) + vol = TestVolume(vid=vid, pool=self, **volume_config) + self._volumes[vid] = vol + return vol + + def get_volume(self, vid): + return self._volumes[vid] class TC_90_AppVM(qubes.tests.vm.qubesvm.QubesVMTestsMixin, diff --git a/qubes/tests/vm/dispvm.py b/qubes/tests/vm/dispvm.py index 25fe4f45..68b3e185 100644 --- a/qubes/tests/vm/dispvm.py +++ b/qubes/tests/vm/dispvm.py @@ -166,3 +166,28 @@ class TC_00_DispVM(qubes.tests.QubesTestCase): self.app.add_new_vm(qubes.vm.dispvm.DispVM, name='test-dispvm', template=self.appvm) self.assertFalse(mock_domains.get_new_unused_dispid.called) + + @mock.patch('os.symlink') + @mock.patch('os.makedirs') + def test_020_copy_storage_pool(self, mock_makedirs, mock_symlink): + self.app.pools['alternative'] = qubes.tests.vm.appvm.TestPool(name='alternative') + self.appvm.template_for_dispvms = True + self.loop.run_until_complete(self.template.create_on_disk()) + self.loop.run_until_complete(self.appvm.create_on_disk(pool='alternative')) + orig_getitem = self.app.domains.__getitem__ + with mock.patch.object(self.app, 'domains', wraps=self.app.domains) \ + as mock_domains: + mock_domains.configure_mock(**{ + 'get_new_unused_dispid': mock.Mock(return_value=42), + '__getitem__.side_effect': orig_getitem + }) + dispvm = self.app.add_new_vm(qubes.vm.dispvm.DispVM, + name='test-dispvm', template=self.appvm) + self.loop.run_until_complete(dispvm.create_on_disk()) + self.assertEqual(dispvm.template, self.appvm) + self.assertEqual(dispvm.volumes['private'].pool, + self.appvm.volumes['private'].pool) + self.assertEqual(dispvm.volumes['root'].pool, + self.appvm.volumes['root'].pool) + self.assertEqual(dispvm.volumes['volatile'].pool, + self.appvm.volumes['volatile'].pool) diff --git a/qubes/vm/dispvm.py b/qubes/vm/dispvm.py index a11b14f6..138de254 100644 --- a/qubes/vm/dispvm.py +++ b/qubes/vm/dispvm.py @@ -109,6 +109,12 @@ class DispVM(qubes.vm.qubesvm.QubesVM): self.volume_config[name] = config.copy() if 'vid' in self.volume_config[name]: del self.volume_config[name]['vid'] + # copy pool setting from base AppVM; root and private would be + # in the same pool anyway (because of snap_on_start), + # but not volatile, which could be surprising + elif 'pool' not in self.volume_config[name] \ + and 'pool' in config: + self.volume_config[name]['pool'] = config['pool'] super(DispVM, self).__init__(app, xml, *args, **kwargs)