From 421ee871375ae40afa0f4a60528eacc85ed0cb69 Mon Sep 17 00:00:00 2001 From: George Amanakis Date: Sat, 4 Jan 2025 18:37:40 +0100 Subject: [PATCH] optimize recv_fix_encryption_hierarchy() 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: 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 --- lib/libzfs/libzfs_sendrecv.c | 100 ++++++++++-------- .../rsend/send_encrypted_hierarchy.ksh | 31 ++++-- 2 files changed, 80 insertions(+), 51 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index b9780720e5a3..97920ce6f21c 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -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, @@ -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)); /* @@ -3518,14 +3530,14 @@ recv_incremental_replication(libzfs_handle_t *hdl, const char *tofs, boolean_t needagain, progress, recursive; 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; @@ -3999,9 +4011,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 && *top_zfs != NULL && !flags->dryrun) { softerr = recv_fix_encryption_hierarchy(hdl, *top_zfs, - stream_nv); + stream_nv, stream_avl); } out: diff --git a/tests/zfs-tests/tests/functional/rsend/send_encrypted_hierarchy.ksh b/tests/zfs-tests/tests/functional/rsend/send_encrypted_hierarchy.ksh index 8417afc88d33..6dd4ae46f947 100755 --- a/tests/zfs-tests/tests/functional/rsend/send_encrypted_hierarchy.ksh +++ b/tests/zfs-tests/tests/functional/rsend/send_encrypted_hierarchy.ksh @@ -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" \ @@ -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."