From b5dd80065cf8a5ca098f5bb53c9365b92b31b9e6 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Fri, 1 Apr 2022 14:06:25 -0400 Subject: [PATCH 1/3] lvm2 pool: Export immutable snapshot of volume Volumes returned by `export()` must be immutable, since otherwise the backup will be inconsistent. Ensure this by exporting a snapshot of the volume, no the volume itself. Fixes QubesOS/qubes-issues#7198. --- qubes/storage/lvm.py | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/qubes/storage/lvm.py b/qubes/storage/lvm.py index 023800cb3..4ac103ced 100644 --- a/qubes/storage/lvm.py +++ b/qubes/storage/lvm.py @@ -156,7 +156,8 @@ def list_volumes(self): continue if vol_info['pool_lv'] != self.thin_pool: continue - if vid.endswith('-snap') or vid.endswith('-import'): + if vid.endswith('-snap') or vid.endswith('-import') or \ + vid.endswith('+export'): # implementation detail volume continue if vid.endswith('-back'): @@ -292,6 +293,7 @@ def __init__(self, volume_group, **kwargs): self._vid_snap = self.vid + '-snap' if self.save_on_stop: self._vid_import = self.vid + '-import' + self._is_exporting = False @property def path(self): @@ -470,13 +472,31 @@ async def remove(self): # pylint: disable=protected-access self.pool._volume_objects_cache.pop(self.vid, None) - async def export(self): + @property + def _vid_export(self) -> str: + return self.vid + '+export' + + @property + def _path_export(self) -> str: + return '/dev/' + self._vid_export + + async def export(self) -> str: ''' Returns an object that can be `open()`. ''' # make sure the device node is available - cmd = ['activate', self.path] - await qubes_lvm_coro(cmd, self.log) - devpath = self.path - return devpath + vid_export = self._vid_export + export_path = self._path_export + if not os.path.exists(export_path): + cmd = ['clone', self._vid_current, vid_export] + await qubes_lvm_coro(cmd, self.log) + self._is_exporting = True + return export_path + + async def export_end(self, path: str) -> None: + assert path == self._path_export, \ + f'Refusing to remove incorrect path {path!r} ' \ + f'(expected {self._path_export!r})' + await self._remove_if_exists(self._vid_export) + self._is_exporting = False @qubes.storage.Volume.locked async def import_volume(self, src_volume): @@ -681,6 +701,8 @@ async def stop(self): changed = await self._remove_if_exists(self._vid_snap) else: changed = await self._remove_if_exists(self.vid) + if not self._is_exporting: + await self._remove_if_exists(self._vid_export) finally: if changed: await reset_cache_coro() From 3f327c659f2eea7e2f1b1861346d2f316fab7e54 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Thu, 7 Apr 2022 21:23:32 -0400 Subject: [PATCH 2/3] Remove a redundant cache invalidation There is no need to invalidate a cache if nothing will access it before it is invalidated again. --- qubes/storage/lvm.py | 1 - 1 file changed, 1 deletion(-) diff --git a/qubes/storage/lvm.py b/qubes/storage/lvm.py index 4ac103ced..afe4b9c6c 100644 --- a/qubes/storage/lvm.py +++ b/qubes/storage/lvm.py @@ -411,7 +411,6 @@ async def _commit(self, vid_to_commit=None, keep=False): cmd = ['rename', self.vid, '{}-{}-back'.format(self.vid, int(time.time()))] await qubes_lvm_coro(cmd, self.log) - await reset_cache_coro() cmd = ['clone' if keep else 'rename', vid_to_commit, From 5eb6240c95f32f9e1b11a49ddab1f602b62e588c Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Fri, 1 Apr 2022 14:17:30 -0400 Subject: [PATCH 3/3] Remove the 'activate' command It isn't used anymore. --- qubes/storage/lvm.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/qubes/storage/lvm.py b/qubes/storage/lvm.py index afe4b9c6c..9af9e6a92 100644 --- a/qubes/storage/lvm.py +++ b/qubes/storage/lvm.py @@ -788,9 +788,6 @@ def _get_lvm_cmdline(cmd): elif action == 'extend': assert len(cmd) == 3, 'wrong number of arguments for extend' lvm_cmd = ["lvextend", "--size=" + cmd[2] + 'B', '--', cmd[1]] - elif action == 'activate': - assert len(cmd) == 2, 'wrong number of arguments for activate' - lvm_cmd = ['lvchange', '--activate=y', '--', cmd[1]] elif action == 'rename': assert len(cmd) == 3, 'wrong number of arguments for rename' lvm_cmd = ['lvrename', '--', cmd[1], cmd[2]]