From 70ac2654f579e138d04ed747967fb0d603b0cacb Mon Sep 17 00:00:00 2001 From: Marcel Menzel Date: Wed, 14 Dec 2022 02:26:10 +0100 Subject: [PATCH 1/8] Change ZEVENT_POOL_GUID to ZEVENT_POOL to display pool names Outgoing mails for ZFS pool events include the pool GUID, but not the actual pool name. Let's change this for better readability, as it is already done in the mails for finished pool resilvers. Reviewed-by: Brian Behlendorf Reviewed-by: Tony Hutter Reviewed-by: George Melikov Reviewed-by Richard Yao Signed-off-by: Marcel Menzel Closes #14272 --- cmd/zed/zed.d/statechange-notify.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/zed/zed.d/statechange-notify.sh b/cmd/zed/zed.d/statechange-notify.sh index c475fdb36660..ae610df20e45 100755 --- a/cmd/zed/zed.d/statechange-notify.sh +++ b/cmd/zed/zed.d/statechange-notify.sh @@ -38,7 +38,7 @@ if [ "${ZEVENT_VDEV_STATE_STR}" != "FAULTED" ] \ fi umask 077 -note_subject="ZFS device fault for pool ${ZEVENT_POOL_GUID} on $(hostname)" +note_subject="ZFS device fault for pool ${ZEVENT_POOL} on $(hostname)" note_pathname="$(mktemp)" { if [ "${ZEVENT_VDEV_STATE_STR}" = "FAULTED" ] ; then @@ -66,7 +66,7 @@ note_pathname="$(mktemp)" [ -n "${ZEVENT_VDEV_GUID}" ] && echo " vguid: ${ZEVENT_VDEV_GUID}" [ -n "${ZEVENT_VDEV_DEVID}" ] && echo " devid: ${ZEVENT_VDEV_DEVID}" - echo " pool: ${ZEVENT_POOL_GUID}" + echo " pool: ${ZEVENT_POOL} (${ZEVENT_POOL_GUID})" } > "${note_pathname}" From dc95911d21a19930848302aac9283fff68e4a41b Mon Sep 17 00:00:00 2001 From: Allan Jude Date: Tue, 13 Dec 2022 20:27:54 -0500 Subject: [PATCH 2/8] zfs list: Allow more fields in ZFS_ITER_SIMPLE mode If the fields to be listed and sorted by are constrained to those populated by dsl_dataset_fast_stat(), then zfs list is much faster, as it does not need to open each objset and reads its properties. A previous optimization by Pawel Dawidek (0cee24064a79f9c01fc4521543c37acea538405f) took advantage of this to make listing snapshot names sorted only by name much faster. However, it was limited to `-o name -s name`, this work extends this optimization to work with: - name - guid - createtxg - numclones - inconsistent - redacted - origin and could be further extended to any other properties supported by dsl_dataset_fast_stat() or similar, that do not require extra locking or reading from disk. This was committed before (9a9e2e343dfa2af28bf7910de77ae73aa006de62), but was reverted due to a regression when used with an older kernel. If the kernel does not populate zc->zc_objset_stats, we now fallback to getting the properties via the slower interface, to avoid problems with newer userland and older kernels. Reviewed-by: Brian Behlendorf Signed-off-by: Allan Jude Closes #14110 --- cmd/zfs/zfs_iter.c | 65 +++++++++++++++++++++++------ cmd/zfs/zfs_iter.h | 12 +----- cmd/zfs/zfs_main.c | 46 ++++++++++----------- cmd/zpool/zpool_main.c | 2 +- contrib/pam_zfs_key/pam_zfs_key.c | 2 +- include/libzfs.h | 22 ++++++---- lib/libzfs/libzfs.abi | 8 +++- lib/libzfs/libzfs_changelist.c | 6 +-- lib/libzfs/libzfs_crypto.c | 2 +- lib/libzfs/libzfs_dataset.c | 69 ++++++++++++++++++++++++------- lib/libzfs/libzfs_iter.c | 53 ++++++++++++++---------- lib/libzfs/libzfs_mount.c | 4 +- lib/libzfs/libzfs_sendrecv.c | 17 ++++---- module/zfs/zfs_ioctl.c | 5 +-- 14 files changed, 205 insertions(+), 108 deletions(-) diff --git a/cmd/zfs/zfs_iter.c b/cmd/zfs/zfs_iter.c index a0a80d481648..0f8ddd93aad7 100644 --- a/cmd/zfs/zfs_iter.c +++ b/cmd/zfs/zfs_iter.c @@ -143,19 +143,20 @@ zfs_callback(zfs_handle_t *zhp, void *data) (cb->cb_types & (ZFS_TYPE_FILESYSTEM | ZFS_TYPE_VOLUME))) && zfs_get_type(zhp) == ZFS_TYPE_FILESYSTEM) { - (void) zfs_iter_filesystems(zhp, zfs_callback, data); + (void) zfs_iter_filesystems(zhp, cb->cb_flags, + zfs_callback, data); } if (((zfs_get_type(zhp) & (ZFS_TYPE_SNAPSHOT | ZFS_TYPE_BOOKMARK)) == 0) && include_snaps) { - (void) zfs_iter_snapshots(zhp, - (cb->cb_flags & ZFS_ITER_SIMPLE) != 0, + (void) zfs_iter_snapshots(zhp, cb->cb_flags, zfs_callback, data, 0, 0); } if (((zfs_get_type(zhp) & (ZFS_TYPE_SNAPSHOT | ZFS_TYPE_BOOKMARK)) == 0) && include_bmarks) { - (void) zfs_iter_bookmarks(zhp, zfs_callback, data); + (void) zfs_iter_bookmarks(zhp, cb->cb_flags, + zfs_callback, data); } cb->cb_depth--; @@ -211,18 +212,58 @@ zfs_free_sort_columns(zfs_sort_column_t *sc) } } -int -zfs_sort_only_by_name(const zfs_sort_column_t *sc) +/* + * Return true if all of the properties to be sorted are populated by + * dsl_dataset_fast_stat(). Note that sc == NULL (no sort) means we + * don't need any extra properties, so returns true. + */ +boolean_t +zfs_sort_only_by_fast(const zfs_sort_column_t *sc) { - return (sc != NULL && sc->sc_next == NULL && - sc->sc_prop == ZFS_PROP_NAME); + while (sc != NULL) { + switch (sc->sc_prop) { + case ZFS_PROP_NAME: + case ZFS_PROP_GUID: + case ZFS_PROP_CREATETXG: + case ZFS_PROP_NUMCLONES: + case ZFS_PROP_INCONSISTENT: + case ZFS_PROP_REDACTED: + case ZFS_PROP_ORIGIN: + break; + default: + return (B_FALSE); + } + sc = sc->sc_next; + } + + return (B_TRUE); } -int -zfs_sort_only_by_createtxg(const zfs_sort_column_t *sc) +boolean_t +zfs_list_only_by_fast(const zprop_list_t *p) { - return (sc != NULL && sc->sc_next == NULL && - sc->sc_prop == ZFS_PROP_CREATETXG); + if (p == NULL) { + /* NULL means 'all' so we can't use simple mode */ + return (B_FALSE); + } + + while (p != NULL) { + switch (p->pl_prop) { + case ZFS_PROP_NAME: + case ZFS_PROP_GUID: + case ZFS_PROP_CREATETXG: + case ZFS_PROP_NUMCLONES: + case ZFS_PROP_INCONSISTENT: + case ZFS_PROP_REDACTED: + case ZFS_PROP_ORIGIN: + break; + default: + return (B_FALSE); + } + p = p->pl_next; + } + + return (B_TRUE); } static int diff --git a/cmd/zfs/zfs_iter.h b/cmd/zfs/zfs_iter.h index effb22ded3fc..d742ec7b2ec4 100644 --- a/cmd/zfs/zfs_iter.h +++ b/cmd/zfs/zfs_iter.h @@ -40,20 +40,12 @@ typedef struct zfs_sort_column { boolean_t sc_reverse; } zfs_sort_column_t; -#define ZFS_ITER_RECURSE (1 << 0) -#define ZFS_ITER_ARGS_CAN_BE_PATHS (1 << 1) -#define ZFS_ITER_PROP_LISTSNAPS (1 << 2) -#define ZFS_ITER_DEPTH_LIMIT (1 << 3) -#define ZFS_ITER_RECVD_PROPS (1 << 4) -#define ZFS_ITER_LITERAL_PROPS (1 << 5) -#define ZFS_ITER_SIMPLE (1 << 6) - int zfs_for_each(int, char **, int options, zfs_type_t, zfs_sort_column_t *, zprop_list_t **, int, zfs_iter_f, void *); int zfs_add_sort_column(zfs_sort_column_t **, const char *, boolean_t); void zfs_free_sort_columns(zfs_sort_column_t *); -int zfs_sort_only_by_name(const zfs_sort_column_t *); -int zfs_sort_only_by_createtxg(const zfs_sort_column_t *); +boolean_t zfs_sort_only_by_fast(const zfs_sort_column_t *); +boolean_t zfs_list_only_by_fast(const zprop_list_t *); #ifdef __cplusplus } diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index 03640a4cdced..44440dc3dd4d 100644 --- a/cmd/zfs/zfs_main.c +++ b/cmd/zfs/zfs_main.c @@ -1531,7 +1531,7 @@ destroy_print_snapshots(zfs_handle_t *fs_zhp, destroy_cbdata_t *cb) int err; assert(cb->cb_firstsnap == NULL); assert(cb->cb_prevsnap == NULL); - err = zfs_iter_snapshots_sorted(fs_zhp, destroy_print_cb, cb, 0, 0); + err = zfs_iter_snapshots_sorted(fs_zhp, 0, destroy_print_cb, cb, 0, 0); if (cb->cb_firstsnap != NULL) { uint64_t used = 0; if (err == 0) { @@ -1557,7 +1557,7 @@ snapshot_to_nvl_cb(zfs_handle_t *zhp, void *arg) if (!cb->cb_doclones && !cb->cb_defer_destroy) { cb->cb_target = zhp; cb->cb_first = B_TRUE; - err = zfs_iter_dependents(zhp, B_TRUE, + err = zfs_iter_dependents(zhp, 0, B_TRUE, destroy_check_dependent, cb); } @@ -1575,7 +1575,8 @@ gather_snapshots(zfs_handle_t *zhp, void *arg) destroy_cbdata_t *cb = arg; int err = 0; - err = zfs_iter_snapspec(zhp, cb->cb_snapspec, snapshot_to_nvl_cb, cb); + err = zfs_iter_snapspec(zhp, 0, cb->cb_snapspec, + snapshot_to_nvl_cb, cb); if (err == ENOENT) err = 0; if (err != 0) @@ -1588,7 +1589,7 @@ gather_snapshots(zfs_handle_t *zhp, void *arg) } if (cb->cb_recurse) - err = zfs_iter_filesystems(zhp, gather_snapshots, cb); + err = zfs_iter_filesystems(zhp, 0, gather_snapshots, cb); out: zfs_close(zhp); @@ -1613,7 +1614,7 @@ destroy_clones(destroy_cbdata_t *cb) * false while destroying the clones. */ cb->cb_defer_destroy = B_FALSE; - err = zfs_iter_dependents(zhp, B_FALSE, + err = zfs_iter_dependents(zhp, 0, B_FALSE, destroy_callback, cb); cb->cb_defer_destroy = defer; zfs_close(zhp); @@ -1824,7 +1825,7 @@ zfs_do_destroy(int argc, char **argv) */ cb.cb_first = B_TRUE; if (!cb.cb_doclones && - zfs_iter_dependents(zhp, B_TRUE, destroy_check_dependent, + zfs_iter_dependents(zhp, 0, B_TRUE, destroy_check_dependent, &cb) != 0) { rv = 1; goto out; @@ -1835,7 +1836,7 @@ zfs_do_destroy(int argc, char **argv) goto out; } cb.cb_batchedsnaps = fnvlist_alloc(); - if (zfs_iter_dependents(zhp, B_FALSE, destroy_callback, + if (zfs_iter_dependents(zhp, 0, B_FALSE, destroy_callback, &cb) != 0) { rv = 1; goto out; @@ -3659,16 +3660,6 @@ found3:; argc -= optind; argv += optind; - /* - * If we are only going to list snapshot names and sort by name or - * by createtxg, then we can use faster version. - */ - if (strcmp(fields, "name") == 0 && - (zfs_sort_only_by_name(sortcol) || - zfs_sort_only_by_createtxg(sortcol))) { - flags |= ZFS_ITER_SIMPLE; - } - /* * If "-o space" and no types were specified, don't display snapshots. */ @@ -3696,6 +3687,15 @@ found3:; cb.cb_first = B_TRUE; + /* + * If we are only going to list and sort by properties that are "fast" + * then we can use "simple" mode and avoid populating the properties + * nvlist. + */ + if (zfs_list_only_by_fast(cb.cb_proplist) && + zfs_sort_only_by_fast(sortcol)) + flags |= ZFS_ITER_SIMPLE; + ret = zfs_for_each(argc, argv, flags, types, sortcol, &cb.cb_proplist, limit, list_callback, &cb); @@ -4006,7 +4006,7 @@ rollback_check(zfs_handle_t *zhp, void *data) } if (cbp->cb_recurse) { - if (zfs_iter_dependents(zhp, B_TRUE, + if (zfs_iter_dependents(zhp, 0, B_TRUE, rollback_check_dependent, cbp) != 0) { zfs_close(zhp); return (-1); @@ -4105,10 +4105,10 @@ zfs_do_rollback(int argc, char **argv) if (cb.cb_create > 0) min_txg = cb.cb_create; - if ((ret = zfs_iter_snapshots(zhp, B_FALSE, rollback_check, &cb, + if ((ret = zfs_iter_snapshots(zhp, 0, rollback_check, &cb, min_txg, 0)) != 0) goto out; - if ((ret = zfs_iter_bookmarks(zhp, rollback_check, &cb)) != 0) + if ((ret = zfs_iter_bookmarks(zhp, 0, rollback_check, &cb)) != 0) goto out; if ((ret = cb.cb_error) != 0) @@ -4250,7 +4250,7 @@ zfs_snapshot_cb(zfs_handle_t *zhp, void *arg) free(name); if (sd->sd_recursive) - rv = zfs_iter_filesystems(zhp, zfs_snapshot_cb, sd); + rv = zfs_iter_filesystems(zhp, 0, zfs_snapshot_cb, sd); zfs_close(zhp); return (rv); } @@ -6310,7 +6310,7 @@ zfs_do_allow_unallow_impl(int argc, char **argv, boolean_t un) if (un && opts.recursive) { struct deleg_perms data = { un, update_perm_nvl }; - if (zfs_iter_filesystems(zhp, set_deleg_perms, + if (zfs_iter_filesystems(zhp, 0, set_deleg_perms, &data) != 0) goto cleanup0; } @@ -6688,7 +6688,7 @@ get_one_dataset(zfs_handle_t *zhp, void *data) /* * Iterate over any nested datasets. */ - if (zfs_iter_filesystems(zhp, get_one_dataset, data) != 0) { + if (zfs_iter_filesystems(zhp, 0, get_one_dataset, data) != 0) { zfs_close(zhp); return (1); } diff --git a/cmd/zpool/zpool_main.c b/cmd/zpool/zpool_main.c index 0872671f428f..af76d20f2d52 100644 --- a/cmd/zpool/zpool_main.c +++ b/cmd/zpool/zpool_main.c @@ -8808,7 +8808,7 @@ check_unsupp_fs(zfs_handle_t *zhp, void *unsupp_fs) (*count)++; } - zfs_iter_filesystems(zhp, check_unsupp_fs, unsupp_fs); + zfs_iter_filesystems(zhp, 0, check_unsupp_fs, unsupp_fs); zfs_close(zhp); diff --git a/contrib/pam_zfs_key/pam_zfs_key.c b/contrib/pam_zfs_key/pam_zfs_key.c index e3fa9e9b2553..99cdb8d7733f 100644 --- a/contrib/pam_zfs_key/pam_zfs_key.c +++ b/contrib/pam_zfs_key/pam_zfs_key.c @@ -535,7 +535,7 @@ zfs_key_config_get_dataset(zfs_key_config_t *config) return (NULL); } - (void) zfs_iter_filesystems(zhp, find_dsname_by_prop_value, + (void) zfs_iter_filesystems(zhp, 0, find_dsname_by_prop_value, config); zfs_close(zhp); char *dsname = config->dsname; diff --git a/include/libzfs.h b/include/libzfs.h index 2806d1f7cff5..e563749226ec 100644 --- a/include/libzfs.h +++ b/include/libzfs.h @@ -646,19 +646,27 @@ _LIBZFS_H void zprop_print_one_property(const char *, zprop_get_cbdata_t *, /* * Iterator functions. */ +#define ZFS_ITER_RECURSE (1 << 0) +#define ZFS_ITER_ARGS_CAN_BE_PATHS (1 << 1) +#define ZFS_ITER_PROP_LISTSNAPS (1 << 2) +#define ZFS_ITER_DEPTH_LIMIT (1 << 3) +#define ZFS_ITER_RECVD_PROPS (1 << 4) +#define ZFS_ITER_LITERAL_PROPS (1 << 5) +#define ZFS_ITER_SIMPLE (1 << 6) + typedef int (*zfs_iter_f)(zfs_handle_t *, void *); _LIBZFS_H int zfs_iter_root(libzfs_handle_t *, zfs_iter_f, void *); -_LIBZFS_H int zfs_iter_children(zfs_handle_t *, zfs_iter_f, void *); -_LIBZFS_H int zfs_iter_dependents(zfs_handle_t *, boolean_t, zfs_iter_f, +_LIBZFS_H int zfs_iter_children(zfs_handle_t *, int, zfs_iter_f, void *); +_LIBZFS_H int zfs_iter_dependents(zfs_handle_t *, int, boolean_t, zfs_iter_f, void *); -_LIBZFS_H int zfs_iter_filesystems(zfs_handle_t *, zfs_iter_f, void *); -_LIBZFS_H int zfs_iter_snapshots(zfs_handle_t *, boolean_t, zfs_iter_f, void *, +_LIBZFS_H int zfs_iter_filesystems(zfs_handle_t *, int, zfs_iter_f, void *); +_LIBZFS_H int zfs_iter_snapshots(zfs_handle_t *, int, zfs_iter_f, void *, uint64_t, uint64_t); -_LIBZFS_H int zfs_iter_snapshots_sorted(zfs_handle_t *, zfs_iter_f, void *, +_LIBZFS_H int zfs_iter_snapshots_sorted(zfs_handle_t *, int, zfs_iter_f, void *, uint64_t, uint64_t); -_LIBZFS_H int zfs_iter_snapspec(zfs_handle_t *, const char *, zfs_iter_f, +_LIBZFS_H int zfs_iter_snapspec(zfs_handle_t *, int, const char *, zfs_iter_f, void *); -_LIBZFS_H int zfs_iter_bookmarks(zfs_handle_t *, zfs_iter_f, void *); +_LIBZFS_H int zfs_iter_bookmarks(zfs_handle_t *, int, zfs_iter_f, void *); _LIBZFS_H int zfs_iter_mounted(zfs_handle_t *, zfs_iter_f, void *); typedef struct get_all_cb { diff --git a/lib/libzfs/libzfs.abi b/lib/libzfs/libzfs.abi index 98873784e7dc..e5115e9731f3 100644 --- a/lib/libzfs/libzfs.abi +++ b/lib/libzfs/libzfs.abi @@ -2925,13 +2925,14 @@ + - + @@ -2940,12 +2941,14 @@ + + @@ -2954,6 +2957,7 @@ + @@ -2961,12 +2965,14 @@ + + diff --git a/lib/libzfs/libzfs_changelist.c b/lib/libzfs/libzfs_changelist.c index e5e735d38e00..d7ea60822419 100644 --- a/lib/libzfs/libzfs_changelist.c +++ b/lib/libzfs/libzfs_changelist.c @@ -552,7 +552,7 @@ change_one(zfs_handle_t *zhp, void *data) } if (!clp->cl_alldependents) - ret = zfs_iter_children(zhp, change_one, data); + ret = zfs_iter_children(zhp, 0, change_one, data); /* * If we added the handle to the changelist, we will re-use it @@ -721,11 +721,11 @@ changelist_gather(zfs_handle_t *zhp, zfs_prop_t prop, int gather_flags, return (NULL); } } else if (clp->cl_alldependents) { - if (zfs_iter_dependents(zhp, B_TRUE, change_one, clp) != 0) { + if (zfs_iter_dependents(zhp, 0, B_TRUE, change_one, clp) != 0) { changelist_free(clp); return (NULL); } - } else if (zfs_iter_children(zhp, change_one, clp) != 0) { + } else if (zfs_iter_children(zhp, 0, change_one, clp) != 0) { changelist_free(clp); return (NULL); } diff --git a/lib/libzfs/libzfs_crypto.c b/lib/libzfs/libzfs_crypto.c index c241aeaa4da0..3ef883701082 100644 --- a/lib/libzfs/libzfs_crypto.c +++ b/lib/libzfs/libzfs_crypto.c @@ -1226,7 +1226,7 @@ load_keys_cb(zfs_handle_t *zhp, void *arg) cb->cb_numfailed++; out: - (void) zfs_iter_filesystems(zhp, load_keys_cb, cb); + (void) zfs_iter_filesystems(zhp, 0, load_keys_cb, cb); zfs_close(zhp); /* always return 0, since this function is best effort */ diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index fd9fbd658a57..9ecb1ac5c6fb 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -527,7 +527,30 @@ make_dataset_simple_handle_zc(zfs_handle_t *pzhp, zfs_cmd_t *zc) zhp->zfs_head_type = pzhp->zfs_type; zhp->zfs_type = ZFS_TYPE_SNAPSHOT; zhp->zpool_hdl = zpool_handle(zhp); - zhp->zfs_dmustats = zc->zc_objset_stats; + + if (zc->zc_objset_stats.dds_creation_txg != 0) { + /* structure assignment */ + zhp->zfs_dmustats = zc->zc_objset_stats; + } else { + if (get_stats_ioctl(zhp, zc) == -1) { + zcmd_free_nvlists(zc); + free(zhp); + return (NULL); + } + if (make_dataset_handle_common(zhp, zc) == -1) { + zcmd_free_nvlists(zc); + free(zhp); + return (NULL); + } + } + + if (zhp->zfs_dmustats.dds_is_snapshot || + strchr(zc->zc_name, '@') != NULL) + zhp->zfs_type = ZFS_TYPE_SNAPSHOT; + else if (zhp->zfs_dmustats.dds_type == DMU_OST_ZVOL) + zhp->zfs_type = ZFS_TYPE_VOLUME; + else if (zhp->zfs_dmustats.dds_type == DMU_OST_ZFS) + zhp->zfs_type = ZFS_TYPE_FILESYSTEM; return (zhp); } @@ -734,7 +757,7 @@ zfs_open(libzfs_handle_t *hdl, const char *path, int types) * Iterate bookmarks to find the right one. */ errno = 0; - if ((zfs_iter_bookmarks(pzhp, zfs_open_bookmarks_cb, + if ((zfs_iter_bookmarks(pzhp, 0, zfs_open_bookmarks_cb, &cb_data) == 0) && (cb_data.zhp == NULL)) { (void) zfs_error(hdl, EZFS_NOENT, errbuf); zfs_close(pzhp); @@ -2086,7 +2109,8 @@ getprop_string(zfs_handle_t *zhp, zfs_prop_t prop, char **source) static boolean_t zfs_is_recvd_props_mode(zfs_handle_t *zhp) { - return (zhp->zfs_props == zhp->zfs_recvd_props); + return (zhp->zfs_props != NULL && + zhp->zfs_props == zhp->zfs_recvd_props); } static void @@ -2288,19 +2312,28 @@ get_numeric_property(zfs_handle_t *zhp, zfs_prop_t prop, zprop_source_t *src, *val = zhp->zfs_dmustats.dds_redacted; break; + case ZFS_PROP_GUID: + if (zhp->zfs_dmustats.dds_guid != 0) + *val = zhp->zfs_dmustats.dds_guid; + else + *val = getprop_uint64(zhp, prop, source); + break; + case ZFS_PROP_CREATETXG: /* * We can directly read createtxg property from zfs * handle for Filesystem, Snapshot and ZVOL types. */ - if ((zhp->zfs_type == ZFS_TYPE_FILESYSTEM) || + if (((zhp->zfs_type == ZFS_TYPE_FILESYSTEM) || (zhp->zfs_type == ZFS_TYPE_SNAPSHOT) || - (zhp->zfs_type == ZFS_TYPE_VOLUME)) { + (zhp->zfs_type == ZFS_TYPE_VOLUME)) && + (zhp->zfs_dmustats.dds_creation_txg != 0)) { *val = zhp->zfs_dmustats.dds_creation_txg; break; + } else { + *val = getprop_uint64(zhp, prop, source); } zfs_fallthrough; - default: switch (zfs_prop_get_type(prop)) { case PROP_TYPE_NUMBER: @@ -2443,7 +2476,7 @@ get_clones_cb(zfs_handle_t *zhp, void *arg) } out: - (void) zfs_iter_children(zhp, get_clones_cb, gca); + (void) zfs_iter_children(zhp, 0, get_clones_cb, gca); zfs_close(zhp); return (0); } @@ -2728,7 +2761,13 @@ zfs_prop_get(zfs_handle_t *zhp, zfs_prop_t prop, char *propbuf, size_t proplen, break; case ZFS_PROP_ORIGIN: - str = getprop_string(zhp, prop, &source); + if (*zhp->zfs_dmustats.dds_origin != '\0') { + str = (char *)&zhp->zfs_dmustats.dds_origin; + } else { + str = getprop_string(zhp, prop, &source); + } + if (str == NULL || *str == '\0') + str = zfs_prop_default_string(prop); if (str == NULL) return (-1); (void) strlcpy(propbuf, str, proplen); @@ -3886,7 +3925,7 @@ zfs_check_snap_cb(zfs_handle_t *zhp, void *arg) if (lzc_exists(name)) fnvlist_add_boolean(dd->nvl, name); - rv = zfs_iter_filesystems(zhp, zfs_check_snap_cb, dd); + rv = zfs_iter_filesystems(zhp, 0, zfs_check_snap_cb, dd); zfs_close(zhp); return (rv); } @@ -4124,7 +4163,7 @@ zfs_snapshot_cb(zfs_handle_t *zhp, void *arg) fnvlist_add_boolean(sd->sd_nvl, name); - rv = zfs_iter_filesystems(zhp, zfs_snapshot_cb, sd); + rv = zfs_iter_filesystems(zhp, 0, zfs_snapshot_cb, sd); } zfs_close(zhp); @@ -4301,7 +4340,7 @@ rollback_destroy(zfs_handle_t *zhp, void *data) rollback_data_t *cbp = data; if (zfs_prop_get_int(zhp, ZFS_PROP_CREATETXG) > cbp->cb_create) { - cbp->cb_error |= zfs_iter_dependents(zhp, B_FALSE, + cbp->cb_error |= zfs_iter_dependents(zhp, 0, B_FALSE, rollback_destroy_dependent, cbp); cbp->cb_error |= zfs_destroy(zhp, B_FALSE); @@ -4341,10 +4380,10 @@ zfs_rollback(zfs_handle_t *zhp, zfs_handle_t *snap, boolean_t force) if (cb.cb_create > 0) min_txg = cb.cb_create; - (void) zfs_iter_snapshots(zhp, B_FALSE, rollback_destroy, &cb, + (void) zfs_iter_snapshots(zhp, 0, rollback_destroy, &cb, min_txg, 0); - (void) zfs_iter_bookmarks(zhp, rollback_destroy, &cb); + (void) zfs_iter_bookmarks(zhp, 0, rollback_destroy, &cb); if (cb.cb_error) return (-1); @@ -4925,7 +4964,7 @@ zfs_hold_one(zfs_handle_t *zhp, void *arg) fnvlist_add_string(ha->nvl, name, ha->tag); if (ha->recursive) - rv = zfs_iter_filesystems(zhp, zfs_hold_one, ha); + rv = zfs_iter_filesystems(zhp, 0, zfs_hold_one, ha); zfs_close(zhp); return (rv); } @@ -5056,7 +5095,7 @@ zfs_release_one(zfs_handle_t *zhp, void *arg) } if (ha->recursive) - rv = zfs_iter_filesystems(zhp, zfs_release_one, ha); + rv = zfs_iter_filesystems(zhp, 0, zfs_release_one, ha); zfs_close(zhp); return (rv); } diff --git a/lib/libzfs/libzfs_iter.c b/lib/libzfs/libzfs_iter.c index a716521ab17d..55cb7a8b5035 100644 --- a/lib/libzfs/libzfs_iter.c +++ b/lib/libzfs/libzfs_iter.c @@ -39,7 +39,8 @@ #include "libzfs_impl.h" static int -zfs_iter_clones(zfs_handle_t *zhp, zfs_iter_f func, void *data) +zfs_iter_clones(zfs_handle_t *zhp, int flags __maybe_unused, zfs_iter_f func, + void *data) { nvlist_t *nvl = zfs_get_clones_nvl(zhp); nvpair_t *pair; @@ -69,6 +70,7 @@ zfs_do_list_ioctl(zfs_handle_t *zhp, int arg, zfs_cmd_t *zc) orig_cookie = zc->zc_cookie; top: (void) strlcpy(zc->zc_name, zhp->zfs_name, sizeof (zc->zc_name)); + zc->zc_objset_stats.dds_creation_txg = 0; rc = zfs_ioctl(zhp->zfs_hdl, arg, zc); if (rc == -1) { @@ -101,7 +103,7 @@ zfs_do_list_ioctl(zfs_handle_t *zhp, int arg, zfs_cmd_t *zc) * Iterate over all child filesystems */ int -zfs_iter_filesystems(zfs_handle_t *zhp, zfs_iter_f func, void *data) +zfs_iter_filesystems(zfs_handle_t *zhp, int flags, zfs_iter_f func, void *data) { zfs_cmd_t zc = {"\0"}; zfs_handle_t *nzhp; @@ -112,16 +114,21 @@ zfs_iter_filesystems(zfs_handle_t *zhp, zfs_iter_f func, void *data) zcmd_alloc_dst_nvlist(zhp->zfs_hdl, &zc, 0); + if ((flags & ZFS_ITER_SIMPLE) == ZFS_ITER_SIMPLE) + zc.zc_simple = B_TRUE; + while ((ret = zfs_do_list_ioctl(zhp, ZFS_IOC_DATASET_LIST_NEXT, &zc)) == 0) { + if (zc.zc_simple) + nzhp = make_dataset_simple_handle_zc(zhp, &zc); + else + nzhp = make_dataset_handle_zc(zhp->zfs_hdl, &zc); /* * Silently ignore errors, as the only plausible explanation is * that the pool has since been removed. */ - if ((nzhp = make_dataset_handle_zc(zhp->zfs_hdl, - &zc)) == NULL) { + if (nzhp == NULL) continue; - } if ((ret = func(nzhp, data)) != 0) { zcmd_free_nvlists(&zc); @@ -136,7 +143,7 @@ zfs_iter_filesystems(zfs_handle_t *zhp, zfs_iter_f func, void *data) * Iterate over all snapshots */ int -zfs_iter_snapshots(zfs_handle_t *zhp, boolean_t simple, zfs_iter_f func, +zfs_iter_snapshots(zfs_handle_t *zhp, int flags, zfs_iter_f func, void *data, uint64_t min_txg, uint64_t max_txg) { zfs_cmd_t zc = {"\0"}; @@ -148,7 +155,7 @@ zfs_iter_snapshots(zfs_handle_t *zhp, boolean_t simple, zfs_iter_f func, zhp->zfs_type == ZFS_TYPE_BOOKMARK) return (0); - zc.zc_simple = simple; + zc.zc_simple = (flags & ZFS_ITER_SIMPLE) != 0; zcmd_alloc_dst_nvlist(zhp->zfs_hdl, &zc, 0); @@ -168,7 +175,7 @@ zfs_iter_snapshots(zfs_handle_t *zhp, boolean_t simple, zfs_iter_f func, while ((ret = zfs_do_list_ioctl(zhp, ZFS_IOC_SNAPSHOT_LIST_NEXT, &zc)) == 0) { - if (simple) + if (zc.zc_simple) nzhp = make_dataset_simple_handle_zc(zhp, &zc); else nzhp = make_dataset_handle_zc(zhp->zfs_hdl, &zc); @@ -190,7 +197,8 @@ zfs_iter_snapshots(zfs_handle_t *zhp, boolean_t simple, zfs_iter_f func, * Iterate over all bookmarks */ int -zfs_iter_bookmarks(zfs_handle_t *zhp, zfs_iter_f func, void *data) +zfs_iter_bookmarks(zfs_handle_t *zhp, int flags __maybe_unused, + zfs_iter_f func, void *data) { zfs_handle_t *nzhp; nvlist_t *props = NULL; @@ -297,8 +305,8 @@ zfs_snapshot_compare(const void *larg, const void *rarg) } int -zfs_iter_snapshots_sorted(zfs_handle_t *zhp, zfs_iter_f callback, void *data, - uint64_t min_txg, uint64_t max_txg) +zfs_iter_snapshots_sorted(zfs_handle_t *zhp, int flags, zfs_iter_f callback, + void *data, uint64_t min_txg, uint64_t max_txg) { int ret = 0; zfs_node_t *node; @@ -308,7 +316,7 @@ zfs_iter_snapshots_sorted(zfs_handle_t *zhp, zfs_iter_f callback, void *data, avl_create(&avl, zfs_snapshot_compare, sizeof (zfs_node_t), offsetof(zfs_node_t, zn_avlnode)); - ret = zfs_iter_snapshots(zhp, B_FALSE, zfs_sort_snaps, &avl, min_txg, + ret = zfs_iter_snapshots(zhp, flags, zfs_sort_snaps, &avl, min_txg, max_txg); for (node = avl_first(&avl); node != NULL; node = AVL_NEXT(&avl, node)) @@ -371,7 +379,7 @@ snapspec_cb(zfs_handle_t *zhp, void *arg) * return ENOENT at the end. */ int -zfs_iter_snapspec(zfs_handle_t *fs_zhp, const char *spec_orig, +zfs_iter_snapspec(zfs_handle_t *fs_zhp, int flags, const char *spec_orig, zfs_iter_f func, void *arg) { char *buf, *comma_separated, *cp; @@ -411,7 +419,7 @@ zfs_iter_snapspec(zfs_handle_t *fs_zhp, const char *spec_orig, } } - err = zfs_iter_snapshots_sorted(fs_zhp, + err = zfs_iter_snapshots_sorted(fs_zhp, flags, snapspec_cb, &ssa, 0, 0); if (ret == 0) ret = err; @@ -448,14 +456,14 @@ zfs_iter_snapspec(zfs_handle_t *fs_zhp, const char *spec_orig, * and as close as possible. */ int -zfs_iter_children(zfs_handle_t *zhp, zfs_iter_f func, void *data) +zfs_iter_children(zfs_handle_t *zhp, int flags, zfs_iter_f func, void *data) { int ret; - if ((ret = zfs_iter_snapshots(zhp, B_FALSE, func, data, 0, 0)) != 0) + if ((ret = zfs_iter_snapshots(zhp, flags, func, data, 0, 0)) != 0) return (ret); - return (zfs_iter_filesystems(zhp, func, data)); + return (zfs_iter_filesystems(zhp, flags, func, data)); } @@ -466,6 +474,7 @@ typedef struct iter_stack_frame { typedef struct iter_dependents_arg { boolean_t first; + int flags; boolean_t allowrecursion; iter_stack_frame_t *stack; zfs_iter_f func; @@ -481,7 +490,7 @@ iter_dependents_cb(zfs_handle_t *zhp, void *arg) ida->first = B_FALSE; if (zhp->zfs_type == ZFS_TYPE_SNAPSHOT) { - err = zfs_iter_clones(zhp, iter_dependents_cb, ida); + err = zfs_iter_clones(zhp, ida->flags, iter_dependents_cb, ida); } else if (zhp->zfs_type != ZFS_TYPE_BOOKMARK) { iter_stack_frame_t isf; iter_stack_frame_t *f; @@ -515,9 +524,10 @@ iter_dependents_cb(zfs_handle_t *zhp, void *arg) isf.zhp = zhp; isf.next = ida->stack; ida->stack = &isf; - err = zfs_iter_filesystems(zhp, iter_dependents_cb, ida); + err = zfs_iter_filesystems(zhp, ida->flags, + iter_dependents_cb, ida); if (err == 0) - err = zfs_iter_snapshots(zhp, B_FALSE, + err = zfs_iter_snapshots(zhp, ida->flags, iter_dependents_cb, ida, 0, 0); ida->stack = isf.next; } @@ -531,10 +541,11 @@ iter_dependents_cb(zfs_handle_t *zhp, void *arg) } int -zfs_iter_dependents(zfs_handle_t *zhp, boolean_t allowrecursion, +zfs_iter_dependents(zfs_handle_t *zhp, int flags, boolean_t allowrecursion, zfs_iter_f func, void *data) { iter_dependents_arg_t ida; + ida.flags = flags; ida.allowrecursion = allowrecursion; ida.stack = NULL; ida.func = func; diff --git a/lib/libzfs/libzfs_mount.c b/lib/libzfs/libzfs_mount.c index 44f7d698c82c..57737bc6c01a 100644 --- a/lib/libzfs/libzfs_mount.c +++ b/lib/libzfs/libzfs_mount.c @@ -940,7 +940,7 @@ zfs_iter_cb(zfs_handle_t *zhp, void *data) } libzfs_add_handle(cbp, zhp); - if (zfs_iter_filesystems(zhp, zfs_iter_cb, cbp) != 0) { + if (zfs_iter_filesystems(zhp, 0, zfs_iter_cb, cbp) != 0) { zfs_close(zhp); return (-1); } @@ -1289,7 +1289,7 @@ zpool_enable_datasets(zpool_handle_t *zhp, const char *mntopts, int flags) * over all child filesystems. */ libzfs_add_handle(&cb, zfsp); - if (zfs_iter_filesystems(zfsp, zfs_iter_cb, &cb) != 0) + if (zfs_iter_filesystems(zfsp, 0, zfs_iter_cb, &cb) != 0) goto out; /* diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 089d46b130be..ac1834733cee 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -616,10 +616,10 @@ send_iterate_fs(zfs_handle_t *zhp, void *arg) min_txg = fromsnap_txg; if (!sd->replicate && tosnap_txg != 0) max_txg = tosnap_txg; - (void) zfs_iter_snapshots_sorted(zhp, send_iterate_snap, sd, + (void) zfs_iter_snapshots_sorted(zhp, 0, send_iterate_snap, sd, min_txg, max_txg); } else { - char snapname[MAXPATHLEN]; + char snapname[MAXPATHLEN] = { 0 }; zfs_handle_t *snap; (void) snprintf(snapname, sizeof (snapname), "%s@%s", @@ -659,7 +659,7 @@ send_iterate_fs(zfs_handle_t *zhp, void *arg) /* Iterate over children. */ if (sd->recursive) - rv = zfs_iter_filesystems(zhp, send_iterate_fs, sd); + rv = zfs_iter_filesystems(zhp, 0, send_iterate_fs, sd); out: /* Restore saved fields. */ @@ -1274,7 +1274,7 @@ dump_filesystem(zfs_handle_t *zhp, send_dump_data_t *sdd) zhp->zfs_name, sdd->tosnap); } } - rv = zfs_iter_snapshots_sorted(zhp, dump_snapshot, sdd, + rv = zfs_iter_snapshots_sorted(zhp, 0, dump_snapshot, sdd, min_txg, max_txg); } else { char snapname[MAXPATHLEN] = { 0 }; @@ -3125,9 +3125,9 @@ guid_to_name_cb(zfs_handle_t *zhp, void *arg) return (EEXIST); } - err = zfs_iter_children(zhp, guid_to_name_cb, gtnd); + err = zfs_iter_children(zhp, 0, guid_to_name_cb, gtnd); if (err != EEXIST && gtnd->bookmark_ok) - err = zfs_iter_bookmarks(zhp, guid_to_name_cb, gtnd); + err = zfs_iter_bookmarks(zhp, 0, guid_to_name_cb, gtnd); zfs_close(zhp); return (err); } @@ -3181,9 +3181,10 @@ guid_to_name_redact_snaps(libzfs_handle_t *hdl, const char *parent, continue; int err = guid_to_name_cb(zfs_handle_dup(zhp), >nd); if (err != EEXIST) - err = zfs_iter_children(zhp, guid_to_name_cb, >nd); + err = zfs_iter_children(zhp, 0, guid_to_name_cb, >nd); if (err != EEXIST && bookmark_ok) - err = zfs_iter_bookmarks(zhp, guid_to_name_cb, >nd); + err = zfs_iter_bookmarks(zhp, 0, guid_to_name_cb, + >nd); zfs_close(zhp); if (err == EEXIST) return (0); diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index a5168b937588..a55d9f88ab7d 100644 --- a/module/zfs/zfs_ioctl.c +++ b/module/zfs/zfs_ioctl.c @@ -2039,7 +2039,7 @@ zfs_ioc_objset_stats_impl(zfs_cmd_t *zc, objset_t *os) dmu_objset_fast_stat(os, &zc->zc_objset_stats); - if (zc->zc_nvlist_dst != 0 && + if (!zc->zc_simple && zc->zc_nvlist_dst != 0 && (error = dsl_prop_get_all(os, &nv)) == 0) { dmu_objset_stats(os, nv); /* @@ -2326,8 +2326,7 @@ zfs_ioc_snapshot_list_next(zfs_cmd_t *zc) } if (zc->zc_simple) { - zc->zc_objset_stats.dds_creation_txg = - dsl_get_creationtxg(ds); + dsl_dataset_fast_stat(ds, &zc->zc_objset_stats); dsl_dataset_rele(ds, FTAG); break; } From 3236c0b891d0a09475bef8b6186ac8beed792fe9 Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Tue, 13 Dec 2022 20:29:21 -0500 Subject: [PATCH 3/8] Cache dbuf_hash() calculation We currently compute a 64-bit hash three times, which consumes 0.8% CPU time on ARC eviction heavy workloads. Caching the 64-bit value in the dbuf allows us to avoid that overhead. Sponsored-By: Wasabi Technology, Inc. Reviewed-by: Brian Behlendorf Reviewed-by: Matthew Ahrens Signed-off-by: Richard Yao Closes #14251 --- include/sys/dbuf.h | 4 +++- module/zfs/dbuf.c | 37 ++++++++++++++++++++++--------------- module/zfs/dnode_sync.c | 4 ++-- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/include/sys/dbuf.h b/include/sys/dbuf.h index 9ba46f0d725f..6215a2c129a2 100644 --- a/include/sys/dbuf.h +++ b/include/sys/dbuf.h @@ -294,6 +294,8 @@ typedef struct dmu_buf_impl { /* Tells us which dbuf cache this dbuf is in, if any */ dbuf_cached_state_t db_caching_status; + uint64_t db_hash; + /* Data which is unique to data (leaf) blocks: */ /* User callback information. */ @@ -364,7 +366,7 @@ void dbuf_rele_and_unlock(dmu_buf_impl_t *db, const void *tag, boolean_t evicting); dmu_buf_impl_t *dbuf_find(struct objset *os, uint64_t object, uint8_t level, - uint64_t blkid); + uint64_t blkid, uint64_t *hash_out); int dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags); void dmu_buf_will_not_fill(dmu_buf_t *db, dmu_tx_t *tx); diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index 52760fb1b57e..5e1ed83863bb 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -339,7 +339,8 @@ dbuf_hash(void *os, uint64_t obj, uint8_t lvl, uint64_t blkid) (dbuf)->db_blkid == (blkid)) dmu_buf_impl_t * -dbuf_find(objset_t *os, uint64_t obj, uint8_t level, uint64_t blkid) +dbuf_find(objset_t *os, uint64_t obj, uint8_t level, uint64_t blkid, + uint64_t *hash_out) { dbuf_hash_table_t *h = &dbuf_hash_table; uint64_t hv; @@ -361,6 +362,8 @@ dbuf_find(objset_t *os, uint64_t obj, uint8_t level, uint64_t blkid) } } mutex_exit(DBUF_HASH_MUTEX(h, idx)); + if (hash_out != NULL) + *hash_out = hv; return (NULL); } @@ -395,13 +398,13 @@ dbuf_hash_insert(dmu_buf_impl_t *db) objset_t *os = db->db_objset; uint64_t obj = db->db.db_object; int level = db->db_level; - uint64_t blkid, hv, idx; + uint64_t blkid, idx; dmu_buf_impl_t *dbf; uint32_t i; blkid = db->db_blkid; - hv = dbuf_hash(os, obj, level, blkid); - idx = hv & h->hash_table_mask; + ASSERT3U(dbuf_hash(os, obj, level, blkid), ==, db->db_hash); + idx = db->db_hash & h->hash_table_mask; mutex_enter(DBUF_HASH_MUTEX(h, idx)); for (dbf = h->hash_table[idx], i = 0; dbf != NULL; @@ -475,12 +478,12 @@ static void dbuf_hash_remove(dmu_buf_impl_t *db) { dbuf_hash_table_t *h = &dbuf_hash_table; - uint64_t hv, idx; + uint64_t idx; dmu_buf_impl_t *dbf, **dbp; - hv = dbuf_hash(db->db_objset, db->db.db_object, - db->db_level, db->db_blkid); - idx = hv & h->hash_table_mask; + ASSERT3U(dbuf_hash(db->db_objset, db->db.db_object, db->db_level, + db->db_blkid), ==, db->db_hash); + idx = db->db_hash & h->hash_table_mask; /* * We mustn't hold db_mtx to maintain lock ordering: @@ -2124,7 +2127,8 @@ dbuf_dirty_lightweight(dnode_t *dn, uint64_t blkid, dmu_tx_t *tx) * Otherwise the buffer contents could be inconsistent between the * dbuf and the lightweight dirty record. */ - ASSERT3P(NULL, ==, dbuf_find(dn->dn_objset, dn->dn_object, 0, blkid)); + ASSERT3P(NULL, ==, dbuf_find(dn->dn_objset, dn->dn_object, 0, blkid, + NULL)); mutex_enter(&dn->dn_mtx); int txgoff = tx->tx_txg & TXG_MASK; @@ -3073,7 +3077,7 @@ dbuf_findbp(dnode_t *dn, int level, uint64_t blkid, int fail_sparse, static dmu_buf_impl_t * dbuf_create(dnode_t *dn, uint8_t level, uint64_t blkid, - dmu_buf_impl_t *parent, blkptr_t *blkptr) + dmu_buf_impl_t *parent, blkptr_t *blkptr, uint64_t hash) { objset_t *os = dn->dn_objset; dmu_buf_impl_t *db, *odb; @@ -3094,6 +3098,7 @@ dbuf_create(dnode_t *dn, uint8_t level, uint64_t blkid, db->db_dnode_handle = dn->dn_handle; db->db_parent = parent; db->db_blkptr = blkptr; + db->db_hash = hash; db->db_user = NULL; db->db_user_immediate_evict = FALSE; @@ -3394,7 +3399,7 @@ dbuf_prefetch_impl(dnode_t *dn, int64_t level, uint64_t blkid, goto no_issue; dmu_buf_impl_t *db = dbuf_find(dn->dn_objset, dn->dn_object, - level, blkid); + level, blkid, NULL); if (db != NULL) { mutex_exit(&db->db_mtx); /* @@ -3559,6 +3564,7 @@ dbuf_hold_impl(dnode_t *dn, uint8_t level, uint64_t blkid, const void *tag, dmu_buf_impl_t **dbp) { dmu_buf_impl_t *db, *parent = NULL; + uint64_t hv; /* If the pool has been created, verify the tx_sync_lock is not held */ spa_t *spa = dn->dn_objset->os_spa; @@ -3574,7 +3580,7 @@ dbuf_hold_impl(dnode_t *dn, uint8_t level, uint64_t blkid, *dbp = NULL; /* dbuf_find() returns with db_mtx held */ - db = dbuf_find(dn->dn_objset, dn->dn_object, level, blkid); + db = dbuf_find(dn->dn_objset, dn->dn_object, level, blkid, &hv); if (db == NULL) { blkptr_t *bp = NULL; @@ -3596,7 +3602,7 @@ dbuf_hold_impl(dnode_t *dn, uint8_t level, uint64_t blkid, } if (err && err != ENOENT) return (err); - db = dbuf_create(dn, level, blkid, parent, bp); + db = dbuf_create(dn, level, blkid, parent, bp, hv); } if (fail_uncached && db->db_state != DB_CACHED) { @@ -3680,7 +3686,8 @@ dbuf_create_bonus(dnode_t *dn) ASSERT(RW_WRITE_HELD(&dn->dn_struct_rwlock)); ASSERT(dn->dn_bonus == NULL); - dn->dn_bonus = dbuf_create(dn, 0, DMU_BONUS_BLKID, dn->dn_dbuf, NULL); + dn->dn_bonus = dbuf_create(dn, 0, DMU_BONUS_BLKID, dn->dn_dbuf, NULL, + dbuf_hash(dn->dn_objset, dn->dn_object, 0, DMU_BONUS_BLKID)); } int @@ -3726,7 +3733,7 @@ dbuf_try_add_ref(dmu_buf_t *db_fake, objset_t *os, uint64_t obj, uint64_t blkid, if (blkid == DMU_BONUS_BLKID) found_db = dbuf_find_bonus(os, obj); else - found_db = dbuf_find(os, obj, 0, blkid); + found_db = dbuf_find(os, obj, 0, blkid, NULL); if (found_db != NULL) { if (db == found_db && dbuf_refcount(db) > db->db_dirtycnt) { diff --git a/module/zfs/dnode_sync.c b/module/zfs/dnode_sync.c index 5eabfb833ef0..2ed8058e8e86 100644 --- a/module/zfs/dnode_sync.c +++ b/module/zfs/dnode_sync.c @@ -70,8 +70,8 @@ dnode_increase_indirection(dnode_t *dn, dmu_tx_t *tx) dmu_buf_impl_t *children[DN_MAX_NBLKPTR]; ASSERT3U(nblkptr, <=, DN_MAX_NBLKPTR); for (i = 0; i < nblkptr; i++) { - children[i] = - dbuf_find(dn->dn_objset, dn->dn_object, old_toplvl, i); + children[i] = dbuf_find(dn->dn_objset, dn->dn_object, + old_toplvl, i, NULL); } /* transfer dnode's block pointers to new indirect block */ From 9be34ec99ec7bb518ad508851b0e60c86bfbbd24 Mon Sep 17 00:00:00 2001 From: Ameer Hamza <106930537+ixhamza@users.noreply.github.com> Date: Wed, 14 Dec 2022 06:30:46 +0500 Subject: [PATCH 4/8] Allow receiver to override encryption properties in case of replication Currently, the receiver fails to override the encryption property for the plain replicated dataset with the error: "cannot receive incremental stream: encryption property 'encryption' cannot be set for incremental streams.". The problem is resolved by allowing the receiver to override the encryption property for plain replicated send. Reviewed-by: Ryan Moeller Reviewed-by: Brian Behlendorf Signed-off-by: Ameer Hamza Closes #14253 Closes #13533 --- lib/libzfs/libzfs_sendrecv.c | 12 +++++++++++- .../zfs_receive/zfs_receive_to_encrypted.ksh | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index ac1834733cee..c79c636e16db 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -4150,6 +4150,15 @@ zfs_setup_cmdline_props(libzfs_handle_t *hdl, zfs_type_t type, goto error; } + /* + * For plain replicated send, we can ignore encryption + * properties other than first stream + */ + if ((zfs_prop_encryption_key_param(prop) || prop == + ZFS_PROP_ENCRYPTION) && !newfs && recursive && !raw) { + continue; + } + /* incremental streams can only exclude encryption properties */ if ((zfs_prop_encryption_key_param(prop) || prop == ZFS_PROP_ENCRYPTION) && !newfs && @@ -4251,7 +4260,8 @@ zfs_setup_cmdline_props(libzfs_handle_t *hdl, zfs_type_t type, if (cp != NULL) *cp = '\0'; - if (!raw && zfs_crypto_create(hdl, namebuf, voprops, NULL, + if (!raw && !(!newfs && recursive) && + zfs_crypto_create(hdl, namebuf, voprops, NULL, B_FALSE, wkeydata_out, wkeylen_out) != 0) { fnvlist_free(voprops); ret = zfs_error(hdl, EZFS_CRYPTOFAILED, errbuf); diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_to_encrypted.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_to_encrypted.ksh index 8bd9a6854950..7e12d30d0e7e 100755 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_to_encrypted.ksh +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_receive/zfs_receive_to_encrypted.ksh @@ -41,6 +41,9 @@ verify_runnable "both" function cleanup { + datasetexists $TESTPOOL/encrypted && \ + destroy_dataset $TESTPOOL/encrypted -r + snapexists $snap && destroy_dataset $snap -f snapexists $snap2 && destroy_dataset $snap2 -f @@ -97,4 +100,15 @@ log_note "Verifying ZFS will not receive to an encrypted child when the" \ "parent key is unloaded" log_mustnot eval "zfs send $snap | zfs receive $TESTPOOL/$TESTFS1/c4" +# Verify that replication can override encryption properties +log_note "Verifying replication can override encryption properties for plain dataset" +typeset key_location="/$TESTPOOL/pkey1" +log_must eval "echo $passphrase > $key_location" +log_must eval "zfs send -R $snap2 | zfs recv -s -F -o encryption=on" \ + "-o keyformat=passphrase -o keylocation=file://$key_location" \ + "-o mountpoint=none $TESTPOOL/encrypted" +log_must test "$(get_prop 'encryption' $TESTPOOL/encrypted)" != "off" +log_must test "$(get_prop 'keyformat' $TESTPOOL/encrypted)" == "passphrase" +log_must test "$(get_prop 'keylocation' $TESTPOOL/encrypted)" == "file://$key_location" + log_pass "ZFS can receive encrypted filesystems into child dataset" From f3f5263f8a9b0f8b51051698f68fbd76e181a685 Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Tue, 13 Dec 2022 20:31:47 -0500 Subject: [PATCH 5/8] Zero end of embedded block buffer in dump_write_embedded() This fixes a kernel stack leak. Reviewed-by: Ryan Moeller Reviewed-by: Brian Behlendorf Tested-by: Nicholas Sherlock Signed-off-by: Richard Yao Closes #13778 Closes #14255 --- module/zfs/dmu_send.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c index 5ce2478e5611..bcbc2ba60082 100644 --- a/module/zfs/dmu_send.c +++ b/module/zfs/dmu_send.c @@ -584,7 +584,13 @@ dump_write_embedded(dmu_send_cookie_t *dscp, uint64_t object, uint64_t offset, decode_embedded_bp_compressed(bp, buf); - if (dump_record(dscp, buf, P2ROUNDUP(drrw->drr_psize, 8)) != 0) + uint32_t psize = drrw->drr_psize; + uint32_t rsize = P2ROUNDUP(psize, 8); + + if (psize != rsize) + memset(buf + psize, 0, rsize - psize); + + if (dump_record(dscp, buf, rsize) != 0) return (SET_ERROR(EINTR)); return (0); } From e6e31dd5406d59edaaf5a7eeebd2fb83fb86236f Mon Sep 17 00:00:00 2001 From: Umer Saleem Date: Wed, 14 Dec 2022 06:33:05 +0500 Subject: [PATCH 6/8] Add native-deb* targets to build native Debian packages In continuation of previous #13451, this commits adds native-deb* targets for make to build native debian packages. Github workflows are updated to build and test native Debian packages. Native packages only build with pre-configured paths (see the dh_auto_configure section in contrib/debian/rules.in). While building native packages, paths should not be configured. Initial config flags e.g. '--enable-debug' are replaced in contrib/debian/rules.in. Additional packages on top of existing zfs packages required to build native packages include debhelper-compat, dh-python, dkms, po-debconf, python3-all-dev, python3-sphinx. Reviewed-by: George Melikov Reviewed-by: Ryan Moeller Reviewed-by: Brian Behlendorf Signed-off-by: Umer Saleem Closes #14265 --- .github/workflows/build-dependencies.txt | 6 ++++ .github/workflows/zfs-tests-functional.yml | 11 ++++--- .github/workflows/zfs-tests-sanity.yml | 11 ++++--- config/deb.am | 24 ++++++++++++--- config/zfs-build.m4 | 1 + configure.ac | 2 ++ contrib/debian/.gitignore | 1 + contrib/debian/control | 3 +- contrib/debian/openzfs-libpam-zfs.postinst | 4 ++- contrib/debian/openzfs-zfs-zed.postinst | 7 ----- contrib/debian/openzfs-zfs-zed.prerm | 16 ---------- contrib/debian/openzfs-zfsutils.install | 1 - contrib/debian/openzfs-zfsutils.postinst | 15 --------- contrib/debian/{rules => rules.in} | 4 +-- scripts/debian-packaging.sh | 36 ---------------------- 15 files changed, 50 insertions(+), 92 deletions(-) create mode 100644 contrib/debian/.gitignore delete mode 100644 contrib/debian/openzfs-zfs-zed.prerm rename contrib/debian/{rules => rules.in} (99%) delete mode 100755 scripts/debian-packaging.sh diff --git a/.github/workflows/build-dependencies.txt b/.github/workflows/build-dependencies.txt index 482d82fff17c..73921865c42a 100644 --- a/.github/workflows/build-dependencies.txt +++ b/.github/workflows/build-dependencies.txt @@ -6,6 +6,9 @@ bc build-essential curl dbench +debhelper-compat +dh-python +dkms fakeroot fio gdb @@ -33,12 +36,15 @@ mdadm nfs-kernel-server pamtester parted +po-debconf python3 +python3-all-dev python3-cffi python3-dev python3-packaging python3-pip python3-setuptools +python3-sphinx rng-tools-debian rsync samba diff --git a/.github/workflows/zfs-tests-functional.yml b/.github/workflows/zfs-tests-functional.yml index 69ca539b7bb6..b592a9c13e70 100644 --- a/.github/workflows/zfs-tests-functional.yml +++ b/.github/workflows/zfs-tests-functional.yml @@ -32,15 +32,18 @@ jobs: ./configure --enable-debug --enable-debuginfo --enable-asan --enable-ubsan - name: Make run: | - make -j$(nproc) --no-print-directory --silent pkg-utils pkg-kmod + make --no-print-directory --silent native-deb-utils native-deb-kmod + mv ../*.deb . + rm ./openzfs-zfs-dkms*.deb ./openzfs-zfs-dracut*.deb - name: Install run: | - sudo dpkg -i *.deb # Update order of directories to search for modules, otherwise # Ubuntu will load kernel-shipped ones. sudo sed -i.bak 's/updates/extra updates/' /etc/depmod.d/ubuntu.conf - sudo depmod - sudo modprobe zfs + sudo dpkg -i *.deb + # Native Debian packages enable and start the services + # Stop zfs-zed daemon, as it may interfere with some ZTS test cases + sudo systemctl stop zfs-zed # Workaround for cloud-init bug # see https://github.com/openzfs/zfs/issues/12644 FILE=/lib/udev/rules.d/10-cloud-init-hook-hotplug.rules diff --git a/.github/workflows/zfs-tests-sanity.yml b/.github/workflows/zfs-tests-sanity.yml index f3fc607cb4fc..7ec534f01d01 100644 --- a/.github/workflows/zfs-tests-sanity.yml +++ b/.github/workflows/zfs-tests-sanity.yml @@ -28,15 +28,18 @@ jobs: ./configure --enable-debug --enable-debuginfo --enable-asan --enable-ubsan - name: Make run: | - make -j$(nproc) --no-print-directory --silent pkg-utils pkg-kmod + make --no-print-directory --silent native-deb-utils native-deb-kmod + mv ../*.deb . + rm ./openzfs-zfs-dkms*.deb ./openzfs-zfs-dracut*.deb - name: Install run: | - sudo dpkg -i *.deb # Update order of directories to search for modules, otherwise # Ubuntu will load kernel-shipped ones. sudo sed -i.bak 's/updates/extra updates/' /etc/depmod.d/ubuntu.conf - sudo depmod - sudo modprobe zfs + sudo dpkg -i *.deb + # Native Debian packages enable and start the services + # Stop zfs-zed daemon, as it may interfere with some ZTS test cases + sudo systemctl stop zfs-zed # Workaround for cloud-init bug # see https://github.com/openzfs/zfs/issues/12644 FILE=/lib/udev/rules.d/10-cloud-init-hook-hotplug.rules diff --git a/config/deb.am b/config/deb.am index 0033dd7591ff..76e54acfa8e4 100644 --- a/config/deb.am +++ b/config/deb.am @@ -1,14 +1,17 @@ -PHONY += deb-kmod deb-dkms deb-utils deb deb-local +PHONY += deb-kmod deb-dkms deb-utils deb deb-local native-deb-local \ + native-deb-utils native-deb-kmod native-deb -deb-local: +native-deb-local: @(if test "${HAVE_DPKGBUILD}" = "no"; then \ echo -e "\n" \ "*** Required util ${DPKGBUILD} missing. Please install the\n" \ "*** package for your distribution which provides ${DPKGBUILD},\n" \ "*** re-run configure, and try again.\n"; \ exit 1; \ - fi; \ - if test "${HAVE_ALIEN}" = "no"; then \ + fi) + +deb-local: native-deb-local + @(if test "${HAVE_ALIEN}" = "no"; then \ echo -e "\n" \ "*** Required util ${ALIEN} missing. Please install the\n" \ "*** package for your distribution which provides ${ALIEN},\n" \ @@ -85,3 +88,16 @@ deb-utils: deb-local rpm-utils-initramfs $$pkg8 $$pkg9 $$pkg10 $$pkg11; deb: deb-kmod deb-dkms deb-utils + +debian: + cp -r contrib/debian debian; chmod +x debian/rules; + +native-deb-utils: native-deb-local debian + cp contrib/debian/control debian/control; \ + $(DPKGBUILD) -b -rfakeroot -us -uc; + +native-deb-kmod: native-deb-local debian + sh scripts/make_gitrev.sh; \ + fakeroot debian/rules override_dh_binary-modules; + +native-deb: native-deb-utils native-deb-kmod diff --git a/config/zfs-build.m4 b/config/zfs-build.m4 index bb3c81a647fe..2703e6c016c4 100644 --- a/config/zfs-build.m4 +++ b/config/zfs-build.m4 @@ -464,6 +464,7 @@ AC_DEFUN([ZFS_AC_DPKG], [ AC_SUBST(HAVE_DPKGBUILD) AC_SUBST(DPKGBUILD) AC_SUBST(DPKGBUILD_VERSION) + AC_SUBST([CFGOPTS], ["$CFGOPTS"]) ]) dnl # diff --git a/configure.ac b/configure.ac index 5cb25b32ae2c..4c75616e4299 100644 --- a/configure.ac +++ b/configure.ac @@ -32,6 +32,7 @@ AC_INIT(m4_esyscmd(awk '/^Name:/ {printf $2}' META), m4_esyscmd(awk '/^Version:/ {printf $2}' META)) +CFGOPTS="$*" AC_LANG(C) ZFS_AC_META AC_CONFIG_AUX_DIR([config]) @@ -65,6 +66,7 @@ ZFS_AC_DEBUG_KMEM_TRACKING ZFS_AC_DEBUG_INVARIANTS AC_CONFIG_FILES([ + contrib/debian/rules Makefile include/Makefile lib/libzfs/libzfs.pc diff --git a/contrib/debian/.gitignore b/contrib/debian/.gitignore new file mode 100644 index 000000000000..de7475d8888c --- /dev/null +++ b/contrib/debian/.gitignore @@ -0,0 +1 @@ +rules diff --git a/contrib/debian/control b/contrib/debian/control index a0db4985ed1a..b9bb23b09ba0 100644 --- a/contrib/debian/control +++ b/contrib/debian/control @@ -2,8 +2,7 @@ Source: openzfs-linux Section: contrib/kernel Priority: optional Maintainer: ZFS on Linux specific mailing list -Build-Depends: abigail-tools, - debhelper-compat (= 12), +Build-Depends: debhelper-compat (= 12), dh-python, dkms (>> 2.1.1.2-5), libaio-dev, diff --git a/contrib/debian/openzfs-libpam-zfs.postinst b/contrib/debian/openzfs-libpam-zfs.postinst index 2db86744e4e6..03893454eee9 100644 --- a/contrib/debian/openzfs-libpam-zfs.postinst +++ b/contrib/debian/openzfs-libpam-zfs.postinst @@ -1,6 +1,8 @@ #!/bin/sh set -e -pam-auth-update --package +if ! $(ldd "/lib/$(dpkg-architecture -qDEB_HOST_MULTIARCH)/security/pam_zfs_key.so" | grep -q "libasan") ; then + pam-auth-update --package +fi #DEBHELPER# diff --git a/contrib/debian/openzfs-zfs-zed.postinst b/contrib/debian/openzfs-zfs-zed.postinst index a615eec95760..ac14957a3fe1 100644 --- a/contrib/debian/openzfs-zfs-zed.postinst +++ b/contrib/debian/openzfs-zfs-zed.postinst @@ -4,13 +4,6 @@ set -e zedd="/usr/lib/zfs-linux/zed.d" etcd="/etc/zfs/zed.d" -# enable all default zedlets that are not overridden -while read -r file ; do - etcfile="${etcd}/${file}" - [ -e "${etcfile}" ] && continue - ln -sfT "${zedd}/${file}" "${etcfile}" -done < "${zedd}/DEFAULT-ENABLED" - # remove the overrides created in prerm find "${etcd}" -maxdepth 1 -lname '/dev/null' -delete # remove any dangling symlinks to old zedlets diff --git a/contrib/debian/openzfs-zfs-zed.prerm b/contrib/debian/openzfs-zfs-zed.prerm deleted file mode 100644 index b8340df53438..000000000000 --- a/contrib/debian/openzfs-zfs-zed.prerm +++ /dev/null @@ -1,16 +0,0 @@ -#!/bin/sh -set -e - -zedd="/usr/lib/zfs-linux/zed.d" -etcd="/etc/zfs/zed.d" - -if [ "$1" != "failed-upgrade" ] && [ -d "${etcd}" ] && [ -d "${zedd}" ] ; then - while read -r file ; do - etcfile="${etcd}/${file}" - ( [ -L "${etcfile}" ] || [ -e "${etcfile}" ] ) && continue - ln -sT /dev/null "${etcfile}" - done < "${zedd}/DEFAULT-ENABLED" -fi - -#DEBHELPER# - diff --git a/contrib/debian/openzfs-zfsutils.install b/contrib/debian/openzfs-zfsutils.install index e10a50e012c1..9c7b05451bab 100644 --- a/contrib/debian/openzfs-zfsutils.install +++ b/contrib/debian/openzfs-zfsutils.install @@ -131,5 +131,4 @@ usr/share/man/man8/zstreamdump.8 usr/share/man/man4/spl.4 usr/share/man/man4/zfs.4 usr/share/man/man7/zpool-features.7 -usr/share/man/man7/dracut.zfs.7 usr/share/man/man8/zpool_influxdb.8 diff --git a/contrib/debian/openzfs-zfsutils.postinst b/contrib/debian/openzfs-zfsutils.postinst index b13a78654c37..7dc208d0dd7b 100644 --- a/contrib/debian/openzfs-zfsutils.postinst +++ b/contrib/debian/openzfs-zfsutils.postinst @@ -1,21 +1,6 @@ #!/bin/sh set -e -# The hostname and hostid of the last system to access a ZFS pool are stored in -# the ZFS pool itself. A pool is foreign if, during `zpool import`, the -# current hostname and hostid are different than the stored values thereof. -# -# The only way of having a stable hostid is to define it in /etc/hostid. -# This postinst helper will check if we already have the hostid stabilized by -# checking the existence of the file /etc/hostid to be 4 bytes at least. -# If this file don't already exists on our system or has less than 4 bytes, then -# a new (random) value is generated with zgenhostid (8) and stored in -# /etc/hostid - -if [ ! -f /etc/hostid ] || [ "$(stat -c %s /etc/hostid)" -lt 4 ] ; then - zgenhostid -fi - # When processed to here but zfs kernel module is not loaded, the subsequent # services would fail to start. In this case the installation process just # fails at the postinst stage. The user could do diff --git a/contrib/debian/rules b/contrib/debian/rules.in similarity index 99% rename from contrib/debian/rules rename to contrib/debian/rules.in index 5f4889445bea..63892c6ca243 100755 --- a/contrib/debian/rules +++ b/contrib/debian/rules.in @@ -35,7 +35,7 @@ override_dh_autoreconf: override_dh_auto_configure: @# Build the userland, but don't build the kernel modules. - dh_auto_configure -- \ + dh_auto_configure -- @CFGOPTS@ \ --bindir=/usr/bin \ --sbindir=/sbin \ --libdir=/lib/"$(DEB_HOST_MULTIARCH)" \ @@ -195,7 +195,7 @@ override_dh_prep-deb-files: override_dh_configure_modules: override_dh_configure_modules_stamp override_dh_configure_modules_stamp: - ./configure \ + ./configure @CFGOPTS@ \ --with-config=kernel \ --with-linux=$(KSRC) \ --with-linux-obj=$(KOBJ) diff --git a/scripts/debian-packaging.sh b/scripts/debian-packaging.sh deleted file mode 100755 index 9cd042fa44da..000000000000 --- a/scripts/debian-packaging.sh +++ /dev/null @@ -1,36 +0,0 @@ -#!/bin/bash - -# -# This script can be used to invoke OpenZFS build from native Debian -# packaging. -# - -print_help () -{ - echo "Usage: $(basename $0) [OPTIONS]" - echo - echo "Options:" - echo " -b, --build Build OpenZFS from Debian Packaging" - echo " -c, --clean Clean the workspace" -} - -if [ "$#" -ne 1 ]; then - print_help - exit 1 -fi - -case $1 in - -b|--build) - cp -r contrib/debian debian - debuild -i -us -uc -b && fakeroot debian/rules override_dh_binary-modules - ;; - -c|--clean) - fakeroot debian/rules override_dh_auto_clean - rm -rf debian - ;; - *) - print_help - ;; -esac - -exit 0 From 24502bd3a79d71b726f723ced55b0e7ccb5db840 Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Wed, 14 Dec 2022 01:35:07 +0000 Subject: [PATCH 7/8] FreeBSD: Remove stray debug printf Reviewed-by: Ryan Moeller Reviewed-by: Alexander Motin Reviewed-by: Richard Yao Signed-off-by: Doug Rabson Closes #14286 Closes #14287 --- module/os/freebsd/zfs/kmod_core.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/module/os/freebsd/zfs/kmod_core.c b/module/os/freebsd/zfs/kmod_core.c index 6885d565e26c..a1b1c8e4e6d9 100644 --- a/module/os/freebsd/zfs/kmod_core.c +++ b/module/os/freebsd/zfs/kmod_core.c @@ -133,18 +133,15 @@ zfsdev_ioctl(struct cdev *dev, ulong_t zcmd, caddr_t arg, int flag, len = IOCPARM_LEN(zcmd); vecnum = zcmd & 0xff; zp = (void *)arg; - uaddr = (void *)zp->zfs_cmd; error = 0; #ifdef ZFS_LEGACY_SUPPORT zcl = NULL; #endif - if (len != sizeof (zfs_iocparm_t)) { - printf("len %d vecnum: %d sizeof (zfs_cmd_t) %ju\n", - len, vecnum, (uintmax_t)sizeof (zfs_cmd_t)); + if (len != sizeof (zfs_iocparm_t)) return (EINVAL); - } + uaddr = (void *)zp->zfs_cmd; zc = kmem_zalloc(sizeof (zfs_cmd_t), KM_SLEEP); #ifdef ZFS_LEGACY_SUPPORT /* From fb11b1570a473dfe864fcac71b5220a28503fa5d Mon Sep 17 00:00:00 2001 From: Ethan Coe-Renner Date: Mon, 12 Dec 2022 15:30:51 -0800 Subject: [PATCH 8/8] Add color output to zfs diff. This adds support to color zfs diff (in the style of git diff) conditional on the ZFS_COLOR environment variable. Signed-off-by: Ethan Coe-Renner --- include/libzutil.h | 2 ++ lib/libzfs/libzfs_diff.c | 40 ++++++++++++++++++++++++++++++++++++++++ lib/libzfs/libzfs_util.c | 9 +++++++-- man/man8/zfs.8 | 4 ++++ 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/include/libzutil.h b/include/libzutil.h index 617dd0cd1715..e285183e082c 100644 --- a/include/libzutil.h +++ b/include/libzutil.h @@ -170,7 +170,9 @@ struct zfs_cmd; * List of colors to use */ #define ANSI_RED "\033[0;31m" +#define ANSI_GREEN "\033[0;32m" #define ANSI_YELLOW "\033[0;33m" +#define ANSI_BLUE "\033[0;34m" #define ANSI_RESET "\033[0m" #define ANSI_BOLD "\033[1m" diff --git a/lib/libzfs/libzfs_diff.c b/lib/libzfs/libzfs_diff.c index 84e140ede665..1330e7c3052a 100644 --- a/lib/libzfs/libzfs_diff.c +++ b/lib/libzfs/libzfs_diff.c @@ -44,6 +44,7 @@ #include #include #include +#include #include "libzfs_impl.h" #define ZDIFF_SNAPDIR "/.zfs/snapshot/" @@ -54,6 +55,10 @@ #define ZDIFF_REMOVED '-' #define ZDIFF_RENAMED "R" +#define ZDIFF_ADDED_COLOR ANSI_GREEN +#define ZDIFF_MODIFIED_COLOR ANSI_YELLOW +#define ZDIFF_REMOVED_COLOR ANSI_RED +#define ZDIFF_RENAMED_COLOR ANSI_BLUE /* * Given a {dsname, object id}, get the object path @@ -128,6 +133,25 @@ stream_bytes(FILE *fp, const char *string) } } +/* + * Takes the type of change (like `print_file`), outputs the appropriate color + */ +static const char * +type_to_color(char type) +{ + if (type == '+') + return (ZDIFF_ADDED_COLOR); + else if (type == '-') + return (ZDIFF_REMOVED_COLOR); + else if (type == 'M') + return (ZDIFF_MODIFIED_COLOR); + else if (type == 'R') + return (ZDIFF_RENAMED_COLOR); + else + return (NULL); +} + + static char get_what(mode_t what) { @@ -175,6 +199,8 @@ static void print_rename(FILE *fp, differ_info_t *di, const char *old, const char *new, zfs_stat_t *isb) { + if (isatty(fileno(fp))) + color_start(ZDIFF_RENAMED_COLOR); if (di->timestamped) (void) fprintf(fp, "%10lld.%09lld\t", (longlong_t)isb->zs_ctime[0], @@ -186,12 +212,18 @@ print_rename(FILE *fp, differ_info_t *di, const char *old, const char *new, (void) fputs(di->scripted ? "\t" : " -> ", fp); print_cmn(fp, di, new); (void) fputc('\n', fp); + + if (isatty(fileno(fp))) + color_end(); } static void print_link_change(FILE *fp, differ_info_t *di, int delta, const char *file, zfs_stat_t *isb) { + if (isatty(fileno(fp))) + color_start(ZDIFF_MODIFIED_COLOR); + if (di->timestamped) (void) fprintf(fp, "%10lld.%09lld\t", (longlong_t)isb->zs_ctime[0], @@ -201,12 +233,17 @@ print_link_change(FILE *fp, differ_info_t *di, int delta, const char *file, (void) fprintf(fp, "%c\t", get_what(isb->zs_mode)); print_cmn(fp, di, file); (void) fprintf(fp, "\t(%+d)\n", delta); + if (isatty(fileno(fp))) + color_end(); } static void print_file(FILE *fp, differ_info_t *di, char type, const char *file, zfs_stat_t *isb) { + if (isatty(fileno(fp))) + color_start(type_to_color(type)); + if (di->timestamped) (void) fprintf(fp, "%10lld.%09lld\t", (longlong_t)isb->zs_ctime[0], @@ -216,6 +253,9 @@ print_file(FILE *fp, differ_info_t *di, char type, const char *file, (void) fprintf(fp, "%c\t", get_what(isb->zs_mode)); print_cmn(fp, di, file); (void) fputc('\n', fp); + + if (isatty(fileno(fp))) + color_end(); } static int diff --git a/lib/libzfs/libzfs_util.c b/lib/libzfs/libzfs_util.c index b4679dbb36fd..c31f18009e0c 100644 --- a/lib/libzfs/libzfs_util.c +++ b/lib/libzfs/libzfs_util.c @@ -2010,15 +2010,20 @@ use_color(void) void color_start(const char *color) { - if (use_color()) + if (use_color()) { fputs(color, stdout); + fflush(stdout); + } } void color_end(void) { - if (use_color()) + if (use_color()) { fputs(ANSI_RESET, stdout); + fflush(stdout); + } + } /* printf() with a color. If color is NULL, then do a normal printf. */ diff --git a/man/man8/zfs.8 b/man/man8/zfs.8 index 52c07925764c..d12377f9b4f2 100644 --- a/man/man8/zfs.8 +++ b/man/man8/zfs.8 @@ -737,6 +737,10 @@ command will be undone if the share is ever unshared (like via a reboot). . .Sh ENVIRONMENT VARIABLES .Bl -tag -width "ZFS_MODULE_TIMEOUT" +.It Sy ZFS_COLOR +Use ANSI color in +.Nm zfs Cm diff +output. .It Sy ZFS_MOUNT_HELPER Cause .Nm zfs Cm mount