From 7f071e70f9f34a39f2b14b0dc3099499406c0d83 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Tue, 27 Jul 2021 13:45:28 +0000 Subject: [PATCH 01/19] libzfs_sendrecv: Use size_t for payload_len We won't be passing a negative value here, so make it clear. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index d07e355e3921..8d2a6376df40 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -86,7 +86,7 @@ typedef struct progress_arg { } progress_arg_t; static int -dump_record(dmu_replay_record_t *drr, void *payload, int payload_len, +dump_record(dmu_replay_record_t *drr, void *payload, size_t payload_len, zio_cksum_t *zc, int outfd) { ASSERT3U(offsetof(dmu_replay_record_t, drr_u.drr_checksum.drr_checksum), From b248e9767efce813afe587110f791dd1759f939d Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Tue, 27 Jul 2021 13:46:35 +0000 Subject: [PATCH 02/19] libzfs_sendrecv: Simplify out guid temporary De-clutter the clode and make it clear the guid is only used here. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 8d2a6376df40..c7be6247c159 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -191,16 +191,14 @@ fsavl_create(nvlist_t *fss) while ((snapelem = nvlist_next_nvpair(snaps, snapelem)) != NULL) { fsavl_node_t *fn; - uint64_t guid; - guid = fnvpair_value_uint64(snapelem); if ((fn = malloc(sizeof (fsavl_node_t))) == NULL) { fsavl_destroy(fsavl); return (NULL); } fn->fn_nvfs = nvfs; fn->fn_snapname = nvpair_name(snapelem); - fn->fn_guid = guid; + fn->fn_guid = fnvpair_value_uint64(snapelem); /* * Note: if there are multiple snaps with the From 1f805547432dda0268dd3d776c5581c973a8b3c9 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Tue, 27 Jul 2021 13:47:27 +0000 Subject: [PATCH 03/19] libzfs_sendrecv: Avoid extra avl_find avl_add does avl_find internally, then avl_insert. We're already doing the avl_find, so using avl_insert directly avoids repeating the search. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index c7be6247c159..f0775ce2bd59 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -204,8 +204,9 @@ fsavl_create(nvlist_t *fss) * Note: if there are multiple snaps with the * same GUID, we ignore all but one. */ - if (avl_find(fsavl, fn, NULL) == NULL) - avl_add(fsavl, fn); + avl_index_t where = 0; + if (avl_find(fsavl, fn, &where) == NULL) + avl_insert(fsavl, fn, where); else free(fn); } From 252575d14738f314c701c94adaaaa186c5f95861 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Tue, 27 Jul 2021 13:48:42 +0000 Subject: [PATCH 04/19] libzfs_sendrecv: Fix leaked holds nvlist There is no need to allocate a holds nvlist. lzc_get_holds does that for us. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index f0775ce2bd59..5b2f62130b69 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -349,12 +349,11 @@ send_iterate_snap(zfs_handle_t *zhp, void *arg) fnvlist_add_nvlist(sd->snapprops, snapname, nv); fnvlist_free(nv); if (sd->holds) { - nvlist_t *holds = fnvlist_alloc(); - int err = lzc_get_holds(zhp->zfs_name, &holds); - if (err == 0) { + nvlist_t *holds; + if (lzc_get_holds(zhp->zfs_name, &holds) == 0) { fnvlist_add_nvlist(sd->snapholds, snapname, holds); + fnvlist_free(holds); } - fnvlist_free(holds); } zfs_close(zhp); From 2d0c4da045504e72e9a3f3083e81e7b9d96f6237 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Tue, 27 Jul 2021 13:49:59 +0000 Subject: [PATCH 05/19] libzfs_sendrecv: Style pass on send_iterate_snap * Add a high level comment. * Use local variables to reduce line wrapping. * Remove extra braces and insert space for clarity. * Assert precondition that the dataset name contains '@' for sanity. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 5b2f62130b69..b5d2438c26c3 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -283,51 +283,54 @@ typedef struct send_data { static void send_iterate_prop(zfs_handle_t *zhp, boolean_t received_only, nvlist_t *nv); +/* + * Collect guid, valid props, optionally holds, etc. of a snapshot. + * This interface is intended for use as a zfs_iter_snapshots_sorted visitor. + */ static int send_iterate_snap(zfs_handle_t *zhp, void *arg) { send_data_t *sd = arg; uint64_t guid = zhp->zfs_dmustats.dds_guid; uint64_t txg = zhp->zfs_dmustats.dds_creation_txg; - char *snapname; - nvlist_t *nv; boolean_t isfromsnap, istosnap, istosnapwithnofrom; + char *snapname = strrchr(zhp->zfs_name, '@') + 1; + const char *from = sd->fromsnap; + const char *to = sd->tosnap; - snapname = strrchr(zhp->zfs_name, '@')+1; - isfromsnap = (sd->fromsnap != NULL && - strcmp(sd->fromsnap, snapname) == 0); - istosnap = (sd->tosnap != NULL && (strcmp(sd->tosnap, snapname) == 0)); - istosnapwithnofrom = (istosnap && sd->fromsnap == NULL); + assert(snapname != (NULL + 1)); + + isfromsnap = (from != NULL && strcmp(from, snapname) == 0); + istosnap = (to != NULL && strcmp(to, snapname) == 0); + istosnapwithnofrom = (istosnap && from == NULL); if (sd->tosnap_txg != 0 && txg > sd->tosnap_txg) { if (sd->verbose) { (void) fprintf(stderr, dgettext(TEXT_DOMAIN, "skipping snapshot %s because it was created " "after the destination snapshot (%s)\n"), - zhp->zfs_name, sd->tosnap); + zhp->zfs_name, to); } zfs_close(zhp); return (0); } fnvlist_add_uint64(sd->parent_snaps, snapname, guid); + /* * NB: if there is no fromsnap here (it's a newly created fs in * an incremental replication), we will substitute the tosnap. */ - if (isfromsnap || (sd->parent_fromsnap_guid == 0 && istosnap)) { + if (isfromsnap || (sd->parent_fromsnap_guid == 0 && istosnap)) sd->parent_fromsnap_guid = guid; - } if (!sd->recursive) { - /* * To allow a doall stream to work properly * with a NULL fromsnap */ - if (sd->doall && sd->fromsnap == NULL && !sd->seenfrom) { + if (sd->doall && from == NULL && !sd->seenfrom) sd->seenfrom = B_TRUE; - } if (!sd->seenfrom && isfromsnap) { sd->seenfrom = B_TRUE; @@ -344,10 +347,11 @@ send_iterate_snap(zfs_handle_t *zhp, void *arg) sd->seento = B_TRUE; } - nv = fnvlist_alloc(); + nvlist_t *nv = fnvlist_alloc(); send_iterate_prop(zhp, sd->backup, nv); fnvlist_add_nvlist(sd->snapprops, snapname, nv); fnvlist_free(nv); + if (sd->holds) { nvlist_t *holds; if (lzc_get_holds(zhp->zfs_name, &holds) == 0) { From 20e91feb125362139c50712a3f9aa5e9627bd22e Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Tue, 27 Jul 2021 16:18:29 +0000 Subject: [PATCH 06/19] libzfs_sendrecv: Style pass on send_iterate_prop * Add a high level comment. * Move locals closer to point of use. * Use fnv* routines rather than explicit verification of success. * Factor out duplicated code by introducing isspacelimit to clarify behavior. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 44 ++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index b5d2438c26c3..b3d1aa4c1e49 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -364,21 +364,23 @@ send_iterate_snap(zfs_handle_t *zhp, void *arg) return (0); } +/* + * Collect all valid props from the handle snap into an nvlist. + */ static void send_iterate_prop(zfs_handle_t *zhp, boolean_t received_only, nvlist_t *nv) { - nvlist_t *props = NULL; - nvpair_t *elem = NULL; + nvlist_t *props; if (received_only) props = zfs_get_recvd_props(zhp); else props = zhp->zfs_props; + nvpair_t *elem = NULL; while ((elem = nvlist_next_nvpair(props, elem)) != NULL) { char *propname = nvpair_name(elem); zfs_prop_t prop = zfs_name_to_prop(propname); - nvlist_t *propnv; if (!zfs_prop_user(propname)) { /* @@ -396,34 +398,26 @@ send_iterate_prop(zfs_handle_t *zhp, boolean_t received_only, nvlist_t *nv) continue; } - verify(nvpair_value_nvlist(elem, &propnv) == 0); - if (prop == ZFS_PROP_QUOTA || prop == ZFS_PROP_RESERVATION || + nvlist_t *propnv = fnvpair_value_nvlist(elem); + + boolean_t isspacelimit = (prop == ZFS_PROP_QUOTA || + prop == ZFS_PROP_RESERVATION || prop == ZFS_PROP_REFQUOTA || - prop == ZFS_PROP_REFRESERVATION) { - char *source; - uint64_t value; - verify(nvlist_lookup_uint64(propnv, - ZPROP_VALUE, &value) == 0); - if (zhp->zfs_type == ZFS_TYPE_SNAPSHOT) + prop == ZFS_PROP_REFRESERVATION); + if (isspacelimit && zhp->zfs_type == ZFS_TYPE_SNAPSHOT) + continue; + + char *source; + if (nvlist_lookup_string(propnv, ZPROP_SOURCE, &source) == 0) { + if (strcmp(source, zhp->zfs_name) != 0 && + strcmp(source, ZPROP_SOURCE_VAL_RECVD) != 0) continue; + } else { /* * May have no source before SPA_VERSION_RECVD_PROPS, * but is still modifiable. */ - if (nvlist_lookup_string(propnv, - ZPROP_SOURCE, &source) == 0) { - if ((strcmp(source, zhp->zfs_name) != 0) && - (strcmp(source, - ZPROP_SOURCE_VAL_RECVD) != 0)) - continue; - } - } else { - char *source; - if (nvlist_lookup_string(propnv, - ZPROP_SOURCE, &source) != 0) - continue; - if ((strcmp(source, zhp->zfs_name) != 0) && - (strcmp(source, ZPROP_SOURCE_VAL_RECVD) != 0)) + if (!isspacelimit) continue; } From 87c8ff7e1321d2cdde4898aa47264b0eae195d87 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Tue, 27 Jul 2021 16:20:15 +0000 Subject: [PATCH 07/19] libzfs_sendrecv: Style pass on send_iterate_fs * Capitalize and punctuate complete sentences in comments. * Separate out a group of locals to add a comment on their purpose. * Remove unnecessary line wrapping. * Make it clear that dds_origin is a string by using explicit character comparison to check for an empty string, rather than implictly treating it as a boolean. * Reorganize manipulation of props and holds nvlists to improve clarity. * There's no need to initialize the snapname buffer with zeros, we're immediately overwriting it. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 68 ++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index b3d1aa4c1e49..f795ef89f04f 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -460,7 +460,7 @@ get_snap_txg(libzfs_handle_t *hdl, const char *fs, const char *snap) } /* - * recursively generate nvlists describing datasets. See comment + * Recursively generate nvlists describing datasets. See comment * for the data structure send_data_t above for description of contents * of the nvlist. */ @@ -471,14 +471,16 @@ send_iterate_fs(zfs_handle_t *zhp, void *arg) nvlist_t *nvfs = NULL, *nv = NULL; int rv = 0; uint64_t min_txg = 0, max_txg = 0; - uint64_t parent_fromsnap_guid_save = sd->parent_fromsnap_guid; - uint64_t fromsnap_txg_save = sd->fromsnap_txg; - uint64_t tosnap_txg_save = sd->tosnap_txg; uint64_t txg = zhp->zfs_dmustats.dds_creation_txg; uint64_t guid = zhp->zfs_dmustats.dds_guid; uint64_t fromsnap_txg, tosnap_txg; char guidstring[64]; + /* These fields are restored on return from a recursive call. */ + uint64_t parent_fromsnap_guid_save = sd->parent_fromsnap_guid; + uint64_t fromsnap_txg_save = sd->fromsnap_txg; + uint64_t tosnap_txg_save = sd->tosnap_txg; + fromsnap_txg = get_snap_txg(zhp->zfs_hdl, zhp->zfs_name, sd->fromsnap); if (fromsnap_txg != 0) sd->fromsnap_txg = fromsnap_txg; @@ -488,14 +490,14 @@ send_iterate_fs(zfs_handle_t *zhp, void *arg) sd->tosnap_txg = tosnap_txg; /* - * on the send side, if the current dataset does not have tosnap, + * On the send side, if the current dataset does not have tosnap, * perform two additional checks: * - * - skip sending the current dataset if it was created later than - * the parent tosnap - * - return error if the current dataset was created earlier than + * - Skip sending the current dataset if it was created later than + * the parent tosnap. + * - Return error if the current dataset was created earlier than * the parent tosnap, unless --skip-missing specified. Then - * just print a warning + * just print a warning. */ if (sd->tosnap != NULL && tosnap_txg == 0) { if (sd->tosnap_txg != 0 && txg > sd->tosnap_txg) { @@ -522,10 +524,9 @@ send_iterate_fs(zfs_handle_t *zhp, void *arg) nvfs = fnvlist_alloc(); fnvlist_add_string(nvfs, "name", zhp->zfs_name); - fnvlist_add_uint64(nvfs, "parentfromsnap", - sd->parent_fromsnap_guid); + fnvlist_add_uint64(nvfs, "parentfromsnap", sd->parent_fromsnap_guid); - if (zhp->zfs_dmustats.dds_origin[0]) { + if (zhp->zfs_dmustats.dds_origin[0] != '\0') { zfs_handle_t *origin = zfs_open(zhp->zfs_hdl, zhp->zfs_dmustats.dds_origin, ZFS_TYPE_SNAPSHOT); if (origin == NULL) { @@ -534,19 +535,19 @@ send_iterate_fs(zfs_handle_t *zhp, void *arg) } fnvlist_add_uint64(nvfs, "origin", origin->zfs_dmustats.dds_guid); - zfs_close(origin); } - /* iterate over props */ + /* Iterate over props. */ if (sd->props || sd->backup || sd->recursive) { nv = fnvlist_alloc(); send_iterate_prop(zhp, sd->backup, nv); + fnvlist_add_nvlist(nvfs, "props", nv); } if (zfs_prop_get_int(zhp, ZFS_PROP_ENCRYPTION) != ZIO_CRYPT_OFF) { boolean_t encroot; - /* determine if this dataset is an encryption root */ + /* Determine if this dataset is an encryption root. */ if (zfs_crypto_get_encryption_root(zhp, &encroot, NULL) != 0) { rv = -1; goto out; @@ -572,22 +573,19 @@ send_iterate_fs(zfs_handle_t *zhp, void *arg) } - if (nv != NULL) - fnvlist_add_nvlist(nvfs, "props", nv); - - /* iterate over snaps, and set sd->parent_fromsnap_guid */ - sd->parent_fromsnap_guid = 0; - sd->parent_snaps = fnvlist_alloc(); - sd->snapprops = fnvlist_alloc(); - if (sd->holds) - sd->snapholds = fnvlist_alloc(); - /* + * Iterate over snaps, and set sd->parent_fromsnap_guid. + * * If this is a "doall" send, a replicate send or we're just trying * to gather a list of previous snapshots, iterate through all the * snaps in the txg range. Otherwise just look at the one we're * interested in. */ + sd->parent_fromsnap_guid = 0; + sd->parent_snaps = fnvlist_alloc(); + sd->snapprops = fnvlist_alloc(); + if (sd->holds) + sd->snapholds = fnvlist_alloc(); if (sd->doall || sd->replicate || sd->tosnap == NULL) { if (!sd->replicate && fromsnap_txg != 0) min_txg = fromsnap_txg; @@ -596,26 +594,26 @@ send_iterate_fs(zfs_handle_t *zhp, void *arg) (void) zfs_iter_snapshots_sorted(zhp, send_iterate_snap, sd, min_txg, max_txg); } else { - char snapname[MAXPATHLEN] = { 0 }; + char snapname[MAXPATHLEN]; zfs_handle_t *snap; (void) snprintf(snapname, sizeof (snapname), "%s@%s", zhp->zfs_name, sd->tosnap); if (sd->fromsnap != NULL) sd->seenfrom = B_TRUE; - snap = zfs_open(zhp->zfs_hdl, snapname, - ZFS_TYPE_SNAPSHOT); + snap = zfs_open(zhp->zfs_hdl, snapname, ZFS_TYPE_SNAPSHOT); if (snap != NULL) (void) send_iterate_snap(snap, sd); } fnvlist_add_nvlist(nvfs, "snaps", sd->parent_snaps); - fnvlist_add_nvlist(nvfs, "snapprops", sd->snapprops); - if (sd->holds) - fnvlist_add_nvlist(nvfs, "snapholds", sd->snapholds); fnvlist_free(sd->parent_snaps); + fnvlist_add_nvlist(nvfs, "snapprops", sd->snapprops); fnvlist_free(sd->snapprops); - fnvlist_free(sd->snapholds); + if (sd->holds) { + fnvlist_add_nvlist(nvfs, "snapholds", sd->snapholds); + fnvlist_free(sd->snapholds); + } /* Do not allow the size of the properties list to exceed the limit */ if ((fnvlist_size(nvfs) + fnvlist_size(sd->fss)) > @@ -629,19 +627,21 @@ send_iterate_fs(zfs_handle_t *zhp, void *arg) rv = EZFS_NOSPC; goto out; } - /* add this fs to nvlist */ + /* Add this fs to nvlist. */ (void) snprintf(guidstring, sizeof (guidstring), "0x%llx", (longlong_t)guid); fnvlist_add_nvlist(sd->fss, guidstring, nvfs); - /* iterate over children */ + /* Iterate over children. */ if (sd->recursive) rv = zfs_iter_filesystems(zhp, send_iterate_fs, sd); out: + /* Restore saved fields. */ sd->parent_fromsnap_guid = parent_fromsnap_guid_save; sd->fromsnap_txg = fromsnap_txg_save; sd->tosnap_txg = tosnap_txg_save; + fnvlist_free(nv); fnvlist_free(nvfs); From 3374ce5ba82f3a660fe34c1f02843002b7d30962 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Thu, 5 Aug 2021 15:28:32 +0000 Subject: [PATCH 08/19] libzfs_sendrecv: Style pass on zfs_send_space * Reduce indentation. * Move locals closer to use. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 77 +++++++++++++++++------------------- 1 file changed, 37 insertions(+), 40 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index f795ef89f04f..32030748b79b 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -726,53 +726,50 @@ static int zfs_send_space(zfs_handle_t *zhp, const char *snapname, const char *from, enum lzc_send_flags flags, uint64_t *spacep) { - libzfs_handle_t *hdl = zhp->zfs_hdl; - int error; - assert(snapname != NULL); - error = lzc_send_space(snapname, from, flags, spacep); - if (error != 0) { - char errbuf[1024]; - (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN, - "warning: cannot estimate space for '%s'"), snapname); - - switch (error) { - case EXDEV: - zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, - "not an earlier snapshot from the same fs")); - return (zfs_error(hdl, EZFS_CROSSTARGET, errbuf)); + int error = lzc_send_space(snapname, from, flags, spacep); + if (error == 0) + return (0); - case ENOENT: - if (zfs_dataset_exists(hdl, snapname, - ZFS_TYPE_SNAPSHOT)) { - zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, - "incremental source (%s) does not exist"), - snapname); - } - return (zfs_error(hdl, EZFS_NOENT, errbuf)); + char errbuf[1024]; + (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN, + "warning: cannot estimate space for '%s'"), snapname); - case EDQUOT: - case EFBIG: - case EIO: - case ENOLINK: - case ENOSPC: - case ENOSTR: - case ENXIO: - case EPIPE: - case ERANGE: - case EFAULT: - case EROFS: - case EINVAL: - zfs_error_aux(hdl, "%s", strerror(error)); - return (zfs_error(hdl, EZFS_BADBACKUP, errbuf)); + libzfs_handle_t *hdl = zhp->zfs_hdl; + switch (error) { + case EXDEV: + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "not an earlier snapshot from the same fs")); + return (zfs_error(hdl, EZFS_CROSSTARGET, errbuf)); - default: - return (zfs_standard_error(hdl, error, errbuf)); + case ENOENT: + if (zfs_dataset_exists(hdl, snapname, + ZFS_TYPE_SNAPSHOT)) { + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "incremental source (%s) does not exist"), + snapname); } - } + return (zfs_error(hdl, EZFS_NOENT, errbuf)); - return (0); + case EDQUOT: + case EFBIG: + case EIO: + case ENOLINK: + case ENOSPC: + case ENOSTR: + case ENXIO: + case EPIPE: + case ERANGE: + case EFAULT: + case EROFS: + case EINVAL: + zfs_error_aux(hdl, "%s", strerror(error)); + return (zfs_error(hdl, EZFS_BADBACKUP, errbuf)); + + default: + return (zfs_standard_error(hdl, error, errbuf)); + } } /* From c7f05a332dfbf50fac2595d4820a3455f566428c Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Thu, 5 Aug 2021 15:29:24 +0000 Subject: [PATCH 09/19] libzfs_sendrecv: Style pass on dump_ioctl * Don't bother building a debug nvlist if we can't return it. * Save errno after ioctl failure in case snprintf clobbers it. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 32030748b79b..a724c30d439f 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -795,23 +795,26 @@ dump_ioctl(zfs_handle_t *zhp, const char *fromsnap, uint64_t fromsnap_obj, zc.zc_fromobj = fromsnap_obj; zc.zc_flags = flags; - thisdbg = fnvlist_alloc(); - if (fromsnap && fromsnap[0] != '\0') { - fnvlist_add_string(thisdbg, "fromsnap", fromsnap); + if (debugnv != NULL) { + thisdbg = fnvlist_alloc(); + if (fromsnap != NULL && fromsnap[0] != '\0') + fnvlist_add_string(thisdbg, "fromsnap", fromsnap); } if (zfs_ioctl(zhp->zfs_hdl, ZFS_IOC_SEND, &zc) != 0) { char errbuf[1024]; + int error = errno; + (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN, "warning: cannot send '%s'"), zhp->zfs_name); - fnvlist_add_uint64(thisdbg, "error", errno); - if (debugnv) { + if (debugnv != NULL) { + fnvlist_add_uint64(thisdbg, "error", error); fnvlist_add_nvlist(debugnv, zhp->zfs_name, thisdbg); + fnvlist_free(thisdbg); } - fnvlist_free(thisdbg); - switch (errno) { + switch (error) { case EXDEV: zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, "not an earlier snapshot from the same fs")); @@ -851,9 +854,10 @@ dump_ioctl(zfs_handle_t *zhp, const char *fromsnap, uint64_t fromsnap_obj, } } - if (debugnv) + if (debugnv != NULL) { fnvlist_add_nvlist(debugnv, zhp->zfs_name, thisdbg); - fnvlist_free(thisdbg); + fnvlist_free(thisdbg); + } return (0); } From 1b3c58c57a2722812cbd64acc37068f584fa7649 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Thu, 5 Aug 2021 15:32:57 +0000 Subject: [PATCH 10/19] libzfs_sendrecv: Initialize in case of failure In zfs_send_progress, initialize \*bytes_written and \*blocks_visited in case we have to return early due to ioctl failure. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index a724c30d439f..e595a6877255 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -883,6 +883,10 @@ zfs_send_progress(zfs_handle_t *zhp, int fd, uint64_t *bytes_written, { zfs_cmd_t zc = {"\0"}; + if (bytes_written != NULL) + *bytes_written = 0; + if (blocks_visited != NULL) + *blocks_visited = 0; (void) strlcpy(zc.zc_name, zhp->zfs_name, sizeof (zc.zc_name)); zc.zc_cookie = fd; if (zfs_ioctl(zhp->zfs_hdl, ZFS_IOC_SEND_PROGRESS, &zc) != 0) From d44e6826ee61afba0250bdeefa437c64dd9df2b0 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Thu, 12 Aug 2021 16:48:56 +0000 Subject: [PATCH 11/19] libzfs_sendrecv: Pull header line out of loop This makes the header print before the sleep as well, which is fine. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index e595a6877255..67f6c912dc39 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -908,13 +908,20 @@ send_progress_thread(void *arg) char buf[16]; time_t t; struct tm *tm; - boolean_t firstloop = B_TRUE; + int err; + + if (!pa->pa_parsable) { + (void) fprintf(stderr, + "TIME %s %sSNAPSHOT %s\n", + pa->pa_estimate ? "BYTES" : " SENT", + pa->pa_verbosity >= 2 ? " BLOCKS " : "", + zhp->zfs_name); + } /* * Print the progress from ZFS_IOC_SEND_PROGRESS every second. */ for (;;) { - int err; (void) sleep(1); if ((err = zfs_send_progress(zhp, pa->pa_fd, &bytes, &blocks)) != 0) { @@ -923,15 +930,6 @@ send_progress_thread(void *arg) return ((void *)(uintptr_t)err); } - if (firstloop && !pa->pa_parsable) { - (void) fprintf(stderr, - "TIME %s %sSNAPSHOT %s\n", - pa->pa_estimate ? "BYTES" : " SENT", - pa->pa_verbosity >= 2 ? " BLOCKS " : "", - zhp->zfs_name); - firstloop = B_FALSE; - } - (void) time(&t); tm = localtime(&t); From b869715c6693cdd93643559818017af75bfe297f Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Thu, 12 Aug 2021 16:50:50 +0000 Subject: [PATCH 12/19] libzfs_sendrecv: Style pass on send_print_verbose * Add missing dgettext calls. * Avoid unnecessary line wraps. * Factor out duplicated parsable check. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 67f6c912dc39..07fb04bb546f 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -964,39 +964,33 @@ send_print_verbose(FILE *fout, const char *tosnap, const char *fromsnap, { if (parsable) { if (fromsnap != NULL) { - (void) fprintf(fout, "incremental\t%s\t%s", - fromsnap, tosnap); + (void) fprintf(fout, dgettext(TEXT_DOMAIN, + "incremental\t%s\t%s"), fromsnap, tosnap); } else { - (void) fprintf(fout, "full\t%s", - tosnap); + (void) fprintf(fout, dgettext(TEXT_DOMAIN, + "full\t%s"), tosnap); } + (void) fprintf(fout, "\t%llu", (longlong_t)size); } else { if (fromsnap != NULL) { if (strchr(fromsnap, '@') == NULL && strchr(fromsnap, '#') == NULL) { (void) fprintf(fout, dgettext(TEXT_DOMAIN, - "send from @%s to %s"), - fromsnap, tosnap); + "send from @%s to %s"), fromsnap, tosnap); } else { (void) fprintf(fout, dgettext(TEXT_DOMAIN, - "send from %s to %s"), - fromsnap, tosnap); + "send from %s to %s"), fromsnap, tosnap); } } else { (void) fprintf(fout, dgettext(TEXT_DOMAIN, - "full send of %s"), - tosnap); + "full send of %s"), tosnap); + } + if (size != 0) { + char buf[16]; + zfs_nicebytes(size, buf, sizeof (buf)); + (void) fprintf(fout, dgettext(TEXT_DOMAIN, + " estimated size is %s"), buf); } - } - - if (parsable) { - (void) fprintf(fout, "\t%llu", - (longlong_t)size); - } else if (size != 0) { - char buf[16]; - zfs_nicebytes(size, buf, sizeof (buf)); - (void) fprintf(fout, dgettext(TEXT_DOMAIN, - " estimated size is %s"), buf); } (void) fprintf(fout, "\n"); } From 86c4289c9c5b6cfacb8dbfe9cf84585e5e69fe95 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Wed, 18 Aug 2021 17:04:46 +0000 Subject: [PATCH 13/19] libzfs_sendrecv: Style pass on dump_snapshot * Add a high level comment. * Avoid unnecessary line wrapping. * Simplify size accounting logic. * Eliminate unnecessary buffer on the stack. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 07fb04bb546f..307c0982029a 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -995,6 +995,10 @@ send_print_verbose(FILE *fout, const char *tosnap, const char *fromsnap, (void) fprintf(fout, "\n"); } +/* + * Send a single filesystem snapshot, updating the send dump data. + * This interface is intended for use as a zfs_iter_snapshots_sorted visitor. + */ static int dump_snapshot(zfs_handle_t *zhp, void *arg) { @@ -1016,8 +1020,7 @@ dump_snapshot(zfs_handle_t *zhp, void *arg) if (!sdd->seenfrom && isfromsnap) { gather_holds(zhp, sdd); sdd->seenfrom = B_TRUE; - (void) strlcpy(sdd->prevsnap, thissnap, - sizeof (sdd->prevsnap)); + (void) strlcpy(sdd->prevsnap, thissnap, sizeof (sdd->prevsnap)); sdd->prevsnap_obj = zfs_prop_get_int(zhp, ZFS_PROP_OBJSETID); zfs_close(zhp); return (0); @@ -1097,14 +1100,12 @@ dump_snapshot(zfs_handle_t *zhp, void *arg) (void) strlcat(fromds, sdd->prevsnap, sizeof (fromds)); } if (zfs_send_space(zhp, zhp->zfs_name, - sdd->prevsnap[0] ? fromds : NULL, flags, &size) != 0) { - size = 0; /* cannot estimate send space */ - } else { + sdd->prevsnap[0] ? fromds : NULL, flags, &size) == 0) { send_print_verbose(fout, zhp->zfs_name, sdd->prevsnap[0] ? sdd->prevsnap : NULL, size, sdd->parsable); + sdd->size += size; } - sdd->size += size; } if (!sdd->dryrun) { @@ -1135,12 +1136,9 @@ dump_snapshot(zfs_handle_t *zhp, void *arg) (void) pthread_join(tid, &status); int error = (int)(uintptr_t)status; if (error != 0 && status != PTHREAD_CANCELED) { - char errbuf[1024]; - (void) snprintf(errbuf, sizeof (errbuf), - dgettext(TEXT_DOMAIN, - "progress thread exited nonzero")); return (zfs_standard_error(zhp->zfs_hdl, error, - errbuf)); + dgettext(TEXT_DOMAIN, + "progress thread exited nonzero"))); } } } From cb07e61f0593e6656345ba267dd285d6334c8ef3 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Wed, 18 Aug 2021 18:01:04 +0000 Subject: [PATCH 14/19] libzfs_sendrecv: Style pass on dump_filesystem * Add high level comments. * Eliminate unnecessarily void arg. * Avoid unnecessary line wrapping. * Initialize sdd fields with the correct types. * Remove extra whitespace. * Refactor replication checks for clarity. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 51 ++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 307c0982029a..b6f0a6b8dec2 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -1149,15 +1149,20 @@ dump_snapshot(zfs_handle_t *zhp, void *arg) return (err); } +/* + * Send all snapshots for a filesystem, updating the send dump data. + */ static int -dump_filesystem(zfs_handle_t *zhp, void *arg) +dump_filesystem(zfs_handle_t *zhp, send_dump_data_t *sdd) { int rv = 0; - send_dump_data_t *sdd = arg; boolean_t missingfrom = B_FALSE; zfs_cmd_t zc = {"\0"}; uint64_t min_txg = 0, max_txg = 0; + /* + * Make sure the tosnap exists. + */ (void) snprintf(zc.zc_name, sizeof (zc.zc_name), "%s@%s", zhp->zfs_name, sdd->tosnap); if (zfs_ioctl(zhp->zfs_hdl, ZFS_IOC_OBJSET_STATS, &zc) != 0) { @@ -1168,47 +1173,52 @@ dump_filesystem(zfs_handle_t *zhp, void *arg) return (0); } + /* + * If this fs does not have fromsnap, and we're doing + * recursive, we need to send a full stream from the + * beginning (or an incremental from the origin if this + * is a clone). If we're doing non-recursive, then let + * them get the error. + */ if (sdd->replicate && sdd->fromsnap) { /* - * If this fs does not have fromsnap, and we're doing - * recursive, we need to send a full stream from the - * beginning (or an incremental from the origin if this - * is a clone). If we're doing non-recursive, then let - * them get the error. + * Make sure the fromsnap exists. */ (void) snprintf(zc.zc_name, sizeof (zc.zc_name), "%s@%s", zhp->zfs_name, sdd->fromsnap); - if (zfs_ioctl(zhp->zfs_hdl, - ZFS_IOC_OBJSET_STATS, &zc) != 0) { + if (zfs_ioctl(zhp->zfs_hdl, ZFS_IOC_OBJSET_STATS, &zc) != 0) missingfrom = B_TRUE; - } } - sdd->seenfrom = sdd->seento = sdd->prevsnap[0] = 0; + sdd->seenfrom = sdd->seento = B_FALSE; + sdd->prevsnap[0] = '\0'; sdd->prevsnap_obj = 0; if (sdd->fromsnap == NULL || missingfrom) sdd->seenfrom = B_TRUE; - - /* * Iterate through all snapshots and process the ones we will be * sending. If we only have a "from" and "to" snapshot to deal * with, we can avoid iterating through all the other snapshots. */ if (sdd->doall || sdd->replicate || sdd->tosnap == NULL) { - if (!sdd->replicate && sdd->fromsnap != NULL) - min_txg = get_snap_txg(zhp->zfs_hdl, zhp->zfs_name, - sdd->fromsnap); - if (!sdd->replicate && sdd->tosnap != NULL) - max_txg = get_snap_txg(zhp->zfs_hdl, zhp->zfs_name, - sdd->tosnap); - rv = zfs_iter_snapshots_sorted(zhp, dump_snapshot, arg, + if (!sdd->replicate) { + if (sdd->fromsnap != NULL) { + min_txg = get_snap_txg(zhp->zfs_hdl, + zhp->zfs_name, sdd->fromsnap); + } + if (sdd->tosnap != NULL) { + max_txg = get_snap_txg(zhp->zfs_hdl, + zhp->zfs_name, sdd->tosnap); + } + } + rv = zfs_iter_snapshots_sorted(zhp, dump_snapshot, sdd, min_txg, max_txg); } else { char snapname[MAXPATHLEN] = { 0 }; zfs_handle_t *snap; + /* Dump fromsnap. */ if (!sdd->seenfrom) { (void) snprintf(snapname, sizeof (snapname), "%s@%s", zhp->zfs_name, sdd->fromsnap); @@ -1220,6 +1230,7 @@ dump_filesystem(zfs_handle_t *zhp, void *arg) rv = -1; } + /* Dump tosnap. */ if (rv == 0) { (void) snprintf(snapname, sizeof (snapname), "%s@%s", zhp->zfs_name, sdd->tosnap); From 5c7f3aa41abf56b03a6c39bf4e7c0a425f7f2652 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Wed, 18 Aug 2021 21:01:21 +0000 Subject: [PATCH 15/19] libzfs_sendrecv: Style pass on dump_filesystems * Add a high level comment. * Eliminate unnecessarily void arg. * Capitalize and punctuate complete sentences in comments. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index b6f0a6b8dec2..0e6083603147 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -1270,10 +1270,12 @@ dump_filesystem(zfs_handle_t *zhp, send_dump_data_t *sdd) return (rv); } +/* + * Send all snapshots for all filesystems in sdd. + */ static int -dump_filesystems(zfs_handle_t *rzhp, void *arg) +dump_filesystems(zfs_handle_t *rzhp, send_dump_data_t *sdd) { - send_dump_data_t *sdd = arg; nvpair_t *fspair; boolean_t needagain, progress; @@ -1326,7 +1328,7 @@ dump_filesystems(zfs_handle_t *rzhp, void *arg) if (parent_guid != 0) { parent_nv = fsavl_find(sdd->fsavl, parent_guid, NULL); if (!nvlist_exists(parent_nv, "sent")) { - /* parent has not been sent; skip this one */ + /* Parent has not been sent; skip this one. */ needagain = B_TRUE; continue; } @@ -1338,7 +1340,7 @@ dump_filesystems(zfs_handle_t *rzhp, void *arg) if (origin_nv != NULL && !nvlist_exists(origin_nv, "sent")) { /* - * origin has not been sent yet; + * Origin has not been sent yet; * skip this clone. */ needagain = B_TRUE; @@ -1361,7 +1363,7 @@ dump_filesystems(zfs_handle_t *rzhp, void *arg) goto again; } - /* clean out the sent flags in case we reuse this fss */ + /* Clean out the sent flags in case we reuse this fss. */ for (fspair = nvlist_next_nvpair(sdd->fss, NULL); fspair; fspair = nvlist_next_nvpair(sdd->fss, fspair)) { nvlist_t *fslist; From 3921beee6aabc14f4446ac8c05a9b42e06e338c0 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Mon, 13 Sep 2021 14:51:02 +0000 Subject: [PATCH 16/19] libzfs_sendrecv: Fix some comment style nits * Capitalize and punctuate complete sentences. * Add a blank line between functions. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 0e6083603147..66019f48a084 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -1402,7 +1402,7 @@ zfs_send_resume_token_to_nvlist(libzfs_handle_t *hdl, const char *token) return (NULL); } - /* convert hexadecimal representation to binary */ + /* Convert hexadecimal representation to binary. */ token = strrchr(token, '-') + 1; int len = strlen(token) / 2; unsigned char *compressed = zfs_alloc(hdl, len); @@ -1417,7 +1417,7 @@ zfs_send_resume_token_to_nvlist(libzfs_handle_t *hdl, const char *token) } } - /* verify checksum */ + /* Verify checksum. */ zio_cksum_t cksum; fletcher_4_native_varsize(compressed, len, &cksum); if (cksum.zc_word[0] != checksum) { @@ -1427,7 +1427,7 @@ zfs_send_resume_token_to_nvlist(libzfs_handle_t *hdl, const char *token) return (NULL); } - /* uncompress */ + /* Uncompress. */ void *packed = zfs_alloc(hdl, packed_len); uLongf packed_len_long = packed_len; if (uncompress(packed, &packed_len_long, compressed, len) != Z_OK || @@ -1439,7 +1439,7 @@ zfs_send_resume_token_to_nvlist(libzfs_handle_t *hdl, const char *token) return (NULL); } - /* unpack nvlist */ + /* Unpack nvlist. */ nvlist_t *nv; int error = nvlist_unpack(packed, packed_len, &nv, KM_SLEEP); free(packed); @@ -1451,6 +1451,7 @@ zfs_send_resume_token_to_nvlist(libzfs_handle_t *hdl, const char *token) } return (nv); } + static enum lzc_send_flags lzc_flags_from_sendflags(const sendflags_t *flags) { From c0714243701051b5e3f3f44a496ac467879266cc Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Mon, 13 Sep 2021 15:10:56 +0000 Subject: [PATCH 17/19] libzfs_sendrecv: Refactor find_redact_book Factor out get_bookmarks, find_redact_pair, and get_redact_complete helper functions to improve the readability of find_redact_book. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 79 ++++++++++++++++++++++++------------ 1 file changed, 52 insertions(+), 27 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 66019f48a084..9b9ee6571fad 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -1557,6 +1557,53 @@ redact_snaps_equal(const uint64_t *snaps1, uint64_t num_snaps1, return (B_TRUE); } +static int +get_bookmarks(const char *path, nvlist_t **bmarksp) +{ + nvlist_t *props = fnvlist_alloc(); + int error; + + fnvlist_add_boolean(props, "redact_complete"); + fnvlist_add_boolean(props, zfs_prop_to_name(ZFS_PROP_REDACT_SNAPS)); + error = lzc_get_bookmarks(path, props, bmarksp); + fnvlist_free(props); + return (error); +} + +static nvpair_t * +find_redact_pair(nvlist_t *bmarks, const uint64_t *redact_snap_guids, + int num_redact_snaps) +{ + nvpair_t *pair; + + for (pair = nvlist_next_nvpair(bmarks, NULL); pair; + pair = nvlist_next_nvpair(bmarks, pair)) { + + nvlist_t *bmark = fnvpair_value_nvlist(pair); + nvlist_t *vallist = fnvlist_lookup_nvlist(bmark, + zfs_prop_to_name(ZFS_PROP_REDACT_SNAPS)); + uint_t len = 0; + uint64_t *bmarksnaps = fnvlist_lookup_uint64_array(vallist, + ZPROP_VALUE, &len); + if (redact_snaps_equal(redact_snap_guids, + num_redact_snaps, bmarksnaps, len)) { + break; + } + } + return (pair); +} + +static boolean_t +get_redact_complete(nvpair_t *pair) +{ + nvlist_t *bmark = fnvpair_value_nvlist(pair); + nvlist_t *vallist = fnvlist_lookup_nvlist(bmark, "redact_complete"); + boolean_t complete = fnvlist_lookup_boolean_value(vallist, + ZPROP_VALUE); + + return (complete); +} + /* * Check that the list of redaction snapshots in the bookmark matches the send * we're resuming, and return whether or not it's complete. @@ -1570,17 +1617,12 @@ find_redact_book(libzfs_handle_t *hdl, const char *path, char **bookname) { char errbuf[1024]; - int error = 0; - nvlist_t *props = fnvlist_alloc(); nvlist_t *bmarks; (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN, "cannot resume send")); - fnvlist_add_boolean(props, "redact_complete"); - fnvlist_add_boolean(props, zfs_prop_to_name(ZFS_PROP_REDACT_SNAPS)); - error = lzc_get_bookmarks(path, props, &bmarks); - fnvlist_free(props); + int error = get_bookmarks(path, &bmarks); if (error != 0) { if (error == ESRCH) { zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, @@ -1594,39 +1636,22 @@ find_redact_book(libzfs_handle_t *hdl, const char *path, } return (zfs_error(hdl, EZFS_BADPROP, errbuf)); } - nvpair_t *pair; - for (pair = nvlist_next_nvpair(bmarks, NULL); pair; - pair = nvlist_next_nvpair(bmarks, pair)) { - - nvlist_t *bmark = fnvpair_value_nvlist(pair); - nvlist_t *vallist = fnvlist_lookup_nvlist(bmark, - zfs_prop_to_name(ZFS_PROP_REDACT_SNAPS)); - uint_t len = 0; - uint64_t *bmarksnaps = fnvlist_lookup_uint64_array(vallist, - ZPROP_VALUE, &len); - if (redact_snaps_equal(redact_snap_guids, - num_redact_snaps, bmarksnaps, len)) { - break; - } - } + nvpair_t *pair = find_redact_pair(bmarks, redact_snap_guids, + num_redact_snaps); if (pair == NULL) { fnvlist_free(bmarks); zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, "no appropriate redaction bookmark exists")); return (zfs_error(hdl, EZFS_BADPROP, errbuf)); } - char *name = nvpair_name(pair); - nvlist_t *bmark = fnvpair_value_nvlist(pair); - nvlist_t *vallist = fnvlist_lookup_nvlist(bmark, "redact_complete"); - boolean_t complete = fnvlist_lookup_boolean_value(vallist, - ZPROP_VALUE); + boolean_t complete = get_redact_complete(pair); if (!complete) { fnvlist_free(bmarks); zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, "incomplete redaction bookmark provided")); return (zfs_error(hdl, EZFS_BADPROP, errbuf)); } - *bookname = strndup(name, ZFS_MAX_DATASET_NAME_LEN); + *bookname = strndup(nvpair_name(pair), ZFS_MAX_DATASET_NAME_LEN); ASSERT3P(*bookname, !=, NULL); fnvlist_free(bmarks); return (0); From e5c233a6e10eb521393b00c0c84c3b14a6adc6f9 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Mon, 13 Sep 2021 15:11:43 +0000 Subject: [PATCH 18/19] libzfs_sendrecv: Factor out lzc_flags_from_resume_nvl Improve the readability of zfs_send_resume_impl by moving resume nvl decoding into a separate helper function. Signed-off-by: Ryan Moeller --- lib/libzfs/libzfs_sendrecv.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 9b9ee6571fad..f31669fab2c7 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -1456,6 +1456,7 @@ static enum lzc_send_flags lzc_flags_from_sendflags(const sendflags_t *flags) { enum lzc_send_flags lzc_flags = 0; + if (flags->largeblock) lzc_flags |= LZC_SEND_FLAG_LARGE_BLOCK; if (flags->embed_data) @@ -1466,6 +1467,7 @@ lzc_flags_from_sendflags(const sendflags_t *flags) lzc_flags |= LZC_SEND_FLAG_RAW; if (flags->saved) lzc_flags |= LZC_SEND_FLAG_SAVED; + return (lzc_flags); } @@ -1657,6 +1659,25 @@ find_redact_book(libzfs_handle_t *hdl, const char *path, return (0); } +static enum lzc_send_flags +lzc_flags_from_resume_nvl(nvlist_t *resume_nvl) +{ + enum lzc_send_flags lzc_flags = 0; + + if (nvlist_exists(resume_nvl, "largeblockok")) + lzc_flags |= LZC_SEND_FLAG_LARGE_BLOCK; + if (nvlist_exists(resume_nvl, "embedok")) + lzc_flags |= LZC_SEND_FLAG_EMBED_DATA; + if (nvlist_exists(resume_nvl, "compressok")) + lzc_flags |= LZC_SEND_FLAG_COMPRESS; + if (nvlist_exists(resume_nvl, "rawok")) + lzc_flags |= LZC_SEND_FLAG_RAW; + if (nvlist_exists(resume_nvl, "savedok")) + lzc_flags |= LZC_SEND_FLAG_SAVED; + + return (lzc_flags); +} + static int zfs_send_resume_impl(libzfs_handle_t *hdl, sendflags_t *flags, int outfd, nvlist_t *resume_nvl) @@ -1668,7 +1689,6 @@ zfs_send_resume_impl(libzfs_handle_t *hdl, sendflags_t *flags, int outfd, zfs_handle_t *zhp; int error = 0; char name[ZFS_MAX_DATASET_NAME_LEN]; - enum lzc_send_flags lzc_flags = 0; FILE *fout = (flags->verbosity > 0 && flags->dryrun) ? stdout : stderr; uint64_t *redact_snap_guids = NULL; int num_redact_snaps = 0; @@ -1695,17 +1715,6 @@ zfs_send_resume_impl(libzfs_handle_t *hdl, sendflags_t *flags, int outfd, fromguid = 0; (void) nvlist_lookup_uint64(resume_nvl, "fromguid", &fromguid); - if (flags->largeblock || nvlist_exists(resume_nvl, "largeblockok")) - lzc_flags |= LZC_SEND_FLAG_LARGE_BLOCK; - if (flags->embed_data || nvlist_exists(resume_nvl, "embedok")) - lzc_flags |= LZC_SEND_FLAG_EMBED_DATA; - if (flags->compress || nvlist_exists(resume_nvl, "compressok")) - lzc_flags |= LZC_SEND_FLAG_COMPRESS; - if (flags->raw || nvlist_exists(resume_nvl, "rawok")) - lzc_flags |= LZC_SEND_FLAG_RAW; - if (flags->saved || nvlist_exists(resume_nvl, "savedok")) - lzc_flags |= LZC_SEND_FLAG_SAVED; - if (flags->saved) { (void) strcpy(name, toname); } else { @@ -1766,6 +1775,9 @@ zfs_send_resume_impl(libzfs_handle_t *hdl, sendflags_t *flags, int outfd, } } + enum lzc_send_flags lzc_flags = lzc_flags_from_sendflags(flags) | + lzc_flags_from_resume_nvl(resume_nvl); + if (flags->verbosity != 0) { /* * Some of these may have come from the resume token, set them From 49c50ac4cab26750fa225a40fb3afd1e29b81d1f Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Mon, 8 Nov 2021 22:35:05 +0000 Subject: [PATCH 19/19] Simplify resume token generation * Improve naming. * Reduce indentation. * Avoid boilerplate logic duplication. Signed-off-by: Ryan Moeller --- include/sys/dsl_dataset.h | 3 +- module/zfs/dsl_dataset.c | 301 +++++++++++++++++--------------------- module/zfs/zcp_get.c | 18 +-- 3 files changed, 142 insertions(+), 180 deletions(-) diff --git a/include/sys/dsl_dataset.h b/include/sys/dsl_dataset.h index 29bbf7e1868a..02147171ae14 100644 --- a/include/sys/dsl_dataset.h +++ b/include/sys/dsl_dataset.h @@ -385,8 +385,7 @@ int dsl_dataset_snap_lookup(dsl_dataset_t *ds, const char *name, void dsl_dataset_dirty(dsl_dataset_t *ds, dmu_tx_t *tx); int get_clones_stat_impl(dsl_dataset_t *ds, nvlist_t *val); -char *get_receive_resume_stats_impl(dsl_dataset_t *ds); -char *get_child_receive_stats(dsl_dataset_t *ds); +char *get_receive_resume_token(dsl_dataset_t *ds); uint64_t dsl_get_refratio(dsl_dataset_t *ds); uint64_t dsl_get_logicalreferenced(dsl_dataset_t *ds); uint64_t dsl_get_compressratio(dsl_dataset_t *ds); diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index 115f3df5d539..415c3b747706 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -2331,161 +2331,147 @@ get_clones_stat(dsl_dataset_t *ds, nvlist_t *nv) nvlist_free(propval); } -/* - * Returns a string that represents the receive resume stats token. It should - * be freed with strfree(). - */ -char * -get_receive_resume_stats_impl(dsl_dataset_t *ds) +static char * +get_receive_resume_token_impl(dsl_dataset_t *ds) { + if (!dsl_dataset_has_resume_receive_state(ds)) + return (NULL); + dsl_pool_t *dp = ds->ds_dir->dd_pool; + char *str; + void *packed; + uint8_t *compressed; + uint64_t val; + nvlist_t *token_nv = fnvlist_alloc(); + size_t packed_size, compressed_size; + + if (zap_lookup(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_FROMGUID, sizeof (val), 1, &val) == 0) { + fnvlist_add_uint64(token_nv, "fromguid", val); + } + if (zap_lookup(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_OBJECT, sizeof (val), 1, &val) == 0) { + fnvlist_add_uint64(token_nv, "object", val); + } + if (zap_lookup(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_OFFSET, sizeof (val), 1, &val) == 0) { + fnvlist_add_uint64(token_nv, "offset", val); + } + if (zap_lookup(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_BYTES, sizeof (val), 1, &val) == 0) { + fnvlist_add_uint64(token_nv, "bytes", val); + } + if (zap_lookup(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_TOGUID, sizeof (val), 1, &val) == 0) { + fnvlist_add_uint64(token_nv, "toguid", val); + } + char buf[MAXNAMELEN]; + if (zap_lookup(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_TONAME, 1, sizeof (buf), buf) == 0) { + fnvlist_add_string(token_nv, "toname", buf); + } + if (zap_contains(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_LARGEBLOCK) == 0) { + fnvlist_add_boolean(token_nv, "largeblockok"); + } + if (zap_contains(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_EMBEDOK) == 0) { + fnvlist_add_boolean(token_nv, "embedok"); + } + if (zap_contains(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_COMPRESSOK) == 0) { + fnvlist_add_boolean(token_nv, "compressok"); + } + if (zap_contains(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_RAWOK) == 0) { + fnvlist_add_boolean(token_nv, "rawok"); + } + if (dsl_dataset_feature_is_active(ds, + SPA_FEATURE_REDACTED_DATASETS)) { + uint64_t num_redact_snaps; + uint64_t *redact_snaps; + VERIFY(dsl_dataset_get_uint64_array_feature(ds, + SPA_FEATURE_REDACTED_DATASETS, &num_redact_snaps, + &redact_snaps)); + fnvlist_add_uint64_array(token_nv, "redact_snaps", + redact_snaps, num_redact_snaps); + } + if (zap_contains(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_REDACT_BOOKMARK_SNAPS) == 0) { + uint64_t num_redact_snaps, int_size; + uint64_t *redact_snaps; + VERIFY0(zap_length(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_REDACT_BOOKMARK_SNAPS, &int_size, + &num_redact_snaps)); + ASSERT3U(int_size, ==, sizeof (uint64_t)); - if (dsl_dataset_has_resume_receive_state(ds)) { - char *str; - void *packed; - uint8_t *compressed; - uint64_t val; - nvlist_t *token_nv = fnvlist_alloc(); - size_t packed_size, compressed_size; - - if (zap_lookup(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_FROMGUID, sizeof (val), 1, &val) == 0) { - fnvlist_add_uint64(token_nv, "fromguid", val); - } - if (zap_lookup(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_OBJECT, sizeof (val), 1, &val) == 0) { - fnvlist_add_uint64(token_nv, "object", val); - } - if (zap_lookup(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_OFFSET, sizeof (val), 1, &val) == 0) { - fnvlist_add_uint64(token_nv, "offset", val); - } - if (zap_lookup(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_BYTES, sizeof (val), 1, &val) == 0) { - fnvlist_add_uint64(token_nv, "bytes", val); - } - if (zap_lookup(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_TOGUID, sizeof (val), 1, &val) == 0) { - fnvlist_add_uint64(token_nv, "toguid", val); - } - char buf[MAXNAMELEN]; - if (zap_lookup(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_TONAME, 1, sizeof (buf), buf) == 0) { - fnvlist_add_string(token_nv, "toname", buf); - } - if (zap_contains(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_LARGEBLOCK) == 0) { - fnvlist_add_boolean(token_nv, "largeblockok"); - } - if (zap_contains(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_EMBEDOK) == 0) { - fnvlist_add_boolean(token_nv, "embedok"); - } - if (zap_contains(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_COMPRESSOK) == 0) { - fnvlist_add_boolean(token_nv, "compressok"); - } - if (zap_contains(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_RAWOK) == 0) { - fnvlist_add_boolean(token_nv, "rawok"); - } - if (dsl_dataset_feature_is_active(ds, - SPA_FEATURE_REDACTED_DATASETS)) { - uint64_t num_redact_snaps; - uint64_t *redact_snaps; - VERIFY(dsl_dataset_get_uint64_array_feature(ds, - SPA_FEATURE_REDACTED_DATASETS, &num_redact_snaps, - &redact_snaps)); - fnvlist_add_uint64_array(token_nv, "redact_snaps", - redact_snaps, num_redact_snaps); - } - if (zap_contains(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_REDACT_BOOKMARK_SNAPS) == 0) { - uint64_t num_redact_snaps, int_size; - uint64_t *redact_snaps; - VERIFY0(zap_length(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_REDACT_BOOKMARK_SNAPS, &int_size, - &num_redact_snaps)); - ASSERT3U(int_size, ==, sizeof (uint64_t)); - - redact_snaps = kmem_alloc(int_size * num_redact_snaps, - KM_SLEEP); - VERIFY0(zap_lookup(dp->dp_meta_objset, ds->ds_object, - DS_FIELD_RESUME_REDACT_BOOKMARK_SNAPS, int_size, - num_redact_snaps, redact_snaps)); - fnvlist_add_uint64_array(token_nv, "book_redact_snaps", - redact_snaps, num_redact_snaps); - kmem_free(redact_snaps, int_size * num_redact_snaps); - } - packed = fnvlist_pack(token_nv, &packed_size); - fnvlist_free(token_nv); - compressed = kmem_alloc(packed_size, KM_SLEEP); - - compressed_size = gzip_compress(packed, compressed, - packed_size, packed_size, 6); - - zio_cksum_t cksum; - fletcher_4_native_varsize(compressed, compressed_size, &cksum); - - size_t alloc_size = compressed_size * 2 + 1; - str = kmem_alloc(alloc_size, KM_SLEEP); - for (int i = 0; i < compressed_size; i++) { - size_t offset = i * 2; - (void) snprintf(str + offset, alloc_size - offset, - "%02x", compressed[i]); - } - str[compressed_size * 2] = '\0'; - char *propval = kmem_asprintf("%u-%llx-%llx-%s", - ZFS_SEND_RESUME_TOKEN_VERSION, - (longlong_t)cksum.zc_word[0], - (longlong_t)packed_size, str); - kmem_free(packed, packed_size); - kmem_free(str, alloc_size); - kmem_free(compressed, packed_size); - return (propval); - } - return (kmem_strdup("")); + redact_snaps = kmem_alloc(int_size * num_redact_snaps, + KM_SLEEP); + VERIFY0(zap_lookup(dp->dp_meta_objset, ds->ds_object, + DS_FIELD_RESUME_REDACT_BOOKMARK_SNAPS, int_size, + num_redact_snaps, redact_snaps)); + fnvlist_add_uint64_array(token_nv, "book_redact_snaps", + redact_snaps, num_redact_snaps); + kmem_free(redact_snaps, int_size * num_redact_snaps); + } + packed = fnvlist_pack(token_nv, &packed_size); + fnvlist_free(token_nv); + compressed = kmem_alloc(packed_size, KM_SLEEP); + + compressed_size = gzip_compress(packed, compressed, + packed_size, packed_size, 6); + + zio_cksum_t cksum; + fletcher_4_native_varsize(compressed, compressed_size, &cksum); + + size_t alloc_size = compressed_size * 2 + 1; + str = kmem_alloc(alloc_size, KM_SLEEP); + for (int i = 0; i < compressed_size; i++) { + size_t offset = i * 2; + (void) snprintf(str + offset, alloc_size - offset, + "%02x", compressed[i]); + } + str[compressed_size * 2] = '\0'; + char *propval = kmem_asprintf("%u-%llx-%llx-%s", + ZFS_SEND_RESUME_TOKEN_VERSION, + (longlong_t)cksum.zc_word[0], + (longlong_t)packed_size, str); + kmem_free(packed, packed_size); + kmem_free(str, alloc_size); + kmem_free(compressed, packed_size); + return (propval); } /* - * Returns a string that represents the receive resume stats token of the - * dataset's child. It should be freed with strfree(). + * Returns a string that represents the receive resume state token. It should + * be freed with strfree(). NULL is returned if no resume state is present. */ char * -get_child_receive_stats(dsl_dataset_t *ds) +get_receive_resume_token(dsl_dataset_t *ds) { - char recvname[ZFS_MAX_DATASET_NAME_LEN + 6]; + /* + * A failed "newfs" (e.g. full) resumable receive leaves + * the stats set on this dataset. Check here for the prop. + */ + char *token = get_receive_resume_token_impl(ds); + if (token != NULL) + return (token); + /* + * A failed incremental resumable receive leaves the + * stats set on our child named "%recv". Check the child + * for the prop. + */ + /* 6 extra bytes for /%recv */ + char name[ZFS_MAX_DATASET_NAME_LEN + 6]; dsl_dataset_t *recv_ds; - dsl_dataset_name(ds, recvname); - if (strlcat(recvname, "/", sizeof (recvname)) < - sizeof (recvname) && - strlcat(recvname, recv_clone_name, sizeof (recvname)) < - sizeof (recvname) && - dsl_dataset_hold(ds->ds_dir->dd_pool, recvname, FTAG, - &recv_ds) == 0) { - char *propval = get_receive_resume_stats_impl(recv_ds); + dsl_dataset_name(ds, name); + if (strlcat(name, "/", sizeof (name)) < sizeof (name) && + strlcat(name, recv_clone_name, sizeof (name)) < sizeof (name) && + dsl_dataset_hold(ds->ds_dir->dd_pool, name, FTAG, &recv_ds) == 0) { + token = get_receive_resume_token_impl(recv_ds); dsl_dataset_rele(recv_ds, FTAG); - return (propval); - } - return (kmem_strdup("")); -} - -static void -get_receive_resume_stats(dsl_dataset_t *ds, nvlist_t *nv) -{ - char *propval = get_receive_resume_stats_impl(ds); - if (strcmp(propval, "") != 0) { - dsl_prop_nvlist_add_string(nv, - ZFS_PROP_RECEIVE_RESUME_TOKEN, propval); - } else { - char *childval = get_child_receive_stats(ds); - if (strcmp(childval, "") != 0) { - dsl_prop_nvlist_add_string(nv, - ZFS_PROP_RECEIVE_RESUME_TOKEN, childval); - } - kmem_strfree(childval); } - kmem_strfree(propval); + return (token); } uint64_t @@ -2761,7 +2747,7 @@ dsl_get_mountpoint(dsl_dataset_t *ds, const char *dsname, char *value, void dsl_dataset_stats(dsl_dataset_t *ds, nvlist_t *nv) { - dsl_pool_t *dp = ds->ds_dir->dd_pool; + dsl_pool_t *dp __maybe_unused = ds->ds_dir->dd_pool; ASSERT(dsl_pool_config_held(dp)); @@ -2823,28 +2809,11 @@ dsl_dataset_stats(dsl_dataset_t *ds, nvlist_t *nv) } if (!dsl_dataset_is_snapshot(ds)) { - /* - * A failed "newfs" (e.g. full) resumable receive leaves - * the stats set on this dataset. Check here for the prop. - */ - get_receive_resume_stats(ds, nv); - - /* - * A failed incremental resumable receive leaves the - * stats set on our child named "%recv". Check the child - * for the prop. - */ - /* 6 extra bytes for /%recv */ - char recvname[ZFS_MAX_DATASET_NAME_LEN + 6]; - dsl_dataset_t *recv_ds; - dsl_dataset_name(ds, recvname); - if (strlcat(recvname, "/", sizeof (recvname)) < - sizeof (recvname) && - strlcat(recvname, recv_clone_name, sizeof (recvname)) < - sizeof (recvname) && - dsl_dataset_hold(dp, recvname, FTAG, &recv_ds) == 0) { - get_receive_resume_stats(recv_ds, nv); - dsl_dataset_rele(recv_ds, FTAG); + char *token = get_receive_resume_token(ds); + if (token != NULL) { + dsl_prop_nvlist_add_string(nv, + ZFS_PROP_RECEIVE_RESUME_TOKEN, token); + kmem_strfree(token); } } } diff --git a/module/zfs/zcp_get.c b/module/zfs/zcp_get.c index fe712afd7ade..eb2c606163e1 100644 --- a/module/zfs/zcp_get.c +++ b/module/zfs/zcp_get.c @@ -344,19 +344,13 @@ get_special_prop(lua_State *state, dsl_dataset_t *ds, const char *dsname, } break; case ZFS_PROP_RECEIVE_RESUME_TOKEN: { - char *token = get_receive_resume_stats_impl(ds); - - (void) strlcpy(strval, token, ZAP_MAXVALUELEN); - if (strcmp(strval, "") == 0) { - char *childval = get_child_receive_stats(ds); - - (void) strlcpy(strval, childval, ZAP_MAXVALUELEN); - if (strcmp(strval, "") == 0) - error = ENOENT; - - kmem_strfree(childval); + char *token = get_receive_resume_token(ds); + if (token != NULL) { + (void) strlcpy(strval, token, ZAP_MAXVALUELEN); + kmem_strfree(token); + } else { + error = ENOENT; } - kmem_strfree(token); break; } case ZFS_PROP_VOLSIZE: