From a748b393f4dcd071ea4d38b32503e9c6bbf8a640 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 26 Jun 2017 11:39:02 +0200 Subject: [PATCH] storage: move remove() to Volume This is continuation of 0f12870 "storage: use direct object references, not only identifiers". QubesOS/qubes-issues#2256 --- qubes/storage/__init__.py | 17 ++++++++--------- qubes/storage/file.py | 20 ++++++++++---------- qubes/storage/kernels.py | 6 +++--- qubes/storage/lvm.py | 24 ++++++++++++------------ qubes/tests/storage_lvm.py | 4 ++-- 5 files changed, 35 insertions(+), 36 deletions(-) diff --git a/qubes/storage/__init__.py b/qubes/storage/__init__.py index b8d4f7be..47507969 100644 --- a/qubes/storage/__init__.py +++ b/qubes/storage/__init__.py @@ -146,6 +146,12 @@ class Volume(object): ''' raise self._not_implemented("create") + def remove(self): + ''' Remove volume. + + This can be implemented as a coroutine.''' + raise self._not_implemented("remove") + def commit(self): ''' Write the snapshot to disk @@ -520,7 +526,7 @@ class Storage(object): for name, volume in self.vm.volumes.items(): self.log.info('Removing volume %s: %s' % (name, volume.vid)) try: - ret = volume.pool.remove(volume) + ret = volume.remove() if asyncio.iscoroutine(ret): futures.append(ret) except (IOError, OSError) as e: @@ -719,12 +725,6 @@ class Pool(object): ''' raise self._not_implemented("init_volume") - def remove(self, volume): - ''' Remove volume. - - This can be implemented as a coroutine.''' - raise self._not_implemented("remove") - def rename(self, volume, old_name, new_name): ''' Called when the domain changes its name ''' raise self._not_implemented("rename") @@ -804,8 +804,7 @@ class VmCreationManager(object): if type is not None and value is not None and tb is not None: for volume in self.vm.volumes.values(): try: - pool = volume.pool - pool.remove(volume) + volume.remove() except Exception: # pylint: disable=broad-except pass os.rmdir(self.vm.dir_path) diff --git a/qubes/storage/file.py b/qubes/storage/file.py index 0241e4e3..eb999c09 100644 --- a/qubes/storage/file.py +++ b/qubes/storage/file.py @@ -79,16 +79,6 @@ class FilePool(qubes.storage.Pool): self._volumes += [volume] return volume - def remove(self, volume): - if not volume.internal: - return # do not remove random attached file volumes - elif volume._is_snapshot: - return # no need to remove, because it's just a snapshot - else: - _remove_if_exists(volume.path) - if volume._is_origin: - _remove_if_exists(volume.path_cow) - def rename(self, volume, old_name, new_name): assert issubclass(volume.__class__, FileVolume) subdir, _, volume_path = volume.vid.split('/', 2) @@ -201,6 +191,16 @@ class FileVolume(qubes.storage.Volume): else: create_sparse_file(self.path, self.size) + def remove(self): + if not self.internal: + return # do not remove random attached file volumes + elif self._is_snapshot: + return # no need to remove, because it's just a snapshot + else: + _remove_if_exists(self.path) + if self._is_origin: + _remove_if_exists(self.path_cow) + def is_dirty(self): return False # TODO: How to implement this? diff --git a/qubes/storage/kernels.py b/qubes/storage/kernels.py index 3637dea5..1f965e92 100644 --- a/qubes/storage/kernels.py +++ b/qubes/storage/kernels.py @@ -93,6 +93,9 @@ class LinuxModules(Volume): def create(self): return self + def remove(self): + pass + def commit(self): return self @@ -154,9 +157,6 @@ class LinuxKernel(Pool): def import_volume(self, dst_pool, dst_volume, src_pool, src_volume): pass - def remove(self, volume): - pass - def rename(self, volume, old_name, new_name): return volume diff --git a/qubes/storage/lvm.py b/qubes/storage/lvm.py index e6784b03..1c5e66c2 100644 --- a/qubes/storage/lvm.py +++ b/qubes/storage/lvm.py @@ -88,18 +88,6 @@ class ThinPool(qubes.storage.Pool): volume_config['pool'] = self return ThinVolume(**volume_config) - def remove(self, volume): - assert volume.vid - if volume.is_dirty(): - cmd = ['remove', volume._vid_snap] - qubes_lvm(cmd, self.log) - - if not os.path.exists(volume.path): - return - cmd = ['remove', volume.vid] - qubes_lvm(cmd, self.log) - reset_cache() - def rename(self, volume, old_name, new_name): ''' Called when the domain changes its name ''' new_vid = "{!s}/vm-{!s}-{!s}".format(self.volume_group, new_name, @@ -296,6 +284,18 @@ class ThinVolume(qubes.storage.Volume): reset_cache() return self + def remove(self): + assert self.vid + if self.is_dirty(): + cmd = ['remove', self._vid_snap] + qubes_lvm(cmd, self.log) + + if not os.path.exists(self.path): + return + cmd = ['remove', self.vid] + qubes_lvm(cmd, self.log) + reset_cache() + def export(self): ''' Returns an object that can be `open()`. ''' devpath = '/dev/' + self.vid diff --git a/qubes/tests/storage_lvm.py b/qubes/tests/storage_lvm.py index 4d1d90ce..1426e932 100644 --- a/qubes/tests/storage_lvm.py +++ b/qubes/tests/storage_lvm.py @@ -137,7 +137,7 @@ class TC_00_ThinPool(ThinPoolBase): volume.create() path = "/dev/%s" % volume.vid self.assertTrue(os.path.exists(path)) - self.pool.remove(volume) + volume.remove() def test_003_read_write_volume(self): ''' Test read-write volume creation ''' @@ -157,7 +157,7 @@ class TC_00_ThinPool(ThinPoolBase): volume.create() path = "/dev/%s" % volume.vid self.assertTrue(os.path.exists(path)) - self.pool.remove(volume) + volume.remove() @skipUnlessLvmPoolExists class TC_01_ThinPool(qubes.tests.SystemTestsMixin, ThinPoolBase):