From 796e6f509673f617faf5f2226c3d8d0732375839 Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Fri, 29 Jan 2021 18:13:29 +0000 Subject: [PATCH 1/2] storage/file: refactor is_dirty() --- qubes/storage/file.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/qubes/storage/file.py b/qubes/storage/file.py index c2374272..596f6ed1 100644 --- a/qubes/storage/file.py +++ b/qubes/storage/file.py @@ -214,11 +214,10 @@ class FileVolume(qubes.storage.Volume): _remove_if_exists(self.path_cow) def is_dirty(self): - if not self.save_on_stop: - return False - if os.path.exists(self.path_cow): - stat = os.stat(self.path_cow) - return stat.st_blocks > 0 + if self.save_on_stop: + with suppress(FileNotFoundError), open(self.path_cow, 'rb') as cow: + cow_used = os.fstat(cow.fileno()).st_blocks * BLKSIZE + return cow_used > 0 return False def resize(self, size): From 05eb0511932a47c5d2c8f462ee1fa829599b59dc Mon Sep 17 00:00:00 2001 From: Rusty Bird Date: Thu, 11 Feb 2021 09:34:48 +0000 Subject: [PATCH 2/2] storage/file: fix is_dirty() false positive is_dirty() returned a false positive if the volume was merely the source of a currently running volume. For example, if fedora-33:root was the source volume for myappvm:root and myappvm was running - then is_dirty() returned True for fedora-33:root, because fedora-33/root-cow.img contains some allocated blocks (one 256 KiB chunk containing only the header) in this scenario, even though fedora-33 is shut down. Fixes QubesOS/qubes-issues#6371 --- linux/system-config/block-snapshot | 4 +++- qubes/storage/file.py | 11 ++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/linux/system-config/block-snapshot b/linux/system-config/block-snapshot index eaea93d3..95fdf1be 100755 --- a/linux/system-config/block-snapshot +++ b/linux/system-config/block-snapshot @@ -4,6 +4,8 @@ # # This creates dm-snapshot device on given arguments +SNAPSHOT_CHUNKSIZE=256 # same as in file.py + dir=$(dirname "$0") if [ "$1" = "prepare" ] || [ "$1" = "cleanup" ]; then # shellcheck disable=SC1090,SC1091 @@ -80,7 +82,7 @@ create_dm_snapshot() { base_dev=$(get_dev "$base") cow_dev=$(get_dev "$cow") base_sz=$(blockdev --getsz "$base_dev") - do_or_die dmsetup create "$dm_devname" --table "0 $base_sz snapshot $base_dev $cow_dev P 256" + do_or_die dmsetup create "$dm_devname" --table "0 $base_sz snapshot $base_dev $cow_dev P $SNAPSHOT_CHUNKSIZE" fi } diff --git a/qubes/storage/file.py b/qubes/storage/file.py index 596f6ed1..176b0a26 100644 --- a/qubes/storage/file.py +++ b/qubes/storage/file.py @@ -33,6 +33,12 @@ import qubes.utils BLKSIZE = 512 +# 256 KiB chunk, same as in block-snapshot script. Header created by +# struct.pack('<4I', 0x70416e53, 1, 1, 256) mimicking write_header() +# in linux/drivers/md/dm-snap-persistent.c +EMPTY_SNAPSHOT = b'SnAp\x01\x00\x00\x00\x01\x00\x00\x00\x00\x01\x00\x00' \ + + bytes(262128) + class FilePool(qubes.storage.Pool): ''' File based 'original' disk implementation @@ -217,7 +223,10 @@ class FileVolume(qubes.storage.Volume): if self.save_on_stop: with suppress(FileNotFoundError), open(self.path_cow, 'rb') as cow: cow_used = os.fstat(cow.fileno()).st_blocks * BLKSIZE - return cow_used > 0 + return (cow_used > 0 and + (cow_used > len(EMPTY_SNAPSHOT) or + cow.read(len(EMPTY_SNAPSHOT)) != EMPTY_SNAPSHOT or + cow_used > cow.seek(0, os.SEEK_HOLE))) return False def resize(self, size):