peter/borgmatic
1
0
Fork 0
mirror of https://github.com/borgmatic-collective/borgmatic.git synced 2026-07-22 02:03:01 +02:00
Code Issues Packages Projects Releases Wiki Activity

Fix the ZFS hook's overzealous unmounting of snapshot paths when a source dataset is at "/" (#1319).

Browse Source
This commit is contained in:
Dan Helfman
2026-06-10 16:15:32 -07:00
parent d74b340024
commit 501f12dce3
3 changed files with 88 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
NEWS
+2
View File
@@ -1,4 +1,6 @@
2.1.7.dev0
* #1319: Fix the ZFS hook's overzealous unmounting of snapshot paths when a source dataset is at
"/".
* Fix a bug in which the "compact" action does not pass a compact threshold of zero to Borg.
2.1.6
borgmatic/hooks/data_source/zfs.py
+42 -26
View File
@@ -403,41 +403,57 @@ def remove_data_source_dumps(hook_config, config, borgmatic_runtime_directory, p
snapshot_dataset_names = {
full_snapshot_name.split('@')[0] for full_snapshot_name in full_snapshot_names
}
hash_to_dataset_mount_point = {}
# Make a map from mount point hash to the corresponding (dataset name, mount point) tuple.
for dataset_name, mount_point in dataset_name_to_mount_point.items():
mount_point_hash = hashlib.shake_256(mount_point.encode('utf-8')).hexdigest(
MOUNT_POINT_HASH_LENGTH
)
hash_to_dataset_mount_point[mount_point_hash] = (dataset_name, mount_point)
for snapshots_directory in glob.glob(snapshots_glob):
if not os.path.isdir(snapshots_directory):
continue
for dataset_name, mount_point in dataset_name_to_mount_point.items():
snapshot_mount_path = os.path.join(snapshots_directory, mount_point.lstrip(os.path.sep))
# Get the dataset and mount point corresponding to the hash found in this snapshot directory
# path. If none is found, bail.
try:
(dataset_name, mount_point) = hash_to_dataset_mount_point[
os.path.basename(snapshots_directory)
]
except KeyError:
continue
# If this dataset name does not correspond to a known snapshot, then this is probably
# just a "shadow" of a nested dataset and therefore there's nothing to unmount.
if not os.path.isdir(snapshot_mount_path) or dataset_name not in snapshot_dataset_names:
snapshot_mount_path = os.path.join(snapshots_directory, mount_point.lstrip(os.path.sep))
# If this dataset name doesn't correspond to a known snapshot, then this is probably
# just a "shadow" of a nested dataset and therefore there's nothing to unmount.
if not os.path.isdir(snapshot_mount_path) or dataset_name not in snapshot_dataset_names:
continue
# This might fail if the path is already mounted, but we swallow errors here since we'll
# do another recursive delete below. The point of doing it here is that we don't want to
# try to unmount a non-mounted directory (which *will* fail), and probing for whether a
# directory is mounted is tough to do in a cross-platform way.
if not dry_run:
shutil.rmtree(snapshot_mount_path, ignore_errors=True)
# If the delete was successful, that means there's nothing to unmount.
if not os.path.isdir(snapshot_mount_path):
continue
# This might fail if the path is already mounted, but we swallow errors here since we'll
# do another recursive delete below. The point of doing it here is that we don't want to
# try to unmount a non-mounted directory (which *will* fail), and probing for whether a
# directory is mounted is tough to do in a cross-platform way.
if not dry_run:
shutil.rmtree(snapshot_mount_path, ignore_errors=True)
logger.debug(f'Unmounting ZFS snapshot at {snapshot_mount_path}{dry_run_label}')
# If the delete was successful, that means there's nothing to unmount.
if not os.path.isdir(snapshot_mount_path):
continue
logger.debug(f'Unmounting ZFS snapshot at {snapshot_mount_path}{dry_run_label}')
if not dry_run:
try:
unmount_snapshot(umount_command, snapshot_mount_path)
except FileNotFoundError:
logger.debug(f'Could not find "{umount_command}" command')
return
except subprocess.CalledProcessError as error:
logger.debug(error)
continue
if not dry_run:
try:
unmount_snapshot(umount_command, snapshot_mount_path)
except FileNotFoundError:
logger.debug(f'Could not find "{umount_command}" command')
return
except subprocess.CalledProcessError as error:
logger.debug(error)
continue
if not dry_run:
shutil.rmtree(snapshot_mount_path, ignore_errors=True)
tests/unit/hooks/data_source/test_zfs.py
+44 -4
View File
@@ -528,6 +528,9 @@ def test_remove_data_source_dumps_unmounts_and_destroys_snapshots():
flexmock(module.borgmatic.config.paths).should_receive(
'replace_temporary_subdirectory_with_glob',
).and_return('/run/borgmatic')
flexmock(module.hashlib).should_receive('shake_256').and_return(
flexmock(hexdigest=lambda length: 'b33f')
)
flexmock(module.glob).should_receive('glob').replace_with(
lambda path: [path.replace('*', 'b33f')],
)
@@ -561,6 +564,9 @@ def test_remove_data_source_dumps_use_custom_commands():
flexmock(module.borgmatic.config.paths).should_receive(
'replace_temporary_subdirectory_with_glob',
).and_return('/run/borgmatic')
flexmock(module.hashlib).should_receive('shake_256').and_return(
flexmock(hexdigest=lambda length: 'b33f')
)
flexmock(module.glob).should_receive('glob').replace_with(
lambda path: [path.replace('*', 'b33f')],
)
@@ -644,6 +650,9 @@ def test_remove_data_source_dumps_bails_for_missing_umount_command():
flexmock(module.borgmatic.config.paths).should_receive(
'replace_temporary_subdirectory_with_glob',
).and_return('/run/borgmatic')
flexmock(module.hashlib).should_receive('shake_256').and_return(
flexmock(hexdigest=lambda length: 'b33f')
)
flexmock(module.glob).should_receive('glob').replace_with(
lambda path: [path.replace('*', 'b33f')],
)
@@ -675,6 +684,9 @@ def test_remove_data_source_dumps_swallows_umount_command_error():
flexmock(module.borgmatic.config.paths).should_receive(
'replace_temporary_subdirectory_with_glob',
).and_return('/run/borgmatic')
flexmock(module.hashlib).should_receive('shake_256').and_return(
flexmock(hexdigest=lambda length: 'b33f')
)
flexmock(module.glob).should_receive('glob').replace_with(
lambda path: [path.replace('*', 'b33f')],
)
@@ -709,6 +721,9 @@ def test_remove_data_source_dumps_skips_unmount_snapshot_directories_that_are_no
flexmock(module.borgmatic.config.paths).should_receive(
'replace_temporary_subdirectory_with_glob',
).and_return('/run/borgmatic')
flexmock(module.hashlib).should_receive('shake_256').and_return(
flexmock(hexdigest=lambda length: 'b33f')
)
flexmock(module.glob).should_receive('glob').replace_with(
lambda path: [path.replace('*', 'b33f')],
)
@@ -739,6 +754,9 @@ def test_remove_data_source_dumps_skips_unmount_snapshot_mount_paths_that_are_no
flexmock(module.borgmatic.config.paths).should_receive(
'replace_temporary_subdirectory_with_glob',
).and_return('/run/borgmatic')
flexmock(module.hashlib).should_receive('shake_256').and_return(
flexmock(hexdigest=lambda length: 'b33f')
)
flexmock(module.glob).should_receive('glob').replace_with(
lambda path: [path.replace('*', 'b33f')],
)
@@ -774,21 +792,37 @@ def test_remove_data_source_dumps_skips_unmount_snapshot_mount_paths_for_unknown
flexmock(module.borgmatic.config.paths).should_receive(
'replace_temporary_subdirectory_with_glob',
).and_return('/run/borgmatic')
flexmock(module.glob).should_receive('glob').replace_with(
lambda path: [path.replace('*', 'b33f')],
flexmock(module.hashlib).should_receive('shake_256').with_args(b'/mnt/dataset').and_return(
flexmock(hexdigest=lambda length: 'd34d')
)
flexmock(module.hashlib).should_receive('shake_256').with_args(
b'/mnt/dataset/shadow'
).and_return(flexmock(hexdigest=lambda length: 'b33f'))
flexmock(module.glob).should_receive('glob').replace_with(
lambda path: [
path.replace('*', 'd34d'),
path.replace('*', 'b33f'),
path.replace('*', 'unknown'),
],
)
flexmock(module.os.path).should_receive('isdir').with_args(
'/run/borgmatic/zfs_snapshots/d34d',
).and_return(True)
flexmock(module.os.path).should_receive('isdir').with_args(
'/run/borgmatic/zfs_snapshots/b33f',
).and_return(True)
flexmock(module.os.path).should_receive('isdir').with_args(
'/run/borgmatic/zfs_snapshots/b33f/mnt/dataset',
'/run/borgmatic/zfs_snapshots/d34d/mnt/dataset',
).and_return(True)
flexmock(module.os.path).should_receive('isdir').with_args(
'/run/borgmatic/zfs_snapshots/b33f/mnt/dataset/shadow',
).and_return(True)
flexmock(module.os.path).should_receive('isdir').with_args(
'/run/borgmatic/zfs_snapshots/unknown',
).and_return(True)
flexmock(module.shutil).should_receive('rmtree')
flexmock(module).should_receive('unmount_snapshot').with_args(
'umount', '/run/borgmatic/zfs_snapshots/b33f/mnt/dataset'
'umount', '/run/borgmatic/zfs_snapshots/d34d/mnt/dataset'
).once()
flexmock(module).should_receive('unmount_snapshot').with_args(
'umount', '/run/borgmatic/zfs_snapshots/b33f/mnt/dataset/shadow'
@@ -817,6 +851,9 @@ def test_remove_data_source_dumps_skips_unmount_snapshot_mount_paths_after_rmtre
flexmock(module.borgmatic.config.paths).should_receive(
'replace_temporary_subdirectory_with_glob',
).and_return('/run/borgmatic')
flexmock(module.hashlib).should_receive('shake_256').and_return(
flexmock(hexdigest=lambda length: 'b33f')
)
flexmock(module.glob).should_receive('glob').replace_with(
lambda path: [path.replace('*', 'b33f')],
)
@@ -852,6 +889,9 @@ def test_remove_data_source_dumps_with_dry_run_skips_unmount_and_destroy():
flexmock(module.borgmatic.config.paths).should_receive(
'replace_temporary_subdirectory_with_glob',
).and_return('/run/borgmatic')
flexmock(module.hashlib).should_receive('shake_256').and_return(
flexmock(hexdigest=lambda length: 'b33f')
)
flexmock(module.glob).should_receive('glob').replace_with(
lambda path: [path.replace('*', 'b33f')],
)