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

@@ -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
+                    volume.remove()
-                    pool.remove(volume)
                 except Exception:  # pylint: disable=broad-except
                     pass
             os.rmdir(self.vm.dir_path)

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?
 

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
 

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

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):