Skip to content

Commit

Permalink
on Linux force convert 'zfs send -I' to a series of 'zfs send -i' as …
Browse files Browse the repository at this point in the history
…a work-around for openzfs/zfs#16394
  • Loading branch information
whoschek committed Jul 28, 2024
1 parent e12c084 commit 7126472
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 11 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ cat /tmp/manpage.md
--no-privilege-elevation flag is for non-root users that have been
granted corresponding ZFS permissions by administrators via 'zfs
allow' delegation mechanism, like so: sudo zfs allow -u
$NON_ROOT_USER_NAME send,hold,bookmark $SRC_DATASET; sudo zfs
$NON_ROOT_USER_NAME send,bookmark $SRC_DATASET; sudo zfs
allow -u $NON_ROOT_USER_NAME
mount,create,receive,rollback,destroy,canmount,mountpoint,readonly,compression,encryption,keylocation,recordsize
$DST_DATASET_OR_POOL; If you do not plan to use the --force flag
Expand Down
12 changes: 6 additions & 6 deletions test/test_wbackup_zfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ def run_wbackup(self, *args, dry_run=None, no_create_bookmark=False, no_use_book
if self.is_no_privilege_elevation():
# test ZFS delegation in combination with --no-privilege-elevation flag
args = args + ['--no-privilege-elevation']
src_permissions = 'send,hold'
src_permissions = 'send'
if not is_solaris_zfs():
src_permissions += ',bookmark'
optional_dst_permissions = ',canmount,mountpoint,readonly,compression,encryption,keylocation,recordsize'
Expand Down Expand Up @@ -1320,9 +1320,9 @@ def permute_snapshot_series(self, max_length=9):
def validate_incremental_replication_steps(self, input_snapshots, expected_results):
# src_dataset = "s@"
src_dataset = ""
for is_bookmark_start in [False, True]:
for force_convert_I_to_i in [False, True]:
steps = self.incremental_replication_steps1(
input_snapshots, src_dataset, is_bookmark_start=is_bookmark_start)
input_snapshots, src_dataset, force_convert_I_to_i=force_convert_I_to_i)
# print(f"input_snapshots:" + ','.join(input_snapshots))
# print("steps: " + ','.join([self.replication_step_to_str(step) for step in steps]))
output_snapshots = [] if len(expected_results) == 0 else [expected_results[0]]
Expand All @@ -1346,13 +1346,13 @@ def apply_incremental_replication_steps(self, steps, input_snapshots):
output_snapshots.append(input_snapshots[end])
return output_snapshots

def incremental_replication_steps1(self, input_snapshots, src_dataset, is_bookmark_start=False):
def incremental_replication_steps1(self, input_snapshots, src_dataset, force_convert_I_to_i=False):
origin_src_snapshots_with_guids = []
has_snapshot = set()
has_snapshot = None if force_convert_I_to_i else set()
guid = 1
for i, snapshot in enumerate(input_snapshots):
origin_src_snapshots_with_guids.append(f"{guid}\t{src_dataset}{snapshot}")
if i > 0 or not is_bookmark_start:
if has_snapshot is not None:
has_snapshot.add(guid)
guid += 1
return self.incremental_replication_steps2(origin_src_snapshots_with_guids, has_snapshot)
Expand Down
12 changes: 8 additions & 4 deletions wbackup_zfs/wbackup_zfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ def argument_parser() -> argparse.ArgumentParser:
"`<NON_ROOT_USER_NAME> ALL=NOPASSWD:/path/to/zfs`). "
"Instead, the --no-privilege-elevation flag is for non-root users that have been granted corresponding "
"ZFS permissions by administrators via 'zfs allow' delegation mechanism, like so: "
"sudo zfs allow -u $NON_ROOT_USER_NAME send,hold,bookmark $SRC_DATASET; "
"sudo zfs allow -u $NON_ROOT_USER_NAME send,bookmark $SRC_DATASET; "
"sudo zfs allow -u $NON_ROOT_USER_NAME mount,create,receive,rollback,destroy,canmount,mountpoint,"
"readonly,compression,encryption,keylocation,recordsize $DST_DATASET_OR_POOL; "
"If you do not plan to use the --force flag or --delete-missing-snapshots or --delete-missing-dataset "
Expand Down Expand Up @@ -1029,6 +1029,10 @@ def incremental_replication_steps_wrapper(self, origin_src_snapshots_with_guids:
guid, snapshot = line.split('\t', 1)
guids.append(guid)
input_snapshots.append(snapshot)
if not self.params.src_sudo and os.geteuid() != 0 and platform.system() == 'Linux':
# If using 'zfs allow' delegation mechanism on Linux, force convert 'zfs send -I' to a series of
# 'zfs send -i' as a workaround for zfs bug https://github.com/openzfs/zfs/issues/16394
has_snapshot = None
return self.incremental_replication_steps(input_snapshots, guids, included_guids, has_snapshot)

def incremental_replication_steps(self, src_snapshots: List[str], guids: List[str],
Expand All @@ -1044,10 +1048,10 @@ def incremental_replication_steps(self, src_snapshots: List[str], guids: List[st
The has_snapshot param is necessary because 'zfs send' CLI with a bookmark as starting snapshot does not
(yet) support including intermediate src_snapshots via -I flag. Thus, if the replication source is a bookmark
we convert a -I step to one or more -i steps.
has_snapshot is None case is necessary as a work-around for https://github.com/openzfs/zfs/issues/16394
"""
assert len(guids) == len(src_snapshots)
assert len(included_guids) >= 0
assert len(has_snapshot) >= 0
steps = []
n = len(guids)
i = 0
Expand Down Expand Up @@ -1080,7 +1084,7 @@ def incremental_replication_steps(self, src_snapshots: List[str], guids: List[st
if start != i:
step = ('-I', src_snapshots[start], src_snapshots[i])
# print(f"r2 {self.replication_step_to_str(step)}")
if guids[start] in has_snapshot:
if has_snapshot is not None and guids[start] in has_snapshot:
steps.append(step)
else: # convert to -i steps
for j in range(start, i):
Expand All @@ -1092,7 +1096,7 @@ def incremental_replication_steps(self, src_snapshots: List[str], guids: List[st
if start != i:
step = ('-I', src_snapshots[start], src_snapshots[i])
# print(f"r3 {self.replication_step_to_str(step)}")
if guids[start] in has_snapshot:
if has_snapshot is not None and guids[start] in has_snapshot:
steps.append(step)
else: # convert to -i steps
for j in range(start, i):
Expand Down

0 comments on commit 7126472

Please sign in to comment.