From c31d317c6349ae6f2a54f4ce8cad80cc1517dcfc Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 11 Mar 2018 15:34:55 +0000 Subject: [PATCH 1/5] storage/reflink: fsync() after resizing existing file Ensure that the updated metadata is written to disk. --- qubes/storage/reflink.py | 1 + 1 file changed, 1 insertion(+) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 7cb9ca10..cc3a5db1 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -378,6 +378,7 @@ def _resize_file(path, size): ''' Resize an existing file. ''' with open(path, 'rb+') as file: file.truncate(size) + os.fsync(file.fileno()) def _create_sparse_file(path, size): ''' Create an empty sparse file. ''' From 023cb4929398104942e8cc9d40d213a7544e55bf Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 11 Mar 2018 15:34:56 +0000 Subject: [PATCH 2/5] storage/reflink: show size in refused volume shrink message Like e6bb282 did for lvm. --- qubes/storage/reflink.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index cc3a5db1..90174f00 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -223,9 +223,9 @@ class ReflinkVolume(qubes.storage.Volume): if size < self.size: raise qubes.storage.StoragePoolException( - 'For your own safety, shrinking of {!s} is disabled.' - ' If you really know what you are doing,' - ' use "truncate" manually.'.format(self.vid)) + 'For your own safety, shrinking of {!s} is disabled' + ' ({:d} < {:d}). If you really know what you are doing,' + ' use "truncate" manually.'.format(self.vid, size, self.size)) try: # assume volume is not (cleanly) stopped ... _resize_file(self._path_dirty, size) From c382eb375239073f24c9e0169503ddad3723c3d1 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 11 Mar 2018 15:34:58 +0000 Subject: [PATCH 3/5] storage/reflink: let _remove_empty_dir() ignore ENOTEMPTY --- qubes/storage/reflink.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 90174f00..4e30b9d2 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -140,12 +140,7 @@ class ReflinkVolume(qubes.storage.Volume): self._prune_revisions(keep=0) _remove_file(self._path_clean) _remove_file(self._path_dirty) - - try: - _remove_empty_dir(os.path.dirname(self._path_dirty)) - except OSError as ex: - if ex.errno is not errno.ENOTEMPTY: - raise + _remove_empty_dir(os.path.dirname(self._path_dirty)) return self @@ -360,10 +355,13 @@ def _remove_file(path): LOGGER.info('Removed file: %s', path) def _remove_empty_dir(path): - with suppress(FileNotFoundError): + try: os.rmdir(path) _fsync_dir(os.path.dirname(path)) LOGGER.info('Removed empty directory: %s', path) + except OSError as ex: + if ex.errno not in (errno.ENOENT, errno.ENOTEMPTY): + raise def _rename_file(src, dst): os.rename(src, dst) From 31810db977b1e881adfcca723dfce1c3e7358b72 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Sun, 11 Mar 2018 15:35:00 +0000 Subject: [PATCH 4/5] storage/reflink: simplify --- qubes/storage/reflink.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 4e30b9d2..7660dedd 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -117,8 +117,7 @@ class ReflinkVolume(qubes.storage.Volume): def verify(self): if self.snap_on_start: - # pylint: disable=protected-access - img = self.source._path_clean + img = self.source._path_clean # pylint: disable=protected-access elif self.save_on_stop: img = self._path_clean else: @@ -133,15 +132,11 @@ class ReflinkVolume(qubes.storage.Volume): ''' Drop volume object from pool; remove volume images from oldest to newest; remove empty VM directory. ''' - with suppress(KeyError): - # pylint: disable=protected-access - del self.pool._volumes[self] - + self.pool._volumes.pop(self, None) # pylint: disable=protected-access self._prune_revisions(keep=0) _remove_file(self._path_clean) _remove_file(self._path_dirty) _remove_empty_dir(os.path.dirname(self._path_dirty)) - return self def is_outdated(self): @@ -172,8 +167,7 @@ class ReflinkVolume(qubes.storage.Volume): self._commit() else: _remove_file(self._path_dirty) - if self.snap_on_start: - _remove_file(self._path_clean) + _remove_file(self._path_clean) return self def _commit(self): From 1743c76ca9271a280b5c7682f2b84c4ffd7d4b9e Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Mon, 12 Mar 2018 16:38:56 +0000 Subject: [PATCH 5/5] storage/reflink: reorder start() to be more readable This also makes slightly more sense in the exotic (and currently unused) case of restarting a crashed snap_on_start *and* save_on_stop volume. --- qubes/storage/reflink.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qubes/storage/reflink.py b/qubes/storage/reflink.py index 7660dedd..9132be3f 100644 --- a/qubes/storage/reflink.py +++ b/qubes/storage/reflink.py @@ -151,12 +151,12 @@ class ReflinkVolume(qubes.storage.Volume): return self.save_on_stop and os.path.exists(self._path_dirty) def start(self): + if self.is_dirty(): # implies self.save_on_stop + return self if self.snap_on_start: # pylint: disable=protected-access _copy_file(self.source._path_clean, self._path_clean) - if self.is_dirty(): # implies self.save_on_stop - return self - if self.save_on_stop or self.snap_on_start: + if self.snap_on_start or self.save_on_stop: _copy_file(self._path_clean, self._path_dirty) else: _create_sparse_file(self._path_dirty, self.size)