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
(0cee240) 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 to work for filesystems as well as snapshots.
This 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.

Additional optimizations could be made by making some
internal processes, like listing snapshots for deletion
use this "fast" mode, since only the name needs to be
looked up, but that is left as future work.

Signed-off-by: Allan Jude <allan@klarasystems.com>

.github/workflows/checkstyle.yaml

@@ -1,7 +1,6 @@
 name: checkstyle
 
 on:
-  push:
   pull_request_target:
 
 jobs:

cmd/zfs/zfs_iter.h

@@ -40,14 +40,6 @@ 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);

cmd/zfs/zfs_main.c

@@ -1391,7 +1391,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) {
@@ -1417,7 +1417,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);
 	}
 
@@ -1435,7 +1435,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)
@@ -1448,7 +1449,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);
@@ -1473,7 +1474,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);
@@ -1684,7 +1685,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;
@@ -1695,7 +1696,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;
@@ -3907,7 +3908,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);
@@ -4006,10 +4007,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)
@@ -4151,7 +4152,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);
 }
@@ -6119,7 +6120,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;
 		}
@@ -6482,7 +6483,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);
 	}

cmd/zpool/zpool_main.c

@@ -8470,7 +8470,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);
 

include/libzfs.h

@@ -605,10 +605,19 @@ 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 *);
 extern int zfs_iter_root(libzfs_handle_t *, zfs_iter_f, void *);
 extern int zfs_iter_children(zfs_handle_t *, int, zfs_iter_f, void *);
-extern int zfs_iter_dependents(zfs_handle_t *, int, zfs_iter_f, void *);
+extern int zfs_iter_dependents(zfs_handle_t *, int, boolean_t, zfs_iter_f,
+    void *);
 extern int zfs_iter_filesystems(zfs_handle_t *, int, zfs_iter_f, void *);
 extern int zfs_iter_snapshots(zfs_handle_t *, int, zfs_iter_f, void *,
     uint64_t, uint64_t);

lib/libzfs/libzfs_changelist.c

@@ -551,7 +551,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);
 	}

lib/libzfs/libzfs_crypto.c

@@ -1035,7 +1035,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 */

lib/libzfs/libzfs_dataset.c

@@ -530,10 +530,17 @@ make_dataset_simple_handle_zc(zfs_handle_t *pzhp, zfs_cmd_t *zc)
 
 	zhp->zfs_hdl = pzhp->zfs_hdl;
 	(void) strlcpy(zhp->zfs_name, zc->zc_name, sizeof (zhp->zfs_name));
-	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; /* structure assignment */
+	zhp->zfs_head_type = pzhp->zfs_type;
+	if (zhp->zfs_dmustats.dds_is_snapshot)
+		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;
+	else
+		abort();	/* we should never see any other types */
 
 	return (zhp);
 }

lib/libzfs/libzfs_iter.c

@@ -121,14 +121,16 @@ zfs_iter_filesystems(zfs_handle_t *zhp, int flags, zfs_iter_f func, void *data)
 
 	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);
@@ -181,7 +183,7 @@ zfs_iter_snapshots(zfs_handle_t *zhp, int flags, 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);
@@ -205,6 +207,7 @@ zfs_iter_snapshots(zfs_handle_t *zhp, int flags, zfs_iter_f func,
 int
 zfs_iter_bookmarks(zfs_handle_t *zhp, int flags, zfs_iter_f func, void *data)
 {
+	zfs_cmd_t zc = {"\0"};
 	zfs_handle_t *nzhp;
 	nvlist_t *props = NULL;
 	nvlist_t *bmarks = NULL;
@@ -482,22 +485,23 @@ 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;
 	void *data;
 } iter_dependents_arg_t;
 
 static int
-iter_dependents_cb(zfs_handle_t *zhp, int flags, void *arg)
+iter_dependents_cb(zfs_handle_t *zhp, void *arg)
 {
 	iter_dependents_arg_t *ida = arg;
 	int err = 0;
 	boolean_t first = ida->first;
 	ida->first = B_FALSE;
 
 	if (zhp->zfs_type == ZFS_TYPE_SNAPSHOT) {
-		err = zfs_iter_clones(zhp, flags, 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;
@@ -531,9 +535,10 @@ iter_dependents_cb(zfs_handle_t *zhp, int flags, void *arg)
 		isf.zhp = zhp;
 		isf.next = ida->stack;
 		ida->stack = &isf;
-		err = zfs_iter_filesystems(zhp, flags, iter_dependents_cb, ida);
+		err = zfs_iter_filesystems(zhp, ida->flags,
+		    iter_dependents_cb, ida);
 		if (err == 0)
-			err = zfs_iter_snapshots(zhp, flags,
+			err = zfs_iter_snapshots(zhp, ida->flags,
 			    iter_dependents_cb, ida, 0, 0);
 		ida->stack = isf.next;
 	}
@@ -551,12 +556,13 @@ 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;
 	ida.data = data;
 	ida.first = B_TRUE;
-	return (iter_dependents_cb(zfs_handle_dup(zhp), flags, &ida));
+	return (iter_dependents_cb(zfs_handle_dup(zhp), &ida));
 }
 
 /*

lib/libzfs/libzfs_mount.c

@@ -1080,7 +1080,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);
 	}
@@ -1428,7 +1428,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;
 
 	/*

lib/libzfs/libzfs_sendrecv.c

@@ -586,7 +586,7 @@ 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] = { 0 };
@@ -629,7 +629,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:
 	sd->parent_fromsnap_guid = parent_fromsnap_guid_save;
@@ -1197,7 +1197,7 @@ dump_filesystem(zfs_handle_t *zhp, void *arg)
 		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,
+		rv = zfs_iter_snapshots_sorted(zhp, 0, dump_snapshot, arg,
 		    min_txg, max_txg);
 	} else {
 		char snapname[MAXPATHLEN] = { 0 };
@@ -2938,9 +2938,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);
 }
@@ -2994,9 +2994,9 @@ guid_to_name_redact_snaps(libzfs_handle_t *hdl, const char *parent,
 			continue;
 		int err = guid_to_name_cb(zfs_handle_dup(zhp), &gtnd);
 		if (err != 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 && 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);
 		if (err == EEXIST)
 			return (0);

module/zfs/zfs_ioctl.c

@@ -2035,7 +2035,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);
 		/*