Skip to content

Commit

Permalink
optimize recv_fix_encryption_hierarchy()
Browse files Browse the repository at this point in the history 
recv_fix_encryption_hierarchy() in its present state goes through all
stream filesystems, and for each one traverses the snapshots in order to
find one that exists locally. This happens by calling guid_to_name() for
each snapshot, which iterates through all children of the filesystem.
This results in CPU utilization of 100% for several minutes (for ~1000
filesystems on a Ryzen 4350G) for 1 thread at the end of a raw receive
(-w, regardless whether encrypted or not, dryrun or not).

Fix this by following a different logic: using the top_fs name, call
gather_nvlist() to gather the nvlists for all local filesystems. For
each one filesystem, go through the snapshots to find the corresponding
stream's filesystem (since we know the snapshots guid and can search
with it in stream_avl for the stream's fs). Then go on to fix the
encryption roots and locations as in its present state.

Avoiding guid_to_name() iteratively makes
recv_fix_encryption_hierarchy() significantly faster (from several
minutes to seconds for ~1000 filesystems on a Ryzen 4350G).

Another problem is the following:
in case we have promoted a clone of the filesystem outside the top
filesystem specified in zfs send, zfs receive does not fail but returns
an error (1): recv_incremental_replication() fails to find its origin and
errors out with needagain=1. This results in recv_fix_hierarchy() not
being called which may render some children of the top fs not mountable
since their encryption root was not updated. To circumvent this make
recv_incremental_replication() silently ignore this error.

Signed-off-by: George Amanakis <[email protected]>
  • Loading branch information
@gamanakis
gamanakis committed Jan 21, 2025
1 parent 919bc4d commit a9aaaf0
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 54 deletions.
115 changes: 68 additions & 47 deletions lib/libzfs/libzfs_sendrecv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3376,66 +3376,78 @@ created_before(libzfs_handle_t *hdl, avl_tree_t *avl,
*/
static int
recv_fix_encryption_hierarchy(libzfs_handle_t *hdl, const char *top_zfs,
nvlist_t *stream_nv)
nvlist_t *stream_nv, avl_tree_t *stream_avl)
{
int err;
nvpair_t *fselem = NULL;
nvlist_t *stream_fss;
nvlist_t *local_nv;
avl_tree_t *local_avl;
boolean_t recursive;

recursive = (nvlist_lookup_boolean(stream_nv, "not_recursive") ==
ENOENT);

stream_fss = fnvlist_lookup_nvlist(stream_nv, "fss");
/* Using top_zfs, gather the nvlists for all local filesystems. */
if ((err = gather_nvlist(hdl, top_zfs, NULL, NULL,
recursive, B_TRUE, B_FALSE, recursive, B_FALSE, B_FALSE, B_FALSE,
B_FALSE, B_TRUE, &local_nv, &local_avl)) != 0)
return (err);

while ((fselem = nvlist_next_nvpair(stream_fss, fselem)) != NULL) {
/*
* Go through the nvlists of the local filesystems and check for
* encryption roots.
*/
while ((fselem = nvlist_next_nvpair(local_nv, fselem)) != NULL) {
zfs_handle_t *zhp = NULL;
uint64_t crypt;
nvlist_t *snaps, *props, *stream_nvfs = NULL;
nvpair_t *snapel = NULL;
nvlist_t *stream_props, *snaps, *stream_nvfs = NULL,
*nvfs = NULL;
boolean_t is_encroot, is_clone, stream_encroot;
char *cp;
const char *stream_keylocation = NULL;
const char *stream_keylocation = NULL, *fsname;
char keylocation[MAXNAMELEN];
char fsname[ZFS_MAX_DATASET_NAME_LEN];

keylocation[0] = '\0';
stream_nvfs = fnvpair_value_nvlist(fselem);
snaps = fnvlist_lookup_nvlist(stream_nvfs, "snaps");
props = fnvlist_lookup_nvlist(stream_nvfs, "props");
stream_encroot = nvlist_exists(stream_nvfs, "is_encroot");

/* find a snapshot from the stream that exists locally */
err = ENOENT;
while ((snapel = nvlist_next_nvpair(snaps, snapel)) != NULL) {
uint64_t guid;

guid = fnvpair_value_uint64(snapel);
err = guid_to_name(hdl, top_zfs, guid, B_FALSE,
fsname);
if (err == 0)
break;
}

if (err != 0)
continue;

cp = strchr(fsname, '@');
if (cp != NULL)
*cp = '\0';
nvpair_t *snapelem;

nvfs = fnvpair_value_nvlist(fselem);
snaps = fnvlist_lookup_nvlist(nvfs, "snaps");
fsname = fnvlist_lookup_string(nvfs, "name");
zhp = zfs_open(hdl, fsname, ZFS_TYPE_DATASET);
if (zhp == NULL) {
err = ENOENT;
goto error;
}

crypt = zfs_prop_get_int(zhp, ZFS_PROP_ENCRYPTION);
is_clone = zhp->zfs_dmustats.dds_origin[0] != '\0';
(void) zfs_crypto_get_encryption_root(zhp, &is_encroot, NULL);

/* we don't need to do anything for unencrypted datasets */
crypt = zfs_prop_get_int(zhp, ZFS_PROP_ENCRYPTION);
if (crypt == ZIO_CRYPT_OFF) {
zfs_close(zhp);
continue;
}

is_clone = zhp->zfs_dmustats.dds_origin[0] != '\0';
(void) zfs_crypto_get_encryption_root(zhp, &is_encroot, NULL);
keylocation[0] = '\0';

/*
* Go through the snapshots of the local filesystem and find
* the stream's filesystem.
*/
for (snapelem = nvlist_next_nvpair(snaps, NULL);
snapelem; snapelem = nvlist_next_nvpair(snaps, snapelem)) {
uint64_t thisguid;

thisguid = fnvpair_value_uint64(snapelem);
stream_nvfs = fsavl_find(stream_avl, thisguid, NULL);

if (stream_nvfs != NULL)
break;
}

if (stream_nvfs == NULL)
continue;

stream_props = fnvlist_lookup_nvlist(stream_nvfs, "props");
stream_encroot = nvlist_exists(stream_nvfs, "is_encroot");

/*
* If the dataset is flagged as an encryption root, was not
* received as a clone and is not currently an encryption root,
Expand All @@ -3451,7 +3463,7 @@ recv_fix_encryption_hierarchy(libzfs_handle_t *hdl, const char *top_zfs,
}
}

stream_keylocation = fnvlist_lookup_string(props,
stream_keylocation = fnvlist_lookup_string(stream_props,
zfs_prop_to_name(ZFS_PROP_KEYLOCATION));

/*
Expand Down Expand Up @@ -3515,19 +3527,19 @@ recv_incremental_replication(libzfs_handle_t *hdl, const char *tofs,
char newname[ZFS_MAX_DATASET_NAME_LEN];
char guidname[32];
int error;
boolean_t needagain, progress, recursive;
boolean_t needagain, progress, recursive, false_origin;
const char *s1, *s2;

if (flags->dryrun)
return (0);

fromsnap = fnvlist_lookup_string(stream_nv, "fromsnap");

recursive = (nvlist_lookup_boolean(stream_nv, "not_recursive") ==
ENOENT);

if (flags->dryrun)
return (0);

again:
needagain = progress = B_FALSE;
needagain = progress = false_origin = B_FALSE;

deleted = fnvlist_alloc();

Expand Down Expand Up @@ -3608,6 +3620,7 @@ recv_incremental_replication(libzfs_handle_t *hdl, const char *tofs,
* them on the next pass.
*/
needagain = B_TRUE;
false_origin = B_TRUE;
continue;
}

Expand Down Expand Up @@ -3801,7 +3814,15 @@ recv_incremental_replication(libzfs_handle_t *hdl, const char *tofs,
goto again;
}

return (needagain || error != 0);
/*
* If false_origin is set then we do the have the wrong origin,
* but we are unable to find the correct one, since the promoted
* clone lies outside of the top fs. In this case continue normally.
*/
if (false_origin)
return (error != 0);
else
return (needagain || error != 0);
}

static int
Expand Down Expand Up @@ -3999,9 +4020,9 @@ zfs_receive_package(libzfs_handle_t *hdl, int fd, const char *destname,
stream_nv, stream_avl, NULL);
}

if (raw && softerr == 0 && *top_zfs != NULL) {
if (raw && softerr == 0 && *top_zfs != NULL && !flags->dryrun) {
softerr = recv_fix_encryption_hierarchy(hdl, *top_zfs,
stream_nv);
stream_nv, stream_avl);
}

out:
Expand Down
31 changes: 24 additions & 7 deletions tests/zfs-tests/tests/functional/rsend/send_encrypted_hierarchy.ksh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,17 @@ log_must eval "zfs receive -d -F $POOL2 < $BACKDIR/fs-before-R"
dstds=$(get_dst_ds $POOL/$FS $POOL2)
log_must cmp_ds_subs $POOL/$FS $dstds

log_must verify_encryption_root $POOL/$FS $POOL/$FS
log_must verify_keylocation $POOL/$FS "prompt"
log_must verify_origin $POOL/$FS "-"
log_must verify_encryption_root $POOL2/$FS $POOL2/$FS
log_must verify_keylocation $POOL2/$FS "prompt"
log_must verify_origin $POOL2/$FS "-"

log_must verify_encryption_root $POOL/clone $POOL/$FS
log_must verify_keylocation $POOL/clone "none"
log_must verify_origin $POOL/clone "$POOL/$FS@snap"
log_must verify_encryption_root $POOL2/clone $POOL2/$FS
log_must verify_keylocation $POOL2/clone "none"
log_must verify_origin $POOL2/clone "$POOL2/$FS@snap"

log_must verify_encryption_root $POOL/$FS/child $POOL/$FS
log_must verify_keylocation $POOL/$FS/child "none"
log_must verify_encryption_root $POOL2/$FS/child $POOL2/$FS
log_must verify_keylocation $POOL2/$FS/child "none"

# Alter the hierarchy and re-send
log_must eval "echo $PASSPHRASE1 | zfs change-key -o keyformat=passphrase" \
Expand All @@ -93,4 +94,20 @@ log_must verify_origin $POOL/clone "-"
log_must verify_encryption_root $POOL/$FS/child $POOL/$FS/child
log_must verify_keylocation $POOL/$FS/child "prompt"

log_must verify_encryption_root $POOL2 "-"
log_must verify_encryption_root $POOL2/clone $POOL2/clone
log_must verify_encryption_root $POOL2/$FS $POOL2/clone
log_must verify_encryption_root $POOL2/$FS/child $POOL2/$FS/child

log_must verify_keylocation $POOL2 "none"
log_must verify_keylocation $POOL2/clone "prompt"
log_must verify_keylocation $POOL2/$FS "none"
log_must verify_keylocation $POOL2/$FS/child "prompt"

log_must verify_origin $POOL2 "-"
log_must verify_origin $POOL2/clone "-"
log_must verify_origin $POOL2/$FS "$POOL2/clone@snap"
log_must verify_origin $POOL2/$FS/child "-"
log_must zfs list

log_pass "Raw recursive sends preserve filesystem structure."

0 comments on commit a9aaaf0

Please sign in to comment.