From 850778b52af89fd20bafe1abfd7eeb6131d1f49e Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 9 Sep 2018 20:01:11 +0000 Subject: [PATCH 01/30] storage/reflink: remove redundant format specifiers --- qubes/storage/reflink.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 574febb3..e541555c 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -132,7 +132,7 @@ class ReflinkVolume(qubes.storage.Volume): if img is None or os.path.exists(img): return True raise qubes.storage.StoragePoolException( - 'Missing image file {!r} for volume {!s}'.format(img, self.vid)) + 'Missing image file {!r} for volume {}'.format(img, self.vid)) def remove(self): ''' Drop volume object from pool; remove volume images from @@ -214,12 +214,12 @@ class ReflinkVolume(qubes.storage.Volume): ''' if not self.rw: raise qubes.storage.StoragePoolException( - 'Cannot resize: {!s} is read-only'.format(self.vid)) + 'Cannot resize: {} is read-only'.format(self.vid)) if size < self.size: raise qubes.storage.StoragePoolException( - 'For your own safety, shrinking of {!s} is disabled' - ' ({:d} < {:d}). If you really know what you are doing,' + 'For your own safety, shrinking of {} is disabled' + ' ({} < {}). If you really know what you are doing,' ' use "truncate" manually.'.format(self.vid, size, self.size)) try: # assume volume is not (cleanly) stopped ... @@ -240,7 +240,7 @@ class ReflinkVolume(qubes.storage.Volume): def _require_save_on_stop(self, method_name): if not self.save_on_stop: raise NotImplementedError( - 'Cannot {!s}: {!s} is not save_on_stop'.format( + 'Cannot {}: {} is not save_on_stop'.format( method_name, self.vid)) def export(self): @@ -408,7 +408,7 @@ def _cmd(*args): stdout=subprocess.PIPE, stderr=subprocess.PIPE).stdout except subprocess.CalledProcessError as ex: - msg = '{!s} err={!r} out={!r}'.format(ex, ex.stderr, ex.stdout) + msg = '{} err={!r} out={!r}'.format(ex, ex.stderr, ex.stdout) raise qubes.storage.StoragePoolException(msg) from ex def is_reflink_supported(dst_dir, src_dir=None): From 677183d8a6932aa9e8dae8de29c8047b222d6412 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 9 Sep 2018 20:01:12 +0000 Subject: [PATCH 02/30] storage/reflink: add revision even if empty It's sort of useful to be able to revert a volume that has only ever been started once to its empty state. And the lvm_thin driver allows it too, so why not. --- qubes/storage/reflink.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index e541555c..2a0321b0 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -184,8 +184,6 @@ class ReflinkVolume(qubes.storage.Volume): def _add_revision(self): if self.revisions_to_keep == 0: return - if _get_file_disk_usage(self._path_clean) == 0: - return ctime = os.path.getctime(self._path_clean) timestamp = qubes.storage.isodate(int(ctime)) _copy_file(self._path_clean, From 18f9356c2c70fef72f8e2cd837c3dde83ecb2e05 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 9 Sep 2018 20:01:13 +0000 Subject: [PATCH 03/30] storage/reflink: refuse to revert() dirty volume --- qubes/storage/reflink.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 2a0321b0..bcf2e3dc 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -197,6 +197,9 @@ class ReflinkVolume(qubes.storage.Volume): _remove_file(self._path_revision(number, timestamp)) def revert(self, revision=None): + if self.is_dirty(): + raise qubes.storage.StoragePoolException( + 'Cannot revert: {} is not cleanly stopped'.format(self.vid)) if revision is None: number, timestamp = list(self.revisions.items())[-1] else: From ef2698adb4aed4fdfb47e6bc186ece1e76c13337 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 9 Sep 2018 20:01:14 +0000 Subject: [PATCH 04/30] storage/reflink: make revisions() more readable, use iglob --- qubes/storage/reflink.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index bcf2e3dc..20fa2a97 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -297,10 +297,10 @@ class ReflinkVolume(qubes.storage.Volume): @property def revisions(self): prefix = self._path_clean + '.' - paths = glob.glob(glob.escape(prefix) + '*@*Z') - items = sorted((path[len(prefix):-1].split('@') for path in paths), - key=lambda item: int(item[0])) - return collections.OrderedDict(items) + paths = glob.iglob(glob.escape(prefix) + '*@*Z') + items = (path[len(prefix):-1].split('@') for path in paths) + return collections.OrderedDict(sorted(items, + key=lambda item: int(item[0]))) @property def usage(self): From 75a4a1340e5f444d8b57eddac04cb9da520afee9 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 9 Sep 2018 20:01:15 +0000 Subject: [PATCH 05/30] storage/reflink: don't recompute static properties per call --- qubes/storage/reflink.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 20fa2a97..e3582c8f 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -116,6 +116,13 @@ class ReflinkPool(qubes.storage.Pool): self.dir_path) class ReflinkVolume(qubes.storage.Volume): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._path_vid = os.path.join(self.pool.dir_path, self.vid) + self._path_clean = self._path_vid + '.img' + self._path_dirty = self._path_vid + '-dirty.img' + self.path = self._path_dirty + def create(self): if self.save_on_stop and not self.snap_on_start: _create_sparse_file(self._path_clean, self.size) @@ -275,18 +282,6 @@ class ReflinkVolume(qubes.storage.Volume): timestamp = self.revisions[number] return self._path_clean + '.' + number + '@' + timestamp + 'Z' - @property - def _path_clean(self): - return os.path.join(self.pool.dir_path, self.vid + '.img') - - @property - def _path_dirty(self): - return os.path.join(self.pool.dir_path, self.vid + '-dirty.img') - - @property - def path(self): - return self._path_dirty - @property def _next_revision_number(self): numbers = self.revisions.keys() From d301aa2e501a33981cc5537b5cbc119081693d8d Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 9 Sep 2018 20:01:17 +0000 Subject: [PATCH 06/30] storage/reflink: delete stale tempfiles on start and remove When the AT_REPLACE flag for linkat() finally lands in the Linux kernel, _replace_file() can be modified to use unnamed (O_TMPFILE) tempfiles. Until then, make sure stale tempfiles from previous crashes can't hang around for too long. --- qubes/storage/reflink.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index e3582c8f..04467cce 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -146,12 +146,17 @@ class ReflinkVolume(qubes.storage.Volume): oldest to newest; remove empty VM directory. ''' self.pool._volumes.pop(self, None) # pylint: disable=protected-access + self._cleanup() self._prune_revisions(keep=0) _remove_file(self._path_clean) _remove_file(self._path_dirty) _remove_empty_dir(os.path.dirname(self._path_dirty)) return self + def _cleanup(self): + for tmp in glob.iglob(glob.escape(self._path_vid) + '*.img*~*'): + _remove_file(tmp) + def is_outdated(self): if self.snap_on_start: with suppress(FileNotFoundError): @@ -164,6 +169,7 @@ class ReflinkVolume(qubes.storage.Volume): return self.save_on_stop and os.path.exists(self._path_dirty) def start(self): + self._cleanup() if self.is_dirty(): # implies self.save_on_stop return self if self.snap_on_start: From 60bf68a74866a94e50e988b497a9709dec427ee3 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 9 Sep 2018 20:01:18 +0000 Subject: [PATCH 07/30] storage/reflink: add _path_import (don't reuse _path_dirty) Import volume data to a new _path_import (instead of _path_dirty) before committing to _path_clean. In case the computer crashes while an import operation is running, the partially written file should not be attached to Xen on the next volume startup. Use -import.img as the filename like 'file' does, to be compatible with qubes.tests.api_admin/TC_00_VMs/test_510_vm_volume_import. --- qubes/storage/reflink.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 04467cce..264a8b5d 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -121,6 +121,7 @@ class ReflinkVolume(qubes.storage.Volume): self._path_vid = os.path.join(self.pool.dir_path, self.vid) self._path_clean = self._path_vid + '.img' self._path_dirty = self._path_vid + '-dirty.img' + self._path_import = self._path_vid + '-import.img' self.path = self._path_dirty def create(self): @@ -156,6 +157,7 @@ class ReflinkVolume(qubes.storage.Volume): def _cleanup(self): for tmp in glob.iglob(glob.escape(self._path_vid) + '*.img*~*'): _remove_file(tmp) + _remove_file(self._path_import) def is_outdated(self): if self.snap_on_start: @@ -183,16 +185,16 @@ class ReflinkVolume(qubes.storage.Volume): def stop(self): if self.save_on_stop: - self._commit() + self._commit(self._path_dirty) else: _remove_file(self._path_dirty) _remove_file(self._path_clean) return self - def _commit(self): + def _commit(self, path_from): self._add_revision() self._prune_revisions() - _rename_file(self._path_dirty, self._path_clean) + _rename_file(path_from, self._path_clean) def _add_revision(self): if self.revisions_to_keep == 0: @@ -263,20 +265,20 @@ class ReflinkVolume(qubes.storage.Volume): def import_data(self): self._require_save_on_stop('import_data') - _create_sparse_file(self._path_dirty, self.size) - return self._path_dirty + _create_sparse_file(self._path_import, self.size) + return self._path_import def import_data_end(self, success): if success: - self._commit() + self._commit(self._path_import) else: - _remove_file(self._path_dirty) + _remove_file(self._path_import) return self def import_volume(self, src_volume): self._require_save_on_stop('import_volume') try: - _copy_file(src_volume.export(), self._path_dirty) + _copy_file(src_volume.export(), self._path_import) except: self.import_data_end(False) raise From 6e8d7d42016514698a7eed0644ea30a699771fc4 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 9 Sep 2018 20:01:19 +0000 Subject: [PATCH 08/30] storage/reflink: no-op import_volume() if not save_on_stop Instead of raising a NotImplementedError, just return self like 'file' and lvm_thin. This is needed when Storage.clone() is modified in another commit* to no longer swallow exceptions. * "storage: factor out _wait_and_reraise(); fix clone/create" --- qubes/storage/reflink.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 264a8b5d..8b169c92 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -276,7 +276,8 @@ class ReflinkVolume(qubes.storage.Volume): return self def import_volume(self, src_volume): - self._require_save_on_stop('import_volume') + if not self.save_on_stop: + return self try: _copy_file(src_volume.export(), self._path_import) except: From e7b7c253acacb9cc8b9190091cf5679e22e7d65f Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 9 Sep 2018 20:01:20 +0000 Subject: [PATCH 09/30] storage/reflink: inline _require_self_on_stop() --- qubes/storage/reflink.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 8b169c92..69353eb1 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -253,18 +253,16 @@ class ReflinkVolume(qubes.storage.Volume): return self - def _require_save_on_stop(self, method_name): + def export(self): if not self.save_on_stop: raise NotImplementedError( - 'Cannot {}: {} is not save_on_stop'.format( - method_name, self.vid)) - - def export(self): - self._require_save_on_stop('export') + 'Cannot export: {} is not save_on_stop'.format(self.vid)) return self._path_clean def import_data(self): - self._require_save_on_stop('import_data') + if not self.save_on_stop: + raise NotImplementedError( + 'Cannot import_data: {} is not save_on_stop'.format(self.vid)) _create_sparse_file(self._path_import, self.size) return self._path_import From 385ba917727683942635748c84c14db332db31ae Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 9 Sep 2018 20:01:22 +0000 Subject: [PATCH 10/30] storage/reflink: resize(): don't look for loopdevs if clean --- qubes/storage/reflink.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 69353eb1..88ef8394 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -240,10 +240,11 @@ class ReflinkVolume(qubes.storage.Volume): try: # assume volume is not (cleanly) stopped ... _resize_file(self._path_dirty, size) + self.size = size except FileNotFoundError: # ... but it actually is. _resize_file(self._path_clean, size) - - self.size = size + self.size = size + return self # resize any corresponding loop devices out = _cmd('losetup', '--associated', self._path_dirty) From fb06a8089abf82f3a8075b7420d64caae745b839 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:10 +0000 Subject: [PATCH 11/30] storage/reflink: _update_loopdev_sizes() without losetup Factor out a function, and use the LOOP_SET_CAPACITY ioctl instead of going through losetup. --- qubes/storage/reflink.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 88ef8394..8dcfab2a 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -28,7 +28,6 @@ import fcntl import glob import logging import os -import re import subprocess import tempfile from contextlib import contextmanager, suppress @@ -36,7 +35,8 @@ from contextlib import contextmanager, suppress import qubes.storage BLKSIZE = 512 -FICLONE = 1074041865 # see ioctl_ficlone manpage +FICLONE = 1074041865 # defined in +LOOP_SET_CAPACITY = 0x4C07 # defined in LOGGER = logging.getLogger('qubes.storage.reflink') @@ -246,12 +246,7 @@ class ReflinkVolume(qubes.storage.Volume): self.size = size return self - # resize any corresponding loop devices - out = _cmd('losetup', '--associated', self._path_dirty) - for match in re.finditer(br'^(/dev/loop[0-9]+): ', out, re.MULTILINE): - loop_dev = match.group(1).decode('ascii') - _cmd('losetup', '--set-capacity', loop_dev) - + _update_loopdev_sizes(self._path_dirty) return self def export(self): @@ -395,6 +390,19 @@ def _create_sparse_file(path, size): tmp.truncate(size) LOGGER.info('Created sparse file: %s', tmp.name) +def _update_loopdev_sizes(img): + ''' Resolve img; update the size of loop devices backed by it. ''' + needle = os.fsencode(os.path.realpath(img)) + b'\n' + for sys_path in glob.iglob('/sys/block/loop[0-9]*/loop/backing_file'): + try: + with open(sys_path, 'rb') as sys_io: + if sys_io.read() != needle: + continue + except FileNotFoundError: + continue + with open('/dev/' + sys_path.split('/')[3]) as dev_io: + fcntl.ioctl(dev_io.fileno(), LOOP_SET_CAPACITY) + def _copy_file(src, dst): ''' Copy src to dst as a reflink if possible, sparse if not. ''' if not os.path.exists(src): From 69af0a48ecb4df096ebce910c563c66e9d2b2ae8 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:12 +0000 Subject: [PATCH 12/30] storage/reflink: inline and simplify _cmd() --- qubes/storage/reflink.py | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 8dcfab2a..bb7c4e4d 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -409,19 +409,10 @@ def _copy_file(src, dst): raise FileNotFoundError(src) with _replace_file(dst) as tmp: LOGGER.info('Copying file: %s -> %s', src, tmp.name) - _cmd('cp', '--sparse=always', '--reflink=auto', src, tmp.name) - -def _cmd(*args): - ''' Run command until finished; return stdout (as bytes) if it - exited 0. Otherwise, raise a detailed StoragePoolException. - ''' - try: - return subprocess.run(args, check=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE).stdout - except subprocess.CalledProcessError as ex: - msg = '{} err={!r} out={!r}'.format(ex, ex.stderr, ex.stdout) - raise qubes.storage.StoragePoolException(msg) from ex + cmd = 'cp', '--sparse=always', '--reflink=auto', src, tmp.name + p = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + if p.returncode != 0: + raise qubes.storage.StoragePoolException(str(p)) def is_reflink_supported(dst_dir, src_dir=None): ''' Return whether destination directory supports reflink copies From edda3a1734505b7cf652c7624524319b778d58a6 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:13 +0000 Subject: [PATCH 13/30] storage/reflink: factor out _ficlone() --- qubes/storage/reflink.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index bb7c4e4d..e8841964 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -403,6 +403,13 @@ def _update_loopdev_sizes(img): with open('/dev/' + sys_path.split('/')[3]) as dev_io: fcntl.ioctl(dev_io.fileno(), LOOP_SET_CAPACITY) +def _attempt_ficlone(src, dst): + try: + fcntl.ioctl(dst.fileno(), FICLONE, src.fileno()) + return True + except OSError: + return False + def _copy_file(src, dst): ''' Copy src to dst as a reflink if possible, sparse if not. ''' if not os.path.exists(src): @@ -424,9 +431,4 @@ def is_reflink_supported(dst_dir, src_dir=None): dst = tempfile.TemporaryFile(dir=dst_dir) src = tempfile.TemporaryFile(dir=src_dir) src.write(b'foo') # don't let any filesystem get clever with empty files - - try: - fcntl.ioctl(dst.fileno(), FICLONE, src.fileno()) - return True - except OSError: - return False + return _attempt_ficlone(src, dst) From 3d986be02a52d699c2b1e0484757b821177f562b Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:14 +0000 Subject: [PATCH 14/30] storage/reflink: native FICLONE in _copy_file() happy path Avoid a subprocess launch, and distinguish reflink vs. fallback copy in the log. --- qubes/storage/reflink.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index e8841964..c9862632 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -412,14 +412,17 @@ def _attempt_ficlone(src, dst): def _copy_file(src, dst): ''' Copy src to dst as a reflink if possible, sparse if not. ''' - if not os.path.exists(src): - raise FileNotFoundError(src) - with _replace_file(dst) as tmp: - LOGGER.info('Copying file: %s -> %s', src, tmp.name) - cmd = 'cp', '--sparse=always', '--reflink=auto', src, tmp.name + with _replace_file(dst) as tmp_io: + with open(src, 'rb') as src_io: + if _attempt_ficlone(src_io, tmp_io): + LOGGER.info('Reflinked file: %s -> %s', src, tmp_io.name) + return True + LOGGER.info('Copying file: %s -> %s', src, tmp_io.name) + cmd = 'cp', '--sparse=always', src, tmp_io.name p = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) if p.returncode != 0: raise qubes.storage.StoragePoolException(str(p)) + return False def is_reflink_supported(dst_dir, src_dir=None): ''' Return whether destination directory supports reflink copies From 1889c9b75fef2da7c120b108b77cd5228c4a4db2 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:15 +0000 Subject: [PATCH 15/30] storage/reflink: run synchronous volume methods in executor Convert create(), verify(), remove(), start(), stop(), revert(), resize(), and import_volume() into coroutine methods, via a decorator that runs them in the event loop's thread-based default executor. This reduces UI hangs by unblocking the event loop, and can e.g. speed up VM starts by starting multiple volumes in parallel. --- qubes/storage/reflink.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index c9862632..d7f93a72 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -22,9 +22,11 @@ but not required. ''' +import asyncio import collections import errno import fcntl +import functools import glob import logging import os @@ -115,20 +117,37 @@ class ReflinkPool(qubes.storage.Pool): [pool for pool in app.pools.values() if pool is not self], self.dir_path) + +def _unblock(method): + ''' Decorator transforming a synchronous volume method into a + coroutine that runs the original method in the event loop's + thread-based default executor, under a per-volume lock. + ''' + @asyncio.coroutine + @functools.wraps(method) + def wrapper(self, *args, **kwargs): + with (yield from self._lock): # pylint: disable=protected-access + return (yield from asyncio.get_event_loop().run_in_executor( + None, functools.partial(method, self, *args, **kwargs))) + return wrapper + class ReflinkVolume(qubes.storage.Volume): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + self._lock = asyncio.Lock() self._path_vid = os.path.join(self.pool.dir_path, self.vid) self._path_clean = self._path_vid + '.img' self._path_dirty = self._path_vid + '-dirty.img' self._path_import = self._path_vid + '-import.img' self.path = self._path_dirty + @_unblock def create(self): if self.save_on_stop and not self.snap_on_start: _create_sparse_file(self._path_clean, self.size) return self + @_unblock def verify(self): if self.snap_on_start: img = self.source._path_clean # pylint: disable=protected-access @@ -142,6 +161,7 @@ class ReflinkVolume(qubes.storage.Volume): raise qubes.storage.StoragePoolException( 'Missing image file {!r} for volume {}'.format(img, self.vid)) + @_unblock def remove(self): ''' Drop volume object from pool; remove volume images from oldest to newest; remove empty VM directory. @@ -170,6 +190,7 @@ class ReflinkVolume(qubes.storage.Volume): def is_dirty(self): return self.save_on_stop and os.path.exists(self._path_dirty) + @_unblock def start(self): self._cleanup() if self.is_dirty(): # implies self.save_on_stop @@ -183,6 +204,7 @@ class ReflinkVolume(qubes.storage.Volume): _create_sparse_file(self._path_dirty, self.size) return self + @_unblock def stop(self): if self.save_on_stop: self._commit(self._path_dirty) @@ -211,6 +233,7 @@ class ReflinkVolume(qubes.storage.Volume): for number, timestamp in list(self.revisions.items())[:-keep or None]: _remove_file(self._path_revision(number, timestamp)) + @_unblock def revert(self, revision=None): if self.is_dirty(): raise qubes.storage.StoragePoolException( @@ -224,6 +247,7 @@ class ReflinkVolume(qubes.storage.Volume): _rename_file(path_revision, self._path_clean) return self + @_unblock def resize(self, size): ''' Expand a read-write volume image; notify any corresponding loop devices of the size change. @@ -269,6 +293,7 @@ class ReflinkVolume(qubes.storage.Volume): _remove_file(self._path_import) return self + @_unblock def import_volume(self, src_volume): if not self.save_on_stop: return self From c75fe098141d3ada2bea5d90e69b8502600e6965 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:16 +0000 Subject: [PATCH 16/30] storage/reflink: is_reflink_supported() -> is_supported() --- qubes/storage/reflink.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index d7f93a72..b70094cb 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -55,7 +55,7 @@ class ReflinkPool(qubes.storage.Pool): def setup(self): created = _make_dir(self.dir_path) - if self.setup_check and not is_reflink_supported(self.dir_path): + if self.setup_check and not is_supported(self.dir_path): if created: _remove_empty_dir(self.dir_path) raise qubes.storage.StoragePoolException( @@ -449,7 +449,7 @@ def _copy_file(src, dst): raise qubes.storage.StoragePoolException(str(p)) return False -def is_reflink_supported(dst_dir, src_dir=None): +def is_supported(dst_dir, src_dir=None): ''' Return whether destination directory supports reflink copies from source directory. (A temporary file is created in each directory, using O_TMPFILE if possible.) From 266d90c2f954d44d3a84473be8b3656a0b226d2f Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:18 +0000 Subject: [PATCH 17/30] storage: fix docstrings --- qubes/storage/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index f506c12b..fe5e98bb 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -252,6 +252,8 @@ class Volume: def revert(self, revision=None): ''' Revert volume to previous revision + This can be implemented as a coroutine. + :param revision: revision to revert volume to, see :py:attr:`revisions` ''' # pylint: disable=unused-argument @@ -616,7 +618,7 @@ class Storage: @asyncio.coroutine def start(self): - ''' Execute the start method on each pool ''' + ''' Execute the start method on each volume ''' futures = [] for volume in self.vm.volumes.values(): ret = volume.start() @@ -631,7 +633,7 @@ class Storage: @asyncio.coroutine def stop(self): - ''' Execute the start method on each pool ''' + ''' Execute the stop method on each volume ''' futures = [] for volume in self.vm.volumes.values(): ret = volume.stop() From 44ca78523f915d217e85343c657fd649a4635d84 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:19 +0000 Subject: [PATCH 18/30] storage: insert missing NotImplementedError in Volume.stop() --- qubes/storage/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index fe5e98bb..b8e5f5b2 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -274,6 +274,7 @@ class Volume: This include committing data if :py:attr:`save_on_stop` is set. This can be implemented as a coroutine.''' + raise self._not_implemented("stop") def verify(self): ''' Verifies the volume. From f0ee73e63f7f836b8108373884607273e6accb7e Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:20 +0000 Subject: [PATCH 19/30] storage: remove broken default parameter from isodate() With that syntax, the default timestamp would have been from the time of the function's definition (not invocation). But all callers are passing an explicit timestamp anyway. --- qubes/storage/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index b8e5f5b2..59d0d210 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -874,7 +874,7 @@ def driver_parameters(name): return [p for p in params if p not in ignored_params] -def isodate(seconds=time.time()): +def isodate(seconds): ''' Helper method which returns an iso date ''' return datetime.utcfromtimestamp(seconds).isoformat("T") From d33bd3f2b629fc030afcc0e9b5fa97f7a57f6b0a Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:21 +0000 Subject: [PATCH 20/30] storage: fix search_pool_containing_dir() Canonicalize both directories, resolving symlink components. Compare with commonpath() instead of startswith(), because /foo doesn't contain /foobar. --- qubes/storage/__init__.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index 59d0d210..4631616b 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -884,17 +884,21 @@ def search_pool_containing_dir(pools, dir_path): This is useful for implementing Pool.included_in method ''' + real_dir_path = os.path.realpath(dir_path) + # prefer filesystem pools for pool in pools: if hasattr(pool, 'dir_path'): - if dir_path.startswith(pool.dir_path): + pool_real_dir_path = os.path.realpath(pool.dir_path) + if os.path.commonpath([pool_real_dir_path, real_dir_path]) == \ + pool_real_dir_path: return pool # then look for lvm for pool in pools: if hasattr(pool, 'thin_pool') and hasattr(pool, 'volume_group'): if (pool.volume_group, pool.thin_pool) == \ - DirectoryThinPool.thin_pool(dir_path): + DirectoryThinPool.thin_pool(real_dir_path): return pool return None From d181bf1aa4912013addc233cc93ba1de585b25c5 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:22 +0000 Subject: [PATCH 21/30] storage: factor out _wait_and_reraise(); fix clone/create _wait_and_reraise() is similar to asyncio.gather(), but it preserves the current behavior of waiting for all futures and only _then_ reraising the first exception (if there is any) in line. Also switch Storage.create() and Storage.clone() to _wait_and_reraise(). Previously, they called asyncio.wait() and implicitly swallowed all exceptions. --- qubes/storage/__init__.py | 43 ++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index 4631616b..c81832a9 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -509,8 +509,7 @@ class Storage: ret = volume.create() if asyncio.iscoroutine(ret): coros.append(ret) - if coros: - yield from asyncio.wait(coros) + yield from _wait_and_reraise(coros) os.umask(old_umask) @@ -552,7 +551,7 @@ class Storage: self.vm.volumes = {} with VmCreationManager(self.vm): - yield from asyncio.wait([self.clone_volume(src_vm, vol_name) + yield from _wait_and_reraise([self.clone_volume(src_vm, vol_name) for vol_name in self.vm.volume_config.keys()]) @property @@ -584,11 +583,7 @@ class Storage: ret = volume.verify() if asyncio.iscoroutine(ret): futures.append(ret) - if futures: - done, _ = yield from asyncio.wait(futures) - for task in done: - # re-raise any exception from async task - task.result() + yield from _wait_and_reraise(futures) self.vm.fire_event('domain-verify-files') return True @@ -608,14 +603,10 @@ class Storage: except (IOError, OSError) as e: self.vm.log.exception("Failed to remove volume %s", name, e) - if futures: - try: - done, _ = yield from asyncio.wait(futures) - for task in done: - # re-raise any exception from async task - task.result() - except (IOError, OSError) as e: - self.vm.log.exception("Failed to remove some volume", e) + try: + yield from _wait_and_reraise(futures) + except (IOError, OSError) as e: + self.vm.log.exception("Failed to remove some volume", e) @asyncio.coroutine def start(self): @@ -626,11 +617,7 @@ class Storage: if asyncio.iscoroutine(ret): futures.append(ret) - if futures: - done, _ = yield from asyncio.wait(futures) - for task in done: - # re-raise any exception from async task - task.result() + yield from _wait_and_reraise(futures) @asyncio.coroutine def stop(self): @@ -641,11 +628,7 @@ class Storage: if asyncio.iscoroutine(ret): futures.append(ret) - if futures: - done, _ = yield from asyncio.wait(futures) - for task in done: - # re-raise any exception from async task - task.result() + yield from _wait_and_reraise(futures) def unused_frontend(self): ''' Find an unused device name ''' @@ -845,6 +828,14 @@ class Pool: return NotImplementedError(msg) +@asyncio.coroutine +def _wait_and_reraise(futures): + if futures: + done, _ = yield from asyncio.wait(futures) + for task in done: # (re-)raise first exception in line + task.result() + + def _sanitize_config(config): ''' Helper function to convert types to appropriate strings ''' # FIXME: find another solution for serializing basic types From 8eb9c64f2082e690dae997481a020a252b5234d1 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:24 +0000 Subject: [PATCH 22/30] tools/qubes-create: fix docstring --- qubes/tools/qubes_create.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qubes/tools/qubes_create.py b/qubes/tools/qubes_create.py index 28059a09..34babd6d 100644 --- a/qubes/tools/qubes_create.py +++ b/qubes/tools/qubes_create.py @@ -18,7 +18,7 @@ # License along with this library; if not, see . # -'''qvm-create - Create new Qubes OS store''' +'''qubes-create - Create new Qubes OS store''' import sys import qubes From 53ef5ed431f47cc5ead3934529a78254c77c6e01 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:25 +0000 Subject: [PATCH 23/30] app: uncouple pool setup from loading initial configuration And ensure that setup is called on every type of these pools, not just lvm_thin. --- qubes/app.py | 23 +++++++++++++++++------ qubes/tests/api_admin.py | 1 + qubes/tools/qubes_create.py | 2 +- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/qubes/app.py b/qubes/app.py index 5deff36f..3609b6b1 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -21,6 +21,7 @@ # import collections +import copy import errno import functools import grp @@ -1064,15 +1065,20 @@ class Qubes(qubes.PropertyHolder): } assert max(self.labels.keys()) == qubes.config.max_default_label + pool_configs = copy.deepcopy(qubes.config.defaults['pool_configs']) + root_volume_group, root_thin_pool = \ qubes.storage.DirectoryThinPool.thin_pool('/') - if root_thin_pool: - self.add_pool( - volume_group=root_volume_group, thin_pool=root_thin_pool, - name='lvm', driver='lvm_thin') - # pool based on /var/lib/qubes will be created here: - for name, config in qubes.config.defaults['pool_configs'].items(): + lvm_config = { + 'name': 'lvm', + 'driver': 'lvm_thin', + 'volume_group': root_volume_group, + 'thin_pool': root_thin_pool + } + pool_configs[lvm_config['name']] = lvm_config + + for name, config in pool_configs.items(): self.pools[name] = self._get_pool(**config) self.default_pool_kernel = 'linux-kernel' @@ -1170,6 +1176,11 @@ class Qubes(qubes.PropertyHolder): raise KeyError(label) + def setup_pools(self): + """ Run implementation specific setup for each storage pool. """ + for pool in self.pools.values(): + pool.setup() + def add_pool(self, name, **kwargs): """ Add a storage pool to config.""" diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index 4a425d4b..9efdd75e 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -60,6 +60,7 @@ class AdminAPITestCase(qubes.tests.QubesTestCase): app = qubes.Qubes('/tmp/qubes-test.xml', load=False) app.vmm = unittest.mock.Mock(spec=qubes.app.VMMConnection) app.load_initial_values() + app.setup_pools() app.default_kernel = '1.0' app.default_netvm = None self.template = app.add_new_vm('TemplateVM', label='black', diff --git a/qubes/tools/qubes_create.py b/qubes/tools/qubes_create.py index 34babd6d..56e66c7f 100644 --- a/qubes/tools/qubes_create.py +++ b/qubes/tools/qubes_create.py @@ -38,7 +38,7 @@ def main(args=None): args = parser.parse_args(args) qubes.Qubes.create_empty_store(args.app, - offline_mode=args.offline_mode) + offline_mode=args.offline_mode).setup_pools() return 0 From 8d1913a8cc47c11ff53abe39c6a782ef48ede1c7 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:26 +0000 Subject: [PATCH 24/30] app: create /var/lib/qubes as file-reflink if supported Use the file-reflink storage driver if /var/lib/qubes is on a filesystem that supports reflinks, e.g. when the btrfs layout was selected in Anaconda. If it doesn't support reflinks (or if detection fails, e.g. in an unprivileged test environment), use 'file' as before. --- qubes/app.py | 12 +++++++++++- qubes/config.py | 3 +-- qubes/tests/storage.py | 6 ++++-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/qubes/app.py b/qubes/app.py index 3609b6b1..7a31e822 100644 --- a/qubes/app.py +++ b/qubes/app.py @@ -61,6 +61,7 @@ import qubes import qubes.ext import qubes.utils import qubes.storage +import qubes.storage.reflink import qubes.vm import qubes.vm.adminvm import qubes.vm.qubesvm @@ -553,7 +554,7 @@ def _default_pool(app): 1. If there is one named 'default', use it. 2. Check if root fs is on LVM thin - use that - 3. Look for file-based pool pointing /var/lib/qubes + 3. Look for file(-reflink)-based pool pointing to /var/lib/qubes 4. Fail ''' if 'default' in app.pools: @@ -1079,6 +1080,15 @@ class Qubes(qubes.PropertyHolder): pool_configs[lvm_config['name']] = lvm_config for name, config in pool_configs.items(): + if 'driver' not in config and 'dir_path' in config: + config['driver'] = 'file' + try: + os.makedirs(config['dir_path'], exist_ok=True) + if qubes.storage.reflink.is_supported(config['dir_path']): + config['driver'] = 'file-reflink' + config['setup_check'] = 'no' # don't check twice + except PermissionError: # looks like a testing environment + pass # stay with 'file' self.pools[name] = self._get_pool(**config) self.default_pool_kernel = 'linux-kernel' diff --git a/qubes/config.py b/qubes/config.py index dfedfe23..da02dbb6 100644 --- a/qubes/config.py +++ b/qubes/config.py @@ -76,9 +76,8 @@ defaults = { 'root_img_size': 10*1024*1024*1024, 'pool_configs': { - # create file pool even when the default one is LVM + # create file(-reflink) pool even when the default one is LVM 'varlibqubes': {'dir_path': qubes_base_dir, - 'driver': 'file', 'name': 'varlibqubes'}, 'linux-kernel': { 'dir_path': os.path.join(qubes_base_dir, diff --git a/qubes/tests/storage.py b/qubes/tests/storage.py index 1af72a08..97d5932f 100644 --- a/qubes/tests/storage.py +++ b/qubes/tests/storage.py @@ -22,6 +22,7 @@ import qubes.storage from qubes.exc import QubesException from qubes.storage import pool_drivers from qubes.storage.file import FilePool +from qubes.storage.reflink import ReflinkPool from qubes.tests import SystemTestCase # :pylint: disable=invalid-name @@ -107,10 +108,11 @@ class TC_00_Pool(SystemTestCase): pool_drivers()) def test_002_get_pool_klass(self): - """ Expect the default pool to be `FilePool` """ + """ Expect the default pool to be `FilePool` or `ReflinkPool` """ # :pylint: disable=protected-access result = self.app.get_pool('varlibqubes') - self.assertIsInstance(result, FilePool) + self.assertTrue(isinstance(result, FilePool) + or isinstance(result, ReflinkPool)) def test_003_pool_exists_default(self): """ Expect the default pool to exists """ From bc30c6f3e8b18788930994fb1ac2888b65dccdb2 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:27 +0000 Subject: [PATCH 25/30] tests: delete orphaned Makefile --- Makefile | 2 -- tests/Makefile | 49 ------------------------------------------------- 2 files changed, 51 deletions(-) delete mode 100644 tests/Makefile diff --git a/Makefile b/Makefile index eb951965..c8787c2e 100644 --- a/Makefile +++ b/Makefile @@ -141,7 +141,6 @@ rpms-dom0: all: $(PYTHON) setup.py build $(MAKE) -C qubes-rpc all -# make all -C tests # Currently supported only on xen install: @@ -158,7 +157,6 @@ endif ln -s qvm-device.1.gz $(DESTDIR)/usr/share/man/man1/qvm-block.1.gz ln -s qvm-device.1.gz $(DESTDIR)/usr/share/man/man1/qvm-pci.1.gz ln -s qvm-device.1.gz $(DESTDIR)/usr/share/man/man1/qvm-usb.1.gz -# $(MAKE) install -C tests $(MAKE) install -C relaxng mkdir -p $(DESTDIR)/etc/qubes ifeq ($(BACKEND_VMM),xen) diff --git a/tests/Makefile b/tests/Makefile deleted file mode 100644 index 8b68f0b2..00000000 --- a/tests/Makefile +++ /dev/null @@ -1,49 +0,0 @@ -PYTHON_TESTSPATH = $(PYTHON_SITEPATH)/qubes/tests - -all: - python -m compileall . - python -O -m compileall . - -install: -ifndef PYTHON_SITEPATH - $(error PYTHON_SITEPATH not defined) -endif - mkdir -p $(DESTDIR)$(PYTHON_TESTSPATH) - cp __init__.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp __init__.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp backup.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp backup.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp backupcompatibility.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp backupcompatibility.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp basic.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp basic.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp block.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp block.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp dispvm.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp dispvm.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp dom0_update.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp dom0_update.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp extra.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp extra.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp hardware.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp hardware.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp hvm.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp hvm.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp mime.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp mime.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp network.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp network.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp pvgrub.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp pvgrub.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp regressions.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp regressions.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp run.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp run.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp storage.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp storage.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp storage_file.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp storage_file.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp storage_xen.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp storage_xen.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) - cp vm_qrexec_gui.py $(DESTDIR)$(PYTHON_TESTSPATH) - cp vm_qrexec_gui.py[co] $(DESTDIR)$(PYTHON_TESTSPATH) From 49e7ce025f1fff4a2054e9c26a42147adb4d0aa1 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:28 +0000 Subject: [PATCH 26/30] tests/integ/backupcompatibility: Storage.verify() is a coro --- qubes/tests/integ/backupcompatibility.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qubes/tests/integ/backupcompatibility.py b/qubes/tests/integ/backupcompatibility.py index c52e0e62..43a2a4e1 100644 --- a/qubes/tests/integ/backupcompatibility.py +++ b/qubes/tests/integ/backupcompatibility.py @@ -19,6 +19,7 @@ from multiprocessing import Queue +import asyncio import os import shutil import subprocess @@ -382,7 +383,7 @@ class TC_00_BackupCompatibility( def assertRestored(self, name, **kwargs): with self.assertNotRaises((KeyError, qubes.exc.QubesException)): vm = self.app.domains[name] - vm.storage.verify() + asyncio.get_event_loop().run_until_complete(vm.storage.verify()) for prop, value in kwargs.items(): if prop == 'klass': self.assertIsInstance(vm, value) From 8c117549ad6591d340e2408a07a3e996949a9523 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:30 +0000 Subject: [PATCH 27/30] tests/integ/basic: use export() in get_rootimg_checksum() volume.path and volume.export() refer to the same thing in lvm_thin and 'file', but not in file-reflink (where volume.path is the -dirty.img, which doesn't exist if the volume is not started). --- qubes/tests/integ/basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qubes/tests/integ/basic.py b/qubes/tests/integ/basic.py index 07cea6cd..2b592182 100644 --- a/qubes/tests/integ/basic.py +++ b/qubes/tests/integ/basic.py @@ -572,7 +572,7 @@ class TC_03_QvmRevertTemplateChanges(qubes.tests.SystemTestCase): def get_rootimg_checksum(self): return subprocess.check_output( - ['sha1sum', self.test_template.volumes['root'].path]).\ + ['sha1sum', self.test_template.volumes['root'].export()]).\ decode().split(' ')[0] def _do_test(self): From b82e739346bab6fbcfb59a9f28e32be809ed1dc1 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:31 +0000 Subject: [PATCH 28/30] tests/integ/storage: add file-reflink integration tests --- qubes/tests/integ/storage.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/qubes/tests/integ/storage.py b/qubes/tests/integ/storage.py index 4d8f1569..253d8e72 100644 --- a/qubes/tests/integ/storage.py +++ b/qubes/tests/integ/storage.py @@ -26,6 +26,7 @@ import subprocess import qubes.storage.lvm import qubes.tests import qubes.tests.storage_lvm +import qubes.tests.storage_reflink import qubes.vm.appvm @@ -318,6 +319,28 @@ class StorageFile(StorageTestMixin, qubes.tests.SystemTestCase): super(StorageFile, self).tearDown() +class StorageReflinkMixin(StorageTestMixin): + def tearDown(self): + self.app.remove_pool(self.pool.name) + super().tearDown() + + def init_pool(self, fs_type, **kwargs): + name = 'test-reflink-integration-on-' + fs_type + dir_path = os.path.join('/var/tmp', name) + qubes.tests.storage_reflink.mkdir_fs(dir_path, fs_type, + cleanup_via=self.addCleanup) + self.pool = self.app.add_pool(name=name, dir_path=dir_path, + driver='file-reflink', **kwargs) + +class StorageReflinkOnBtrfs(StorageReflinkMixin, qubes.tests.SystemTestCase): + def init_pool(self): + super().init_pool('btrfs') + +class StorageReflinkOnExt4(StorageReflinkMixin, qubes.tests.SystemTestCase): + def init_pool(self): + super().init_pool('ext4', setup_check='no') + + @qubes.tests.storage_lvm.skipUnlessLvmPoolExists class StorageLVM(StorageTestMixin, qubes.tests.SystemTestCase): def init_pool(self): From 797bbc43a0676e3ee57e7db3d313f68d10f4085a Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:32 +0000 Subject: [PATCH 29/30] tests/storage_reflink: test some file-reflink helpers Tested: - _copy_file() - _create_sparse_file() - _resize_file() - _update_loopdev_sizes() Smoke tested by calls from the functions above: - _replace_file() - _rename_file() - _make_dir() - _fsync_dir() --- qubes/tests/__init__.py | 1 + qubes/tests/storage_reflink.py | 154 +++++++++++++++++++++++++++++++++ rpm_spec/core-dom0.spec.in | 1 + 3 files changed, 156 insertions(+) create mode 100644 qubes/tests/storage_reflink.py diff --git a/qubes/tests/__init__.py b/qubes/tests/__init__.py index 140e7f43..b13f2cc0 100644 --- a/qubes/tests/__init__.py +++ b/qubes/tests/__init__.py @@ -1223,6 +1223,7 @@ def load_tests(loader, tests, pattern): # pylint: disable=unused-argument 'qubes.tests.vm.init', 'qubes.tests.storage', 'qubes.tests.storage_file', + 'qubes.tests.storage_reflink', 'qubes.tests.storage_lvm', 'qubes.tests.storage_kernels', 'qubes.tests.ext', diff --git a/qubes/tests/storage_reflink.py b/qubes/tests/storage_reflink.py new file mode 100644 index 00000000..fff26144 --- /dev/null +++ b/qubes/tests/storage_reflink.py @@ -0,0 +1,154 @@ +# +# The Qubes OS Project, https://www.qubes-os.org +# +# Copyright (C) 2018 Rusty Bird +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, see . +# + +''' Tests for the file-reflink storage driver ''' + +# pylint: disable=protected-access +# pylint: disable=invalid-name + +import os +import shutil +import subprocess +import sys + +import qubes.tests +from qubes.storage import reflink + + +class ReflinkMixin: + def setUp(self, fs_type='btrfs'): # pylint: disable=arguments-differ + super().setUp() + self.test_dir = '/var/tmp/test-reflink-units-on-' + fs_type + mkdir_fs(self.test_dir, fs_type, cleanup_via=self.addCleanup) + + def test_000_copy_file(self): + source = os.path.join(self.test_dir, 'source-file') + dest = os.path.join(self.test_dir, 'new-directory', 'dest-file') + content = os.urandom(1024**2) + + with open(source, 'wb') as source_io: + source_io.write(content) + + ficlone_succeeded = reflink._copy_file(source, dest) + self.assertEqual(ficlone_succeeded, self.ficlone_supported) + + self.assertNotEqual(os.stat(source).st_ino, os.stat(dest).st_ino) + with open(source, 'rb') as source_io: + self.assertEqual(source_io.read(), content) + with open(dest, 'rb') as dest_io: + self.assertEqual(dest_io.read(), content) + + def test_001_create_and_resize_files_and_update_loopdevs(self): + img_real = os.path.join(self.test_dir, 'img-real') + img_sym = os.path.join(self.test_dir, 'img-sym') + size_initial = 111 * 1024**2 + size_resized = 222 * 1024**2 + + os.symlink(img_real, img_sym) + reflink._create_sparse_file(img_real, size_initial) + self.assertEqual(reflink._get_file_disk_usage(img_real), 0) + self.assertEqual(os.stat(img_real).st_size, size_initial) + + dev_from_real = setup_loopdev(img_real, cleanup_via=self.addCleanup) + dev_from_sym = setup_loopdev(img_sym, cleanup_via=self.addCleanup) + + reflink._resize_file(img_real, size_resized) + self.assertEqual(reflink._get_file_disk_usage(img_real), 0) + self.assertEqual(os.stat(img_real).st_size, size_resized) + + reflink_update_loopdev_sizes(os.path.join(self.test_dir, 'unrelated')) + + for dev in (dev_from_real, dev_from_sym): + self.assertEqual(get_blockdev_size(dev), size_initial) + + reflink_update_loopdev_sizes(img_sym) + + for dev in (dev_from_real, dev_from_sym): + self.assertEqual(get_blockdev_size(dev), size_resized) + +class TC_00_ReflinkOnBtrfs(ReflinkMixin, qubes.tests.QubesTestCase): + def setUp(self): # pylint: disable=arguments-differ + super().setUp('btrfs') + self.ficlone_supported = True + +class TC_01_ReflinkOnExt4(ReflinkMixin, qubes.tests.QubesTestCase): + def setUp(self): # pylint: disable=arguments-differ + super().setUp('ext4') + self.ficlone_supported = False + + +def setup_loopdev(img, cleanup_via=None): + dev = str.strip(cmd('sudo', 'losetup', '-f', '--show', img).decode()) + if cleanup_via is not None: + cleanup_via(detach_loopdev, dev) + return dev + +def detach_loopdev(dev): + cmd('sudo', 'losetup', '-d', dev) + +def get_fs_type(directory): + # 'stat -f -c %T' would identify ext4 as 'ext2/ext3' + return cmd('df', '--output=fstype', directory).decode().splitlines()[1] + +def mkdir_fs(directory, fs_type, + accessible=True, max_size=100*1024**3, cleanup_via=None): + os.mkdir(directory) + + if get_fs_type(directory) != fs_type: + img = os.path.join(directory, 'img') + with open(img, 'xb') as img_io: + img_io.truncate(max_size) + cmd('mkfs.' + fs_type, img) + dev = setup_loopdev(img) + os.remove(img) + cmd('sudo', 'mount', dev, directory) + detach_loopdev(dev) + + if accessible: + cmd('sudo', 'chmod', '777', directory) + else: + cmd('sudo', 'chmod', '000', directory) + cmd('sudo', 'chattr', '+i', directory) # cause EPERM on write as root + + if cleanup_via is not None: + cleanup_via(rmtree_fs, directory) + +def rmtree_fs(directory): + if os.path.ismount(directory): + cmd('sudo', 'umount', '-l', directory) + # loop device and backing file are garbage collected automatically + cmd('sudo', 'chattr', '-i', directory) + cmd('sudo', 'chmod', '777', directory) + shutil.rmtree(directory) + +def get_blockdev_size(dev): + return int(cmd('sudo', 'blockdev', '--getsize64', dev)) + +def reflink_update_loopdev_sizes(img): + env = [k + '=' + v for k, v in os.environ.items() # 'sudo -E' alone would + if k.startswith('PYTHON')] # drop some of these + code = ('from qubes.storage import reflink\n' + 'reflink._update_loopdev_sizes(%r)' % img) + cmd('sudo', '-E', 'env', *env, sys.executable, '-c', code) + +def cmd(*argv): + p = subprocess.run(argv, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + if p.returncode != 0: + raise Exception(str(p)) # this will show stdout and stderr + return p.stdout diff --git a/rpm_spec/core-dom0.spec.in b/rpm_spec/core-dom0.spec.in index 283434cb..5dc6ddfc 100644 --- a/rpm_spec/core-dom0.spec.in +++ b/rpm_spec/core-dom0.spec.in @@ -305,6 +305,7 @@ fi %{python3_sitelib}/qubes/tests/init.py %{python3_sitelib}/qubes/tests/storage.py %{python3_sitelib}/qubes/tests/storage_file.py +%{python3_sitelib}/qubes/tests/storage_reflink.py %{python3_sitelib}/qubes/tests/storage_kernels.py %{python3_sitelib}/qubes/tests/storage_lvm.py %{python3_sitelib}/qubes/tests/tarwriter.py From cf1ea5cee1dfbe718a3d5328f97cff3d951560c3 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Tue, 11 Sep 2018 23:50:33 +0000 Subject: [PATCH 30/30] tests/app: test varlibqubes pool driver selection --- qubes/tests/app.py | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/qubes/tests/app.py b/qubes/tests/app.py index 04ad2171..63b630c9 100644 --- a/qubes/tests/app.py +++ b/qubes/tests/app.py @@ -30,6 +30,7 @@ import qubes.events import qubes.tests import qubes.tests.init +import qubes.tests.storage_reflink class TestApp(qubes.tests.TestEmitter): pass @@ -264,6 +265,44 @@ class TC_30_VMCollection(qubes.tests.QubesTestCase): # pass +class TC_80_QubesInitialPools(qubes.tests.QubesTestCase): + def setUp(self): + super().setUp() + self.app = qubes.Qubes('/tmp/qubestest.xml', load=False, + offline_mode=True) + self.test_dir = '/var/tmp/test-varlibqubes' + self.test_patch = mock.patch.dict( + qubes.config.defaults['pool_configs']['varlibqubes'], + {'dir_path': self.test_dir}) + self.test_patch.start() + + def tearDown(self): + self.test_patch.stop() + self.app.close() + del self.app + + def get_driver(self, fs_type, accessible): + qubes.tests.storage_reflink.mkdir_fs(self.test_dir, fs_type, + accessible=accessible, cleanup_via=self.addCleanup) + self.app.load_initial_values() + + varlibqubes = self.app.pools['varlibqubes'] + self.assertEqual(varlibqubes.dir_path, self.test_dir) + return varlibqubes.driver + + def test_100_varlibqubes_btrfs_accessible(self): + self.assertEqual(self.get_driver('btrfs', True), 'file-reflink') + + def test_101_varlibqubes_btrfs_inaccessible(self): + self.assertEqual(self.get_driver('btrfs', False), 'file') + + def test_102_varlibqubes_ext4_accessible(self): + self.assertEqual(self.get_driver('ext4', True), 'file') + + def test_103_varlibqubes_ext4_inaccessible(self): + self.assertEqual(self.get_driver('ext4', False), 'file') + + class TC_89_QubesEmpty(qubes.tests.QubesTestCase): def tearDown(self): try: