From 377f331d52f4edd82d13a59b76e71667e9d0b856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Thu, 11 Jan 2018 03:56:30 +0100 Subject: [PATCH 1/4] storage/lvm: fix size reporting just after creating LV Force cache refresh after registering new pool - it might be just created. QubesOS/qubes-issues#3438 --- qubes/storage/lvm.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qubes/storage/lvm.py b/qubes/storage/lvm.py index e814bb06..158dcaad 100644 --- a/qubes/storage/lvm.py +++ b/qubes/storage/lvm.py @@ -97,7 +97,8 @@ class ThinPool(qubes.storage.Pool): return volume def setup(self): - pass # TODO Should we create a non existing pool? + reset_cache() + # TODO Should we create a non existing pool? def get_volume(self, vid): ''' Return a volume with given vid''' From bcf42c13faca9a1133a8380b73fbf935a1534396 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Fri, 12 Jan 2018 05:12:08 +0100 Subject: [PATCH 2/4] storage/lvm: check for LVM LV existence and type when creating ThinPool Check if requested thin pool exists and really is thin pool. QubesOS/qubes-issues#3438 --- qubes/storage/lvm.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/qubes/storage/lvm.py b/qubes/storage/lvm.py index 158dcaad..8e9839de 100644 --- a/qubes/storage/lvm.py +++ b/qubes/storage/lvm.py @@ -98,6 +98,13 @@ class ThinPool(qubes.storage.Pool): def setup(self): reset_cache() + cache_key = self.volume_group + '/' + self.thin_pool + if cache_key not in size_cache: + raise qubes.storage.StoragePoolException( + 'Thin pool {} does not exist'.format(cache_key)) + if size_cache[cache_key]['attr'][0] != 't': + raise qubes.storage.StoragePoolException( + 'Volume {} is not a thin pool'.format(cache_key)) # TODO Should we create a non existing pool? def get_volume(self, vid): From 510fad9163220d121c325f853a201b3df7349549 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 19 Mar 2018 03:29:26 +0100 Subject: [PATCH 3/4] storage/file: import data into temporary volume Similar to LVM changes, this fixes/improves multiple things: - no old data visible in the volume - failed import do not leave broken volume - parially imported data not visible to running VM QubesOS/qubes-issues#3169 --- qubes/storage/file.py | 20 ++++++++++++++++-- qubes/tests/api_admin.py | 2 +- qubes/tests/storage_file.py | 42 +++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/qubes/storage/file.py b/qubes/storage/file.py index 10c1b304..48c0d318 100644 --- a/qubes/storage/file.py +++ b/qubes/storage/file.py @@ -269,9 +269,20 @@ class FileVolume(qubes.storage.Volume): copy_file(src_volume.export(), self.path) return self - def import_data(self): - return self.path + if not self.save_on_stop: + raise qubes.storage.StoragePoolException( + "Can not import into save_on_stop=False volume {!s}".format( + self)) + create_sparse_file(self.path_import, self.size) + return self.path_import + + def import_data_end(self, success): + if success: + os.rename(self.path_import, self.path) + else: + os.unlink(self.path_import) + return self def reset(self): ''' Remove and recreate a volatile volume ''' @@ -321,6 +332,11 @@ class FileVolume(qubes.storage.Volume): img_name = self.vid + '-cow.img' return os.path.join(self.dir_path, img_name) + @property + def path_import(self): + img_name = self.vid + '-import.img' + return os.path.join(self.dir_path, img_name) + def verify(self): ''' Verifies the volume. ''' if not os.path.exists(self.path) and \ diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index 501e4154..cbb0d992 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -1624,7 +1624,7 @@ class TC_00_VMs(AdminAPITestCase): value = self.call_mgmt_func(b'admin.vm.volume.Import', b'test-vm1', b'private') self.assertEqual(value, '{} {}'.format( - 2*2**30, '/tmp/qubes-test-dir/appvms/test-vm1/private.img')) + 2*2**30, '/tmp/qubes-test-dir/appvms/test-vm1/private-import.img')) self.assertFalse(self.app.save.called) def test_511_vm_volume_import_running(self): diff --git a/qubes/tests/storage_file.py b/qubes/tests/storage_file.py index 496c45c5..212415bd 100644 --- a/qubes/tests/storage_file.py +++ b/qubes/tests/storage_file.py @@ -312,6 +312,48 @@ class TC_01_FileVolumes(qubes.tests.QubesTestCase): volume.revisions_to_keep = 2 self.assertEqual(volume.revisions_to_keep, 1) + def test_020_import_data(self): + config = { + 'name': 'root', + 'pool': self.POOL_NAME, + 'save_on_stop': True, + 'rw': True, + 'size': 1024 * 1024, + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.POOL_NAME).init_volume(vm, config) + volume.create() + import_path = volume.import_data() + self.assertNotEqual(volume.path, import_path) + with open(import_path, 'w+') as import_file: + import_file.write('test') + volume.import_data_end(True) + self.assertFalse(os.path.exists(import_path), import_path) + with open(volume.path) as volume_file: + volume_data = volume_file.read().strip('\0') + self.assertEqual(volume_data, 'test') + + def test_021_import_data_fail(self): + config = { + 'name': 'root', + 'pool': self.POOL_NAME, + 'save_on_stop': True, + 'rw': True, + 'size': 1024 * 1024, + } + vm = qubes.tests.storage.TestVM(self) + volume = self.app.get_pool(self.POOL_NAME).init_volume(vm, config) + volume.create() + import_path = volume.import_data() + self.assertNotEqual(volume.path, import_path) + with open(import_path, 'w+') as import_file: + import_file.write('test') + volume.import_data_end(False) + self.assertFalse(os.path.exists(import_path), import_path) + with open(volume.path) as volume_file: + volume_data = volume_file.read().strip('\0') + self.assertNotEqual(volume_data, 'test') + def assertVolumePath(self, vm, dev_name, expected, rw=True): # :pylint: disable=invalid-name volumes = vm.volumes From 5eceff84cb35dd905b9e8e761defee0cd6ed55bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 21 Mar 2018 01:48:20 +0100 Subject: [PATCH 4/4] storage/file: use proper exception instead of assert Return readable message to the user. --- qubes/storage/file.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/qubes/storage/file.py b/qubes/storage/file.py index 48c0d318..b0b4e38b 100644 --- a/qubes/storage/file.py +++ b/qubes/storage/file.py @@ -261,9 +261,10 @@ class FileVolume(qubes.storage.Volume): return self.path def import_volume(self, src_volume): - msg = "Can not import snapshot volume {!s} in to pool {!s} " - msg = msg.format(src_volume, self) - assert not src_volume.snap_on_start, msg + if src_volume.snap_on_start: + raise qubes.storage.StoragePoolException( + "Can not import snapshot volume {!s} in to pool {!s} ".format( + src_volume, self)) if self.save_on_stop: _remove_if_exists(self.path) copy_file(src_volume.export(), self.path)