From 11c7b4bb512023b76cc9a70c987abceea8e0e785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 14 Mar 2018 22:54:00 +0100 Subject: [PATCH 01/10] storage/lvm: improve handling interrupted commit First rename volume to backup revision, regardless of revisions_to_keep, then rename -snap to current volume. And only then remove backup revision (if exceed revisions_to_keep). This way even if commit operation is interrupted, there is still a volume with the data. This requires also adjusting few functions to actually fallback to most recent backup revision if the current volume isn't found - create _vid_current property for this purpose. Also, use -snap volume for clone operation and commit it normally later. This makes it safer to interrupt or even revert. QubesOS/qubes-issues#2256 --- qubes/storage/lvm.py | 195 +++++++++++++++++++++++++------------ qubes/tests/storage_lvm.py | 4 +- 2 files changed, 135 insertions(+), 64 deletions(-) diff --git a/qubes/storage/lvm.py b/qubes/storage/lvm.py index aa7c3c12..83d2c3bb 100644 --- a/qubes/storage/lvm.py +++ b/qubes/storage/lvm.py @@ -20,7 +20,6 @@ ''' Driver for storing vm images in a LVM thin pool ''' import logging -import operator import os import subprocess @@ -44,8 +43,37 @@ def check_lvm_version(): lvm_is_very_old = check_lvm_version() + class ThinPool(qubes.storage.Pool): ''' LVM Thin based pool implementation + + Volumes are stored as LVM thin volumes, in thin pool specified by + *volume_group*/*thin_pool* arguments. LVM volume naming scheme: + + vm-{vm_name}-{volume_name}[-suffix] + + Where suffix can be one of: + "-snap" - snapshot for currently running VM, at VM shutdown will be + either discarded (if save_on_stop=False), or committed + (if save_on_stop=True) + "-{revision_id}" - volume revision - new revision is automatically + created at each VM shutdown, *revisions_to_keep* control how many + old revisions (in addition to the current one) should be stored + "" (no suffix) - the most recent committed volume state; also volatile + volume (snap_on_start=False, save_on_stop=False) + + On VM startup, new volume is created, depending on volume type, + according to the table below: + + snap_on_start, save_on_stop + False, False, - no suffix, fresh empty volume + False, True, - "-snap", snapshot of last committed revision + True , False, - "-snap", snapshot of last committed revision + of source volume (from VM's template) + True, True, - unsupported configuration + + Volume's revision_id format is "{timestamp}-back", where timestamp is in + '%s' format (seconds since unix epoch) ''' # pylint: disable=protected-access size_cache = None @@ -137,14 +165,10 @@ class ThinPool(qubes.storage.Pool): if vid.endswith('-back'): # old revisions continue - config = { - 'pool': self, - 'vid': vid, - 'name': vid, - 'volume_group': self.volume_group, - 'rw': vol_info['attr'][1] == 'w', - } - volumes += [ThinVolume(**config)] + volume = self.get_volume(vid) + if volume in volumes: + continue + volumes.append(volume) return volumes @property @@ -200,6 +224,19 @@ def init_cache(log=logging.getLogger('qubes.storage.lvm')): size_cache = init_cache() + +def _revision_sort_key(revision): + '''Sort key for revisions. Sort them by time + + :returns timestamp + ''' + if isinstance(revision, tuple): + revision = revision[0] + if '-' in revision: + revision = revision.split('-')[0] + return int(revision) + + class ThinVolume(qubes.storage.Volume): ''' Default LVM thin volume implementation ''' # pylint: disable=too-few-public-methods @@ -217,7 +254,19 @@ class ThinVolume(qubes.storage.Volume): @property def path(self): - return '/dev/' + self.vid + return '/dev/' + self._vid_current + + @property + def _vid_current(self): + if self.vid in size_cache: + return self.vid + vol_revisions = self.revisions + if vol_revisions: + last_revision = \ + max(vol_revisions.items(), key=_revision_sort_key)[0] + return self.vid + '-' + last_revision + # detached pool? return expected path + return self.vid @property def revisions(self): @@ -229,7 +278,8 @@ class ThinVolume(qubes.storage.Volume): if not revision_vid.endswith('-back'): continue revision_vid = revision_vid[len(name_prefix):] - seconds = int(revision_vid[:-len('-back')]) + # get revision without suffix + seconds = int(revision_vid.split('-')[0]) iso_date = qubes.storage.isodate(seconds).split('.', 1)[0] revisions[revision_vid] = iso_date return revisions @@ -239,7 +289,7 @@ class ThinVolume(qubes.storage.Volume): try: if self.is_dirty(): return qubes.storage.lvm.size_cache[self._vid_snap]['size'] - return qubes.storage.lvm.size_cache[self.vid]['size'] + return qubes.storage.lvm.size_cache[self._vid_current]['size'] except KeyError: return self._size @@ -273,19 +323,31 @@ class ThinVolume(qubes.storage.Volume): ''' if revisions is None: revisions = sorted(self.revisions.items(), - key=operator.itemgetter(1)) + key=_revision_sort_key) # pylint: disable=invalid-unary-operand-type revisions = revisions[:(-self.revisions_to_keep) or None] revisions = [rev_id for rev_id, _ in revisions] for rev_id in revisions: + # safety check + assert rev_id != self._vid_current try: cmd = ['remove', self.vid + '-' + rev_id] qubes_lvm(cmd, self.log) except qubes.storage.StoragePoolException: pass - def _commit(self): + def _commit(self, vid_to_commit=None, keep=False): + ''' + Commit temporary volume into current one. By default + :py:attr:`_vid_snap` is used (which is created by :py:meth:`start()`), + but can be overriden by *vid_to_commit* argument. + + :param vid_to_commit: LVM volume ID to commit into this one + :param keep: whether to keep or not *vid_to_commit*. + IOW use 'clone' or 'rename' methods. + :return: None + ''' msg = "Trying to commit {!s}, but it has save_on_stop == False" msg = msg.format(self) assert self.save_on_stop, msg @@ -293,39 +355,40 @@ class ThinVolume(qubes.storage.Volume): msg = "Trying to commit {!s}, but it has rw == False" msg = msg.format(self) assert self.rw, msg - assert hasattr(self, '_vid_snap') - - if self.revisions_to_keep > 0: - cmd = ['clone', self.vid, - '{}-{}-back'.format(self.vid, int(time.time()))] - qubes_lvm(cmd, self.log) - reset_cache() - self._remove_revisions() + if vid_to_commit is None: + assert hasattr(self, '_vid_snap') + vid_to_commit = self._vid_snap # TODO: when converting this function to coroutine, this _must_ be # under a lock - # remove old volume only after _successful_ clone of the new one - cmd = ['rename', self.vid, self.vid + '-tmp'] - qubes_lvm(cmd, self.log) - try: - cmd = ['clone', self._vid_snap, self.vid] - qubes_lvm(cmd, self.log) - except: - # restore original volume - cmd = ['rename', self.vid + '-tmp', self.vid] - qubes_lvm(cmd, self.log) - raise - else: - cmd = ['remove', self.vid + '-tmp'] - qubes_lvm(cmd, self.log) + if not os.path.exists('/dev/' + vid_to_commit): + # nothing to commit + return + if self._vid_current == self.vid: + cmd = ['rename', self.vid, + '{}-{}-back'.format(self.vid, int(time.time()))] + qubes_lvm(cmd, self.log) + reset_cache() + + cmd = ['clone' if keep else 'rename', + vid_to_commit, + self.vid] + qubes_lvm(cmd, self.log) + reset_cache() + # make sure the one we've committed right now is properly + # detected as the current one - before removing anything + assert self._vid_current == self.vid + + # and remove old snapshots, if needed + self._remove_revisions() def create(self): assert self.vid assert self.size if self.save_on_stop: if self.source: - cmd = ['clone', str(self.source), self.vid] + cmd = ['clone', self.source.path, self.vid] else: cmd = [ 'create', @@ -349,7 +412,7 @@ class ThinVolume(qubes.storage.Volume): self._remove_revisions(self.revisions.keys()) if not os.path.exists(self.path): return - cmd = ['remove', self.vid] + cmd = ['remove', self.path] qubes_lvm(cmd, self.log) reset_cache() # pylint: disable=protected-access @@ -358,8 +421,8 @@ class ThinVolume(qubes.storage.Volume): def export(self): ''' Returns an object that can be `open()`. ''' # make sure the device node is available - qubes_lvm(['activate', self.vid], self.log) - devpath = '/dev/' + self.vid + qubes_lvm(['activate', self.path], self.log) + devpath = self.path return devpath @asyncio.coroutine @@ -367,28 +430,32 @@ class ThinVolume(qubes.storage.Volume): if not src_volume.save_on_stop: return self + if self.is_dirty(): + raise qubes.storage.StoragePoolException( + 'Cannot import to dirty volume {} -' + ' start and stop a qube to cleanup'.format(self.vid)) # HACK: neat trick to speed up testing if you have same physical thin # pool assigned to two qubes-pools i.e: qubes_dom0 and test-lvm # pylint: disable=line-too-long if isinstance(src_volume.pool, ThinPool) and \ src_volume.pool.thin_pool == self.pool.thin_pool: # NOQA - cmd = ['remove', self.vid] - qubes_lvm(cmd, self.log) - cmd = ['clone', str(src_volume), str(self)] - qubes_lvm(cmd, self.log) + self._commit(src_volume.path, keep=True) else: - if src_volume.size != self.size: - self.resize(src_volume.size) + cmd = ['create', self.pool._pool_id, self._vid_snap.split('/')[1], + str(src_volume.size)] + qubes_lvm(cmd) src_path = src_volume.export() - cmd = ['dd', 'if=' + src_path, 'of=/dev/' + self.vid, + cmd = ['dd', 'if=' + src_path, 'of=/dev/' + self._vid_snap, 'conv=sparse'] p = yield from asyncio.create_subprocess_exec(*cmd) yield from p.wait() if p.returncode != 0: + cmd = ['remove', self._vid_snap] + qubes_lvm(cmd) raise qubes.storage.StoragePoolException( 'Failed to import volume {!r}, dd exit code: {}'.format( src_volume, p.returncode)) - reset_cache() + self._commit() return self @@ -408,20 +475,24 @@ class ThinVolume(qubes.storage.Volume): if self._vid_snap not in size_cache: return False return (size_cache[self._vid_snap]['origin'] != - self.source.vid.split('/')[1]) - + self.source.path.split('/')[-1]) def revert(self, revision=None): + if self.is_dirty(): + raise qubes.storage.StoragePoolException( + 'Cannot revert dirty volume {}, stop the qube first'.format( + self.vid)) if revision is None: revision = \ - max(self.revisions.items(), key=operator.itemgetter(1))[0] - old_path = self.path + '-' + revision + max(self.revisions.items(), key=_revision_sort_key)[0] + old_path = '/dev/' + self.vid + '-' + revision if not os.path.exists(old_path): msg = "Volume {!s} has no {!s}".format(self, old_path) raise qubes.storage.StoragePoolException(msg) - cmd = ['remove', self.vid] - qubes_lvm(cmd, self.log) + if self.vid in size_cache: + cmd = ['remove', self.vid] + qubes_lvm(cmd, self.log) cmd = ['clone', self.vid + '-' + revision, self.vid] qubes_lvm(cmd, self.log) reset_cache() @@ -450,7 +521,7 @@ class ThinVolume(qubes.storage.Volume): cmd = ['extend', self._vid_snap, str(size)] qubes_lvm(cmd, self.log) elif self.save_on_stop or not self.snap_on_start: - cmd = ['extend', self.vid, str(size)] + cmd = ['extend', self._vid_current, str(size)] qubes_lvm(cmd, self.log) reset_cache() @@ -462,9 +533,9 @@ class ThinVolume(qubes.storage.Volume): pass if self.source is None: - cmd = ['clone', self.vid, self._vid_snap] + cmd = ['clone', self._vid_current, self._vid_snap] else: - cmd = ['clone', str(self.source), self._vid_snap] + cmd = ['clone', self.source.path, self._vid_snap] qubes_lvm(cmd, self.log) @@ -483,10 +554,10 @@ class ThinVolume(qubes.storage.Volume): try: if self.save_on_stop: self._commit() - if self.snap_on_start or self.save_on_stop: + if self.snap_on_start and not self.save_on_stop: cmd = ['remove', self._vid_snap] qubes_lvm(cmd, self.log) - else: + elif not self.snap_on_start and not self.save_on_stop: cmd = ['remove', self.vid] qubes_lvm(cmd, self.log) finally: @@ -499,9 +570,9 @@ class ThinVolume(qubes.storage.Volume): # volatile volumes don't need any files return True if self.source is not None: - vid = str(self.source) + vid = self.source.path[len('/dev/'):] else: - vid = self.vid + vid = self._vid_current try: vol_info = size_cache[vid] if vol_info['attr'][4] != 'a': @@ -528,7 +599,7 @@ class ThinVolume(qubes.storage.Volume): def usage(self): # lvm thin usage always returns at least the same usage as # the parent try: - return qubes.storage.lvm.size_cache[self.vid]['usage'] + return qubes.storage.lvm.size_cache[self._vid_current]['usage'] except KeyError: return 0 diff --git a/qubes/tests/storage_lvm.py b/qubes/tests/storage_lvm.py index f7082cad..462d6cee 100644 --- a/qubes/tests/storage_lvm.py +++ b/qubes/tests/storage_lvm.py @@ -138,7 +138,7 @@ class TC_00_ThinPool(ThinPoolBase): self.assertEqual(volume.size, qubes.config.defaults['root_img_size']) volume.create() path = "/dev/%s" % volume.vid - self.assertTrue(os.path.exists(path)) + self.assertTrue(os.path.exists(path), path) volume.remove() def test_003_read_write_volume(self): @@ -158,7 +158,7 @@ class TC_00_ThinPool(ThinPoolBase): self.assertEqual(volume.size, qubes.config.defaults['root_img_size']) volume.create() path = "/dev/%s" % volume.vid - self.assertTrue(os.path.exists(path)) + self.assertTrue(os.path.exists(path), path) volume.remove() def test_004_size(self): From 8cf92642836a9ef6aefba6032e2fc6d6ccd52f91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Thu, 15 Mar 2018 02:14:29 +0100 Subject: [PATCH 02/10] tests: LVM volume naming migration, and new naming in general --- qubes/tests/storage_lvm.py | 342 ++++++++++++++++++++++++++++++++++++- 1 file changed, 341 insertions(+), 1 deletion(-) diff --git a/qubes/tests/storage_lvm.py b/qubes/tests/storage_lvm.py index 462d6cee..bb61907e 100644 --- a/qubes/tests/storage_lvm.py +++ b/qubes/tests/storage_lvm.py @@ -29,10 +29,11 @@ import os import subprocess import tempfile import unittest +import unittest.mock import qubes.tests import qubes.storage -from qubes.storage.lvm import ThinPool, ThinVolume +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'] @@ -240,6 +241,345 @@ class TC_00_ThinPool(ThinPoolBase): self.assertEqual(self._get_size(path), new_size) self.assertEqual(volume.size, new_size) + def _get_lv_uuid(self, lv): + sudo = [] if os.getuid() == 0 else ['sudo'] + lvs_output = subprocess.check_output( + sudo + ['lvs', '--noheadings', '-o', 'lv_uuid', lv]) + return lvs_output.strip() + + def _get_lv_origin_uuid(self, lv): + sudo = [] if os.getuid() == 0 else ['sudo'] + if qubes.storage.lvm.lvm_is_very_old: + # no support for origin_uuid directly + lvs_output = subprocess.check_output( + sudo + ['lvs', '--noheadings', '-o', 'origin', lv]) + lvs_output = subprocess.check_output( + sudo + ['lvs', '--noheadings', '-o', 'lv_uuid', + lv.rsplit('/', 1)[0] + '/' + lvs_output.strip().decode()]) + else: + lvs_output = subprocess.check_output( + sudo + ['lvs', '--noheadings', '-o', 'origin_uuid', lv]) + return lvs_output.strip() + + def test_008_commit(self): + ''' Test volume changes commit''' + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + volume.create() + path_snap = '/dev/' + volume._vid_snap + self.assertFalse(os.path.exists(path_snap), path_snap) + origin_uuid = self._get_lv_uuid(volume.path) + volume.start() + snap_uuid = self._get_lv_uuid(path_snap) + self.assertNotEqual(origin_uuid, snap_uuid) + path = volume.path + self.assertTrue(path.startswith('/dev/' + volume.vid), + '{} does not start with /dev/{}'.format(path, volume.vid)) + self.assertTrue(os.path.exists(path), path) + volume.remove() + + def test_009_interrupted_commit(self): + ''' Test volume changes commit''' + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 2, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + # mock logging, to not interfere with time.time() mock + volume.log = unittest.mock.Mock() + # do not call volume.create(), do it manually to simulate + # interrupted commit + revisions = ['-1521065904-back', '-1521065905-back', '-snap'] + orig_uuids = {} + for rev in revisions: + cmd = ['create', self.pool._pool_id, + volume.vid.split('/')[1] + rev, str(config['size'])] + qubes_lvm(cmd) + orig_uuids[rev] = self._get_lv_uuid(volume.vid + rev) + qubes.storage.lvm.reset_cache() + path_snap = '/dev/' + volume._vid_snap + self.assertTrue(volume.is_dirty()) + self.assertEqual(volume.path, + '/dev/' + volume.vid + revisions[1]) + expected_revisions = { + revisions[0].lstrip('-'): '2018-03-14T22:18:24', + revisions[1].lstrip('-'): '2018-03-14T22:18:25', + } + self.assertEqual(volume.revisions, expected_revisions) + volume.start() + self.assertEqual(volume.revisions, expected_revisions) + snap_uuid = self._get_lv_uuid(path_snap) + self.assertEqual(orig_uuids['-snap'], snap_uuid) + self.assertTrue(volume.is_dirty()) + self.assertEqual(volume.path, + '/dev/' + volume.vid + revisions[1]) + with unittest.mock.patch('time.time') as mock_time: + mock_time.side_effect = [521065906] + volume.stop() + expected_revisions = { + revisions[0].lstrip('-'): '2018-03-14T22:18:24', + revisions[1].lstrip('-'): '2018-03-14T22:18:25', + } + self.assertFalse(volume.is_dirty()) + self.assertEqual(volume.revisions, expected_revisions) + self.assertEqual(volume.path, '/dev/' + volume.vid) + self.assertEqual(snap_uuid, self._get_lv_uuid(volume.path)) + self.assertFalse(os.path.exists(path_snap), path_snap) + + volume.remove() + + def test_010_migration1(self): + '''Start with old revisions, then start interacting using new code''' + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 2, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + # mock logging, to not interfere with time.time() mock + volume.log = unittest.mock.Mock() + # do not call volume.create(), do it manually to have old LV naming + revisions = ['', '-1521065904-back', '-1521065905-back'] + orig_uuids = {} + for rev in revisions: + cmd = ['create', self.pool._pool_id, + volume.vid.split('/')[1] + rev, str(config['size'])] + qubes_lvm(cmd) + orig_uuids[rev] = self._get_lv_uuid(volume.vid + rev) + qubes.storage.lvm.reset_cache() + path_snap = '/dev/' + volume._vid_snap + self.assertFalse(os.path.exists(path_snap), path_snap) + expected_revisions = { + revisions[1].lstrip('-'): '2018-03-14T22:18:24', + revisions[2].lstrip('-'): '2018-03-14T22:18:25', + } + self.assertEqual(volume.revisions, expected_revisions) + self.assertEqual(volume.path, '/dev/' + volume.vid) + + volume.start() + snap_uuid = self._get_lv_uuid(path_snap) + self.assertNotEqual(orig_uuids[''], snap_uuid) + snap_origin_uuid = self._get_lv_origin_uuid(path_snap) + self.assertEqual(orig_uuids[''], snap_origin_uuid) + path = volume.path + self.assertEqual(path, '/dev/' + volume.vid) + self.assertTrue(os.path.exists(path), path) + + with unittest.mock.patch('time.time') as mock_time: + mock_time.side_effect = ('1521065906', '1521065907') + volume.stop() + revisions.extend(['-1521065906-back']) + expected_revisions = { + revisions[2].lstrip('-'): '2018-03-14T22:18:25', + revisions[3].lstrip('-'): '2018-03-14T22:18:26', + } + self.assertEqual(volume.revisions, expected_revisions) + self.assertEqual(volume.path, '/dev/' + volume.vid) + path_snap = '/dev/' + volume._vid_snap + self.assertFalse(os.path.exists(path_snap), path_snap) + self.assertTrue(os.path.exists('/dev/' + volume.vid)) + self.assertEqual(self._get_lv_uuid(volume.path), snap_uuid) + prev_path = '/dev/' + volume.vid + revisions[3] + self.assertEqual(self._get_lv_uuid(prev_path), orig_uuids['']) + + volume.remove() + for rev in revisions: + path = '/dev/' + volume.vid + rev + self.assertFalse(os.path.exists(path), path) + + def test_011_migration2(self): + '''VM started with old code, stopped with new''' + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 1, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + # mock logging, to not interfere with time.time() mock + volume.log = unittest.mock.Mock() + # do not call volume.create(), do it manually to have old LV naming + revisions = ['', '-snap'] + orig_uuids = {} + for rev in revisions: + cmd = ['create', self.pool._pool_id, + volume.vid.split('/')[1] + rev, str(config['size'])] + qubes_lvm(cmd) + orig_uuids[rev] = self._get_lv_uuid(volume.vid + rev) + qubes.storage.lvm.reset_cache() + path_snap = '/dev/' + volume._vid_snap + self.assertTrue(os.path.exists(path_snap), path_snap) + expected_revisions = {} + self.assertEqual(volume.revisions, expected_revisions) + self.assertEqual(volume.path, '/dev/' + volume.vid) + self.assertTrue(volume.is_dirty()) + + path = volume.path + self.assertEqual(path, '/dev/' + volume.vid) + self.assertTrue(os.path.exists(path), path) + + with unittest.mock.patch('time.time') as mock_time: + mock_time.side_effect = ('1521065906', '1521065907') + volume.stop() + revisions.extend(['-1521065906-back']) + expected_revisions = { + revisions[2].lstrip('-'): '2018-03-14T22:18:26', + } + self.assertEqual(volume.revisions, expected_revisions) + self.assertEqual(volume.path, '/dev/' + volume.vid) + path_snap = '/dev/' + volume._vid_snap + self.assertFalse(os.path.exists(path_snap), path_snap) + self.assertTrue(os.path.exists('/dev/' + volume.vid)) + self.assertEqual(self._get_lv_uuid(volume.path), orig_uuids['-snap']) + prev_path = '/dev/' + volume.vid + revisions[2] + self.assertEqual(self._get_lv_uuid(prev_path), orig_uuids['']) + + volume.remove() + for rev in revisions: + path = '/dev/' + volume.vid + rev + self.assertFalse(os.path.exists(path), path) + + def test_012_migration3(self): + '''VM started with old code, started again with new, stopped with new''' + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 1, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + # mock logging, to not interfere with time.time() mock + volume.log = unittest.mock.Mock() + # do not call volume.create(), do it manually to have old LV naming + revisions = ['', '-snap'] + orig_uuids = {} + for rev in revisions: + cmd = ['create', self.pool._pool_id, + volume.vid.split('/')[1] + rev, str(config['size'])] + qubes_lvm(cmd) + orig_uuids[rev] = self._get_lv_uuid(volume.vid + rev) + qubes.storage.lvm.reset_cache() + path_snap = '/dev/' + volume._vid_snap + self.assertTrue(os.path.exists(path_snap), path_snap) + expected_revisions = {} + self.assertEqual(volume.revisions, expected_revisions) + self.assertTrue(volume.path, '/dev/' + volume.vid) + self.assertTrue(volume.is_dirty()) + + volume.start() + self.assertEqual(volume.revisions, expected_revisions) + self.assertEqual(volume.path, '/dev/' + volume.vid) + # -snap LV should be unchanged + self.assertEqual(self._get_lv_uuid(volume._vid_snap), + orig_uuids['-snap']) + + volume.remove() + for rev in revisions: + path = '/dev/' + volume.vid + rev + self.assertFalse(os.path.exists(path), path) + + def test_013_migration4(self): + '''revisions_to_keep=0, VM started with old code, stopped with new''' + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 0, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + # mock logging, to not interfere with time.time() mock + volume.log = unittest.mock.Mock() + # do not call volume.create(), do it manually to have old LV naming + revisions = ['', '-snap'] + orig_uuids = {} + for rev in revisions: + cmd = ['create', self.pool._pool_id, + volume.vid.split('/')[1] + rev, str(config['size'])] + qubes_lvm(cmd) + orig_uuids[rev] = self._get_lv_uuid(volume.vid + rev) + qubes.storage.lvm.reset_cache() + path_snap = '/dev/' + volume._vid_snap + self.assertTrue(os.path.exists(path_snap), path_snap) + expected_revisions = {} + self.assertEqual(volume.revisions, expected_revisions) + self.assertEqual(volume.path, '/dev/' + volume.vid) + self.assertTrue(volume.is_dirty()) + + with unittest.mock.patch('time.time') as mock_time: + mock_time.side_effect = ('1521065906', '1521065907') + volume.stop() + expected_revisions = {} + self.assertEqual(volume.revisions, expected_revisions) + self.assertEqual(volume.path, '/dev/' + volume.vid) + + volume.remove() + for rev in revisions: + path = '/dev/' + volume.vid + rev + self.assertFalse(os.path.exists(path), path) + + def test_014_commit_keep_0(self): + ''' Test volume changes commit, with revisions_to_keep=0''' + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 0, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + # mock logging, to not interfere with time.time() mock + volume.log = unittest.mock.Mock() + volume.create() + self.assertFalse(volume.is_dirty()) + path = volume.path + expected_revisions = {} + self.assertEqual(volume.revisions, expected_revisions) + + volume.start() + self.assertEqual(volume.revisions, expected_revisions) + path_snap = '/dev/' + volume._vid_snap + snap_uuid = self._get_lv_uuid(path_snap) + self.assertTrue(volume.is_dirty()) + self.assertEqual(volume.path, path) + + with unittest.mock.patch('time.time') as mock_time: + mock_time.side_effect = [521065906] + volume.stop() + self.assertFalse(volume.is_dirty()) + self.assertEqual(volume.revisions, {}) + self.assertEqual(volume.path, '/dev/' + volume.vid) + self.assertEqual(snap_uuid, self._get_lv_uuid(volume.path)) + self.assertFalse(os.path.exists(path_snap), path_snap) + + volume.remove() + @skipUnlessLvmPoolExists class TC_01_ThinPool(ThinPoolBase, qubes.tests.SystemTestCase): From aea0de35ad0bfb574b34b5ecb06a24544eb11c4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Thu, 15 Mar 2018 02:59:35 +0100 Subject: [PATCH 03/10] tests: ThinVolume.revert() --- qubes/tests/storage_lvm.py | 71 +++++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/qubes/tests/storage_lvm.py b/qubes/tests/storage_lvm.py index bb61907e..55612feb 100644 --- a/qubes/tests/storage_lvm.py +++ b/qubes/tests/storage_lvm.py @@ -24,7 +24,6 @@ 'volume_group/thin_pool' combination. Pool variables without a prefix represent a :py:class:`qubes.storage.lvm.ThinPool`. ''' - import os import subprocess import tempfile @@ -580,6 +579,76 @@ class TC_00_ThinPool(ThinPoolBase): volume.remove() + def test_020_revert_last(self): + ''' Test volume revert''' + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 2, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + volume.create() + volume.start() + volume.stop() + volume.start() + volume.stop() + self.assertEqual(len(volume.revisions), 2) + revisions = volume.revisions + revision_id = max(revisions.keys()) + current_path = volume.path + current_uuid = self._get_lv_uuid(volume.path) + rev_uuid = self._get_lv_uuid(volume.vid + '-' + revision_id) + self.assertFalse(volume.is_dirty()) + self.assertNotEqual(current_uuid, rev_uuid) + volume.revert() + path_snap = '/dev/' + volume._vid_snap + self.assertFalse(os.path.exists(path_snap), path_snap) + self.assertEqual(current_path, volume.path) + new_uuid = self._get_lv_origin_uuid(volume.path) + self.assertEqual(new_uuid, rev_uuid) + self.assertEqual(volume.revisions, revisions) + + volume.remove() + + def test_021_revert_earlier(self): + ''' Test volume revert''' + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 2, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + volume.create() + volume.start() + volume.stop() + volume.start() + volume.stop() + self.assertEqual(len(volume.revisions), 2) + revisions = volume.revisions + revision_id = min(revisions.keys()) + current_path = volume.path + current_uuid = self._get_lv_uuid(volume.path) + rev_uuid = self._get_lv_uuid(volume.vid + '-' + revision_id) + self.assertFalse(volume.is_dirty()) + self.assertNotEqual(current_uuid, rev_uuid) + volume.revert(revision_id) + path_snap = '/dev/' + volume._vid_snap + self.assertFalse(os.path.exists(path_snap), path_snap) + self.assertEqual(current_path, volume.path) + new_uuid = self._get_lv_origin_uuid(volume.path) + self.assertEqual(new_uuid, rev_uuid) + self.assertEqual(volume.revisions, revisions) + + volume.remove() + @skipUnlessLvmPoolExists class TC_01_ThinPool(ThinPoolBase, qubes.tests.SystemTestCase): From 2b80f0c044aae2a393feef496f77f3a61c297fd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Thu, 15 Mar 2018 22:11:05 +0100 Subject: [PATCH 04/10] storage/lvm: use temporary volume for data import Do not write directly to main volume, instead create temporary volume and only commit it to the main one when operation is finished. This solve multiple problems: - import operation can be aborted, without data loss - importing new data over existing volume will not leave traces of previous content - especially when importing smaller volume to bigger one - import operation can be reverted - it create separate revision, similar to start/stop - easier to prevent qube from starting during import operation - template still can be used when importing new version QubesOS/qubes-issues#2256 --- qubes/storage/lvm.py | 74 ++++++++++++++++++++++++++++++++------ qubes/tests/storage_lvm.py | 59 ++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 11 deletions(-) diff --git a/qubes/storage/lvm.py b/qubes/storage/lvm.py index 83d2c3bb..01953647 100644 --- a/qubes/storage/lvm.py +++ b/qubes/storage/lvm.py @@ -159,7 +159,7 @@ class ThinPool(qubes.storage.Pool): continue if vol_info['pool_lv'] != self.thin_pool: continue - if vid.endswith('-snap'): + if vid.endswith('-snap') or vid.endswith('-import'): # implementation detail volume continue if vid.endswith('-back'): @@ -249,6 +249,8 @@ class ThinVolume(qubes.storage.Volume): if self.snap_on_start or self.save_on_stop: self._vid_snap = self.vid + '-snap' + if self.save_on_stop: + self._vid_import = self.vid + '-import' self._size = size @@ -409,6 +411,13 @@ class ThinVolume(qubes.storage.Volume): except AttributeError: pass + try: + if os.path.exists('/dev/' + self._vid_import): + cmd = ['remove', self._vid_import] + qubes_lvm(cmd, self.log) + except AttributeError: + pass + self._remove_revisions(self.revisions.keys()) if not os.path.exists(self.path): return @@ -434,36 +443,74 @@ class ThinVolume(qubes.storage.Volume): raise qubes.storage.StoragePoolException( 'Cannot import to dirty volume {} -' ' start and stop a qube to cleanup'.format(self.vid)) + self.abort_if_import_in_progress() # HACK: neat trick to speed up testing if you have same physical thin # pool assigned to two qubes-pools i.e: qubes_dom0 and test-lvm # pylint: disable=line-too-long if isinstance(src_volume.pool, ThinPool) and \ src_volume.pool.thin_pool == self.pool.thin_pool: # NOQA - self._commit(src_volume.path, keep=True) + self._commit(src_volume.path[len('/dev/'):], keep=True) else: - cmd = ['create', self.pool._pool_id, self._vid_snap.split('/')[1], + cmd = ['create', + self.pool._pool_id, # pylint: disable=protected-access + self._vid_import.split('/')[1], str(src_volume.size)] - qubes_lvm(cmd) + qubes_lvm(cmd, self.log) src_path = src_volume.export() - cmd = ['dd', 'if=' + src_path, 'of=/dev/' + self._vid_snap, - 'conv=sparse'] + cmd = ['dd', 'if=' + src_path, 'of=/dev/' + self._vid_import, + 'conv=sparse', 'status=none'] + 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() if p.returncode != 0: - cmd = ['remove', self._vid_snap] - qubes_lvm(cmd) + cmd = ['remove', self._vid_import] + qubes_lvm(cmd, self.log) raise qubes.storage.StoragePoolException( 'Failed to import volume {!r}, dd exit code: {}'.format( src_volume, p.returncode)) - self._commit() + self._commit(self._vid_import) return self def import_data(self): ''' Returns an object that can be `open()`. ''' - devpath = '/dev/' + self.vid + if self.is_dirty(): + raise qubes.storage.StoragePoolException( + 'Cannot import data to dirty volume {}, stop the qube first'. + format(self.vid)) + self.abort_if_import_in_progress() + # pylint: disable=protected-access + cmd = ['create', self.pool._pool_id, self._vid_import.split('/')[1], + str(self.size)] + qubes_lvm(cmd, self.log) + reset_cache() + devpath = '/dev/' + self._vid_import return devpath + def import_data_end(self, success): + '''Either commit imported data, or discard temporary volume''' + if not os.path.exists('/dev/' + self._vid_import): + raise qubes.storage.StoragePoolException( + 'No import operation in progress on {}'.format(self.vid)) + if success: + self._commit(self._vid_import) + else: + cmd = ['remove', self._vid_import] + qubes_lvm(cmd, self.log) + + def abort_if_import_in_progress(self): + try: + devpath = '/dev/' + self._vid_import + if os.path.exists(devpath): + raise qubes.storage.StoragePoolException( + 'Import operation in progress on {}'.format(self.vid)) + except AttributeError: # self._vid_import + # no vid_import - import definitely not in progress + pass + def is_dirty(self): if self.save_on_stop: return os.path.exists('/dev/' + self._vid_snap) @@ -482,6 +529,7 @@ class ThinVolume(qubes.storage.Volume): raise qubes.storage.StoragePoolException( 'Cannot revert dirty volume {}, stop the qube first'.format( self.vid)) + self.abort_if_import_in_progress() if revision is None: revision = \ max(self.revisions.items(), key=_revision_sort_key)[0] @@ -520,6 +568,10 @@ class ThinVolume(qubes.storage.Volume): if self.is_dirty(): cmd = ['extend', self._vid_snap, str(size)] qubes_lvm(cmd, self.log) + elif hasattr(self, '_vid_import') and \ + os.path.exists('/dev/' + self._vid_import): + cmd = ['extend', self._vid_import, str(size)] + qubes_lvm(cmd, self.log) elif self.save_on_stop or not self.snap_on_start: cmd = ['extend', self._vid_current, str(size)] qubes_lvm(cmd, self.log) @@ -538,8 +590,8 @@ class ThinVolume(qubes.storage.Volume): cmd = ['clone', self.source.path, self._vid_snap] qubes_lvm(cmd, self.log) - def start(self): + self.abort_if_import_in_progress() try: if self.snap_on_start or self.save_on_stop: if not self.save_on_stop or not self.is_dirty(): diff --git a/qubes/tests/storage_lvm.py b/qubes/tests/storage_lvm.py index 55612feb..a693670e 100644 --- a/qubes/tests/storage_lvm.py +++ b/qubes/tests/storage_lvm.py @@ -649,6 +649,65 @@ class TC_00_ThinPool(ThinPoolBase): volume.remove() + def test_030_import_data(self): + ''' Test volume import''' + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 2, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + volume.create() + current_uuid = self._get_lv_uuid(volume.path) + self.assertFalse(volume.is_dirty()) + import_path = volume.import_data() + import_uuid = self._get_lv_uuid(import_path) + self.assertNotEqual(current_uuid, import_uuid) + # success - commit data + volume.import_data_end(True) + new_current_uuid = self._get_lv_uuid(volume.path) + self.assertEqual(new_current_uuid, import_uuid) + revisions = volume.revisions + self.assertEqual(len(revisions), 1) + revision = revisions.popitem()[0] + self.assertEqual(current_uuid, + self._get_lv_uuid(volume.vid + '-' + revision)) + self.assertFalse(os.path.exists(import_path), import_path) + + volume.remove() + + def test_031_import_data_fail(self): + ''' Test volume import''' + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 2, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + volume.create() + current_uuid = self._get_lv_uuid(volume.path) + self.assertFalse(volume.is_dirty()) + import_path = volume.import_data() + import_uuid = self._get_lv_uuid(import_path) + self.assertNotEqual(current_uuid, import_uuid) + # fail - discard data + volume.import_data_end(False) + new_current_uuid = self._get_lv_uuid(volume.path) + self.assertEqual(new_current_uuid, current_uuid) + revisions = volume.revisions + self.assertEqual(len(revisions), 0) + self.assertFalse(os.path.exists(import_path), import_path) + + volume.remove() + @skipUnlessLvmPoolExists class TC_01_ThinPool(ThinPoolBase, qubes.tests.SystemTestCase): From 76c872a43aa688eed1a266e843e702325e2b6b2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sat, 17 Mar 2018 01:10:43 +0100 Subject: [PATCH 05/10] tests: collect all SIGCHLD before cleaning event loop On python 3.6.4 apparently it requires two callbacks runs to cleanup stale SIGCHLD handlers. --- qubes/tests/__init__.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/qubes/tests/__init__.py b/qubes/tests/__init__.py index 865534e2..7965c7fe 100644 --- a/qubes/tests/__init__.py +++ b/qubes/tests/__init__.py @@ -421,6 +421,13 @@ class QubesTestCase(unittest.TestCase): except asyncio.TimeoutError: raise AssertionError('libvirt event impl drain timeout') + # this is stupid, but apparently it requires two passes + # to cleanup SIGCHLD handlers + self.loop.stop() + self.loop.run_forever() + self.loop.stop() + self.loop.run_forever() + # Check there are no Tasks left. assert not self.loop._ready assert not self.loop._scheduled From 4282a41fcb8bb770ac47ea32f093350b14c63554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sat, 17 Mar 2018 01:12:33 +0100 Subject: [PATCH 06/10] tests: LVM: import, list_volumes, volatile volume, snapshot volume --- qubes/tests/storage_lvm.py | 223 +++++++++++++++++++++++++++++++++++++ 1 file changed, 223 insertions(+) diff --git a/qubes/tests/storage_lvm.py b/qubes/tests/storage_lvm.py index a693670e..b9f5ccba 100644 --- a/qubes/tests/storage_lvm.py +++ b/qubes/tests/storage_lvm.py @@ -708,6 +708,229 @@ class TC_00_ThinPool(ThinPoolBase): volume.remove() + def test_032_import_volume_same_pool(self): + '''Import volume from the same pool''' + # source volume + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 2, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + source_volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + source_volume.create() + + source_uuid = self._get_lv_uuid(source_volume.path) + + # destination volume + config = { + 'name': 'root2', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 2, + 'size': qubes.config.defaults['root_img_size'], + } + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + volume.log = unittest.mock.Mock() + with unittest.mock.patch('time.time') as mock_time: + mock_time.side_effect = [1521065905] + volume.create() + + self.assertEqual(volume.revisions, {}) + uuid_before = self._get_lv_uuid(volume.path) + + with unittest.mock.patch('time.time') as mock_time: + mock_time.side_effect = [1521065906] + self.loop.run_until_complete( + volume.import_volume(source_volume)) + + uuid_after = self._get_lv_uuid(volume.path) + self.assertNotEqual(uuid_after, uuid_before) + + # also should be different than source volume (clone, not the same LV) + self.assertNotEqual(uuid_after, source_uuid) + self.assertEqual(self._get_lv_origin_uuid(volume.path), source_uuid) + + expected_revisions = { + '1521065906-back': '2018-03-14T22:18:26', + } + self.assertEqual(volume.revisions, expected_revisions) + + volume.remove() + source_volume.remove() + + def test_033_import_volume_different_pool(self): + '''Import volume from a different pool''' + source_volume = unittest.mock.Mock() + # destination volume + config = { + 'name': 'root2', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 2, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + volume.log = unittest.mock.Mock() + with unittest.mock.patch('time.time') as mock_time: + mock_time.side_effect = [1521065905] + volume.create() + + self.assertEqual(volume.revisions, {}) + uuid_before = self._get_lv_uuid(volume.path) + + with tempfile.NamedTemporaryFile() as source_volume_file: + source_volume_file.write(b'test-content') + source_volume_file.flush() + source_volume.size = 16 * 1024 * 1024 # 16MiB + source_volume.export.return_value = source_volume_file.name + with unittest.mock.patch('time.time') as mock_time: + mock_time.side_effect = [1521065906] + self.loop.run_until_complete( + volume.import_volume(source_volume)) + + uuid_after = self._get_lv_uuid(volume.path) + self.assertNotEqual(uuid_after, uuid_before) + self.assertEqual(volume.size, 16 * 1024 * 1024) + + volume_content = subprocess.check_output(['sudo', 'cat', volume.path]) + self.assertEqual(volume_content.rstrip(b'\0'), b'test-content') + + expected_revisions = { + '1521065906-back': '2018-03-14T22:18:26', + } + self.assertEqual(volume.revisions, expected_revisions) + + volume.remove() + + def test_040_volatile(self): + '''Volatile volume test''' + config = { + 'name': 'volatile', + 'pool': self.pool.name, + 'rw': True, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.pool.name).init_volume(vm, config) + # volatile volume don't need any file, verify should succeed + self.assertTrue(volume.verify()) + volume.create() + self.assertTrue(volume.verify()) + self.assertFalse(volume.save_on_stop) + self.assertFalse(volume.snap_on_start) + path = volume.path + self.assertEqual(path, '/dev/' + volume.vid) + self.assertFalse(os.path.exists(path)) + volume.start() + self.assertTrue(os.path.exists(path)) + vol_uuid = self._get_lv_uuid(path) + volume.start() + self.assertTrue(os.path.exists(path)) + vol_uuid2 = self._get_lv_uuid(path) + self.assertNotEqual(vol_uuid, vol_uuid2) + volume.stop() + self.assertFalse(os.path.exists(path)) + + def test_050_snapshot_volume(self): + ''' Test snapshot volume creation ''' + config_origin = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'size': qubes.config.defaults['root_img_size'], + } + vm = qubes.tests.storage.TestVM(self) + volume_origin = self.app.get_pool(self.pool.name).init_volume( + vm, config_origin) + volume_origin.create() + config_snapshot = { + 'name': 'root2', + 'pool': self.pool.name, + 'snap_on_start': True, + 'source': volume_origin, + 'rw': True, + 'size': qubes.config.defaults['root_img_size'], + } + volume = self.app.get_pool(self.pool.name).init_volume( + vm, config_snapshot) + self.assertIsInstance(volume, ThinVolume) + self.assertEqual(volume.name, 'root2') + self.assertEqual(volume.pool, self.pool.name) + self.assertEqual(volume.size, qubes.config.defaults['root_img_size']) + # only origin volume really needs to exist, verify should succeed + # even before create + self.assertTrue(volume.verify()) + volume.create() + path = volume.path + self.assertEqual(path, '/dev/' + volume.vid) + self.assertFalse(os.path.exists(path), path) + volume.start() + # snapshot volume isn't considered dirty at any time + self.assertFalse(volume.is_dirty()) + # not outdated yet + self.assertFalse(volume.is_outdated()) + origin_uuid = self._get_lv_uuid(volume_origin.path) + snap_origin_uuid = self._get_lv_origin_uuid(volume._vid_snap) + self.assertEqual(origin_uuid, snap_origin_uuid) + + # now make it outdated + volume_origin.start() + volume_origin.stop() + self.assertTrue(volume.is_outdated()) + origin_uuid = self._get_lv_uuid(volume_origin.path) + self.assertNotEqual(origin_uuid, snap_origin_uuid) + + volume.stop() + # stopped volume is never outdated + self.assertFalse(volume.is_outdated()) + path = volume.path + self.assertFalse(os.path.exists(path), path) + path = '/dev/' + volume._vid_snap + self.assertFalse(os.path.exists(path), path) + + volume.remove() + volume_origin.remove() + + def test_100_pool_list_volumes(self): + config = { + 'name': 'root', + 'pool': self.pool.name, + 'save_on_stop': True, + 'rw': True, + 'revisions_to_keep': 2, + 'size': qubes.config.defaults['root_img_size'], + } + config2 = config.copy() + vm = qubes.tests.storage.TestVM(self) + volume1 = self.app.get_pool(self.pool.name).init_volume(vm, config) + volume1.create() + config2['name'] = 'private' + volume2 = self.app.get_pool(self.pool.name).init_volume(vm, config2) + volume2.create() + + # create some revisions + volume1.start() + volume1.stop() + + # and have one in dirty state + volume2.start() + + self.assertIn(volume1, list(self.pool.volumes)) + self.assertIn(volume2, list(self.pool.volumes)) + volume1.remove() + self.assertNotIn(volume1, list(self.pool.volumes)) + self.assertIn(volume2, list(self.pool.volumes)) + volume2.remove() + self.assertNotIn(volume1, list(self.pool.volumes)) + self.assertNotIn(volume1, list(self.pool.volumes)) @skipUnlessLvmPoolExists class TC_01_ThinPool(ThinPoolBase, qubes.tests.SystemTestCase): From d211a2771a6a50215ad0ad3185778086b8969960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Thu, 22 Mar 2018 01:33:51 +0100 Subject: [PATCH 07/10] api/admin: expose volume path in admin.vm.volume.Info Since (for LVM at least) path is dynamic now, add information about it to volume info. This is not very useful outside of dom0, but in dom0 it can be very useful for various scripts. This will disclose current volume revision id, but it is already possible to deduce it from snapshots list. --- qubes/api/admin.py | 2 +- qubes/tests/api_admin.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qubes/api/admin.py b/qubes/api/admin.py index a4a803c5..d0b9e32d 100644 --- a/qubes/api/admin.py +++ b/qubes/api/admin.py @@ -335,7 +335,7 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI): volume = self.dest.volumes[self.arg] # properties defined in API volume_properties = [ - 'pool', 'vid', 'size', 'usage', 'rw', 'source', + 'pool', 'vid', 'size', 'usage', 'rw', 'source', 'path', 'save_on_stop', 'snap_on_start', 'revisions_to_keep'] def _serialize(value): diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index 3d785ac4..4a425d4b 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -39,7 +39,7 @@ import qubes.storage # properties defined in API volume_properties = [ - 'pool', 'vid', 'size', 'usage', 'rw', 'source', + 'pool', 'vid', 'size', 'usage', 'rw', 'source', 'path', 'save_on_stop', 'snap_on_start', 'revisions_to_keep'] From e644378f18f028a2216f2cc2d4f877ac346acc68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sat, 14 Apr 2018 21:33:38 +0200 Subject: [PATCH 08/10] tests: adjust for variable volume path LVM volumes now have variable volume path. Compare strip path before comparing content's hash in tests. --- qubes/tests/integ/basic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qubes/tests/integ/basic.py b/qubes/tests/integ/basic.py index 5b0eb089..b8799a0f 100644 --- a/qubes/tests/integ/basic.py +++ b/qubes/tests/integ/basic.py @@ -570,7 +570,8 @@ 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'].path]).\ + decode().split(' ')[0] def _do_test(self): checksum_before = self.get_rootimg_checksum() From 2af1815ab784ad30a033df97484f58f0dbd919f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 15 Jul 2018 21:30:04 +0200 Subject: [PATCH 09/10] storage/lvm: add repr(ThinPool) for more meaningful test reports --- qubes/storage/lvm.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/qubes/storage/lvm.py b/qubes/storage/lvm.py index 01953647..18756acf 100644 --- a/qubes/storage/lvm.py +++ b/qubes/storage/lvm.py @@ -90,6 +90,12 @@ class ThinPool(qubes.storage.Pool): self._volume_objects_cache = {} + def __repr__(self): + return '<{} at {:#x} name={!r} volume_group={!r} thin_pool={!r}>'.\ + format( + type(self).__name__, id(self), + self.name, self.volume_group, self.thin_pool) + @property def config(self): return { From 69e3018b94288a965c928558f149504de057775d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 15 Jul 2018 21:31:48 +0200 Subject: [PATCH 10/10] tests: fix handling app.pools iteration --- qubes/tests/storage_lvm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qubes/tests/storage_lvm.py b/qubes/tests/storage_lvm.py index b9f5ccba..f6b67d5b 100644 --- a/qubes/tests/storage_lvm.py +++ b/qubes/tests/storage_lvm.py @@ -84,7 +84,7 @@ class ThinPoolBase(qubes.tests.QubesTestCase): ''' Returns the pool matching the specified ``volume_group`` & ``thin_pool``, or None. ''' - pools = [p for p in self.app.pools + pools = [p for p in self.app.pools.values() if issubclass(p.__class__, ThinPool)] for pool in pools: if pool.volume_group == volume_group \