Introduce getting holds and listing bookmarks through ZCP

Consumers of ZFS Channel Programs can now list bookmarks,
and get holds from datasets. A minor-refactoring was also
applied to distinguish between user and system properties
in ZCP.

Reviewed-by: Paul Dagnelie <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Matt Ahrens <[email protected]>
Reviewed-by: Serapheim Dimitropoulos <[email protected]>
Ported-by: Serapheim Dimitropoulos <[email protected]>
Signed-off-by: Dan Kimmel <[email protected]>

OpenZFS-issue: https://illumos.org/issues/8862
Closes #7902 

10 files changed, 593 insertions, 25 deletions

diff --git a/man/man8/zfs-program.8 b/man/man8/zfs-program.8
index 532fda19b..7c58af0d2 100644
--- a/man/man8/zfs-program.8
+++ b/man/man8/zfs-program.8
@@ -457,8 +457,34 @@ dataset (string)
 .Bd -ragged -compact -offset "xxxx"
 Must be a valid filesystem or volume.
 .Ed
+.It Em zfs.list.bookmarks(dataset)
+Iterate through all bookmarks of the given dataset. Each bookmark is returned
+as a string containing the full dataset name, e.g. "pool/fs#bookmark".
+.Pp
+dataset (string)
+.Bd -ragged -compact -offset "xxxx"
+Must be a valid filesystem or volume.
+.Ed
+.It Em zfs.list.holds(snapshot)
+Iterate through all user holds on the given snapshot. Each hold is returned
+as a pair of the hold's tag and the timestamp (in seconds since the epoch) at
+which it was created.
+.Pp
+snapshot (string)
+.Bd -ragged -compact -offset "xxxx"
+Must be a valid snapshot.
+.Ed
 .It Em zfs.list.properties(dataset)
-Iterate through all user properties for the given dataset.
+An alias for zfs.list.user_properties (see relevant entry).
+.Pp
+dataset (string)
+.Bd -ragged -compact -offset "xxxx"
+Must be a valid filesystem, snapshot, or volume.
+.Ed
+.It Em zfs.list.user_properties(dataset)
+Iterate through all user properties for the given dataset. For each
+step of the iteration, output the property name, its value, and its source.
+Throws a Lua error if the dataset is invalid.
 .Pp
 dataset (string)
 .Bd -ragged -compact -offset "xxxx"
diff --git a/module/zfs/zcp_iter.c b/module/zfs/zcp_iter.c
index f26445520..7600e662d 100644
--- a/module/zfs/zcp_iter.c
+++ b/module/zfs/zcp_iter.c
@@ -14,7 +14,7 @@
  */
 
 /*
- * Copyright (c) 2016 by Delphix. All rights reserved.
+ * Copyright (c) 2016, 2018 by Delphix. All rights reserved.
  */
 
 #include <sys/lua/lua.h>
@@ -23,6 +23,7 @@
 #include <sys/dmu.h>
 #include <sys/dsl_prop.h>
 #include <sys/dsl_synctask.h>
+#include <sys/dsl_bookmark.h>
 #include <sys/dsl_dataset.h>
 #include <sys/dsl_pool.h>
 #include <sys/dmu_tx.h>
@@ -124,8 +125,6 @@ zcp_clones_list(lua_State *state)
 {
 	const char *snapname = lua_tostring(state, 1);
 	dsl_pool_t *dp = zcp_run_info(state)->zri_pool;
-	boolean_t issnap;
-	uint64_t dsobj, cursor;
 
 	/*
 	 * zcp_dataset_hold will either successfully return the requested
@@ -135,9 +134,9 @@ zcp_clones_list(lua_State *state)
 	dsl_dataset_t *ds = zcp_dataset_hold(state, dp, snapname, FTAG);
 	if (ds == NULL)
 		return (1); /* not reached; zcp_dataset_hold() longjmp'd */
-	cursor = 0;
-	issnap = ds->ds_is_snapshot;
-	dsobj = ds->ds_object;
+	boolean_t issnap = ds->ds_is_snapshot;
+	uint64_t cursor = 0;
+	uint64_t dsobj = ds->ds_object;
 	dsl_dataset_rele(ds, FTAG);
 
 	if (!issnap) {
@@ -323,7 +322,7 @@ zcp_children_list(lua_State *state)
 }
 
 static int
-zcp_props_list_gc(lua_State *state)
+zcp_user_props_list_gc(lua_State *state)
 {
 	nvlist_t **props = lua_touserdata(state, 1);
 	if (*props != NULL)
@@ -332,7 +331,7 @@ zcp_props_list_gc(lua_State *state)
 }
 
 static int
-zcp_props_iter(lua_State *state)
+zcp_user_props_iter(lua_State *state)
 {
 	char *source, *val;
 	nvlist_t *nvprop;
@@ -361,11 +360,33 @@ zcp_props_iter(lua_State *state)
 	return (3);
 }
 
-static int zcp_props_list(lua_State *);
+static int zcp_user_props_list(lua_State *);
+static zcp_list_info_t zcp_user_props_list_info = {
+	.name = "user_properties",
+	.func = zcp_user_props_list,
+	.gc = zcp_user_props_list_gc,
+	.pargs = {
+	    { .za_name = "filesystem | snapshot | volume",
+	    .za_lua_type = LUA_TSTRING},
+	    {NULL, 0}
+	},
+	.kwargs = {
+	    {NULL, 0}
+	}
+};
+
+/*
+ * 'properties' was the initial name for 'user_properties' seen
+ * above. 'user_properties' is a better name as it distinguishes
+ * these properties from 'system_properties' which are different.
+ * In order to avoid breaking compatibility between different
+ * versions of ZFS, we declare 'properties' as an alias for
+ * 'user_properties'.
+ */
 static zcp_list_info_t zcp_props_list_info = {
 	.name = "properties",
-	.func = zcp_props_list,
-	.gc = zcp_props_list_gc,
+	.func = zcp_user_props_list,
+	.gc = zcp_user_props_list_gc,
 	.pargs = {
 	    { .za_name = "filesystem | snapshot | volume",
 	    .za_lua_type = LUA_TSTRING},
@@ -377,7 +398,7 @@ static zcp_list_info_t zcp_props_list_info = {
 };
 
 static int
-zcp_props_list(lua_State *state)
+zcp_user_props_list(lua_State *state)
 {
 	const char *dsname = lua_tostring(state, 1);
 	dsl_pool_t *dp = zcp_run_info(state)->zri_pool;
@@ -392,23 +413,24 @@ zcp_props_list(lua_State *state)
 	dsl_dataset_rele(ds, FTAG);
 
 	/*
-	 * Set the metatable for the properties list to free it on completion.
+	 * Set the metatable for the properties list to free it on
+	 * completion.
 	 */
-	luaL_getmetatable(state, zcp_props_list_info.name);
+	luaL_getmetatable(state, zcp_user_props_list_info.name);
 	(void) lua_setmetatable(state, -2);
 
 	lua_pushlightuserdata(state, NULL);
-	lua_pushcclosure(state, &zcp_props_iter, 2);
+	lua_pushcclosure(state, &zcp_user_props_iter, 2);
 	return (1);
 }
 
 
 /*
- * Populate nv with all valid properties and their values for the given
+ * Populate nv with all valid system properties and their values for the given
  * dataset.
  */
 static void
-zcp_dataset_props(dsl_dataset_t *ds, nvlist_t *nv)
+zcp_dataset_system_props(dsl_dataset_t *ds, nvlist_t *nv)
 {
 	for (int prop = ZFS_PROP_TYPE; prop < ZFS_NUM_PROPS; prop++) {
 		/* Do not display hidden props */
@@ -435,8 +457,8 @@ static zcp_list_info_t zcp_system_props_list_info = {
 };
 
 /*
- * Get a list of all visble properties and their values for a given dataset.
- * Returned on the stack as a Lua table.
+ * Get a list of all visble system properties and their values for a given
+ * dataset. Returned on the stack as a Lua table.
  */
 static int
 zcp_system_props_list(lua_State *state)
@@ -454,8 +476,8 @@ zcp_system_props_list(lua_State *state)
 	if (ds == NULL)
 		return (1); /* not reached; zcp_dataset_hold() longjmp'd */
 
-	/* Get the names of all valid properties for this dataset */
-	zcp_dataset_props(ds, nv);
+	/* Get the names of all valid system properties for this dataset */
+	zcp_dataset_system_props(ds, nv);
 	dsl_dataset_rele(ds, FTAG);
 
 	/* push list as lua table */
@@ -469,6 +491,213 @@ zcp_system_props_list(lua_State *state)
 }
 
 static int
+zcp_bookmarks_iter(lua_State *state)
+{
+	char ds_name[ZFS_MAX_DATASET_NAME_LEN];
+	char bookmark_name[ZFS_MAX_DATASET_NAME_LEN];
+	uint64_t dsobj = lua_tonumber(state, lua_upvalueindex(1));
+	uint64_t cursor = lua_tonumber(state, lua_upvalueindex(2));
+	dsl_pool_t *dp = zcp_run_info(state)->zri_pool;
+	dsl_dataset_t *ds;
+	zap_attribute_t za;
+	zap_cursor_t zc;
+
+	int err = dsl_dataset_hold_obj(dp, dsobj, FTAG, &ds);
+	if (err == ENOENT) {
+		return (0);
+	} else if (err != 0) {
+		return (luaL_error(state,
+		    "unexpected error %d from dsl_dataset_hold_obj(dsobj)",
+		    err));
+	}
+
+	if (!dsl_dataset_is_zapified(ds)) {
+		dsl_dataset_rele(ds, FTAG);
+		return (0);
+	}
+
+	err = zap_lookup(dp->dp_meta_objset, ds->ds_object,
+	    DS_FIELD_BOOKMARK_NAMES, sizeof (ds->ds_bookmarks_obj), 1,
+	    &ds->ds_bookmarks_obj);
+	if (err != 0 && err != ENOENT) {
+		dsl_dataset_rele(ds, FTAG);
+		return (luaL_error(state,
+		    "unexpected error %d from zap_lookup()", err));
+	}
+	if (ds->ds_bookmarks_obj == 0) {
+		dsl_dataset_rele(ds, FTAG);
+		return (0);
+	}
+
+	/* Store the dataset's name so we can append the bookmark's name */
+	dsl_dataset_name(ds, ds_name);
+
+	zap_cursor_init_serialized(&zc, ds->ds_dir->dd_pool->dp_meta_objset,
+	    ds->ds_bookmarks_obj, cursor);
+	dsl_dataset_rele(ds, FTAG);
+
+	err = zap_cursor_retrieve(&zc, &za);
+	if (err != 0) {
+		zap_cursor_fini(&zc);
+		if (err != ENOENT) {
+			return (luaL_error(state,
+			    "unexpected error %d from zap_cursor_retrieve()",
+			    err));
+		}
+		return (0);
+	}
+	zap_cursor_advance(&zc);
+	cursor = zap_cursor_serialize(&zc);
+	zap_cursor_fini(&zc);
+
+	/* Create the full "pool/fs#bookmark" string to return */
+	int n = snprintf(bookmark_name, ZFS_MAX_DATASET_NAME_LEN, "%s#%s",
+	    ds_name, za.za_name);
+	if (n >= ZFS_MAX_DATASET_NAME_LEN) {
+		return (luaL_error(state,
+		    "unexpected error %d from snprintf()", ENAMETOOLONG));
+	}
+
+	lua_pushnumber(state, cursor);
+	lua_replace(state, lua_upvalueindex(2));
+
+	(void) lua_pushstring(state, bookmark_name);
+	return (1);
+}
+
+static int zcp_bookmarks_list(lua_State *);
+static zcp_list_info_t zcp_bookmarks_list_info = {
+	.name = "bookmarks",
+	.func = zcp_bookmarks_list,
+	.pargs = {
+	    { .za_name = "dataset", .za_lua_type = LUA_TSTRING},
+	    {NULL, 0}
+	},
+	.kwargs = {
+	    {NULL, 0}
+	}
+};
+
+static int
+zcp_bookmarks_list(lua_State *state)
+{
+	const char *dsname = lua_tostring(state, 1);
+	dsl_pool_t *dp = zcp_run_info(state)->zri_pool;
+
+	dsl_dataset_t *ds = zcp_dataset_hold(state, dp, dsname, FTAG);
+	if (ds == NULL)
+		return (1); /* not reached; zcp_dataset_hold() longjmp'd */
+
+	boolean_t issnap = ds->ds_is_snapshot;
+	uint64_t dsobj = ds->ds_object;
+	uint64_t cursor = 0;
+	dsl_dataset_rele(ds, FTAG);
+
+	if (issnap) {
+		return (zcp_argerror(state, 1, "%s is a snapshot", dsname));
+	}
+
+	lua_pushnumber(state, dsobj);
+	lua_pushnumber(state, cursor);
+	lua_pushcclosure(state, &zcp_bookmarks_iter, 2);
+	return (1);
+}
+
+static int
+zcp_holds_iter(lua_State *state)
+{
+	uint64_t dsobj = lua_tonumber(state, lua_upvalueindex(1));
+	uint64_t cursor = lua_tonumber(state, lua_upvalueindex(2));
+	dsl_pool_t *dp = zcp_run_info(state)->zri_pool;
+	dsl_dataset_t *ds;
+	zap_attribute_t za;
+	zap_cursor_t zc;
+
+	int err = dsl_dataset_hold_obj(dp, dsobj, FTAG, &ds);
+	if (err == ENOENT) {
+		return (0);
+	} else if (err != 0) {
+		return (luaL_error(state,
+		    "unexpected error %d from dsl_dataset_hold_obj(dsobj)",
+		    err));
+	}
+
+	if (dsl_dataset_phys(ds)->ds_userrefs_obj == 0) {
+		dsl_dataset_rele(ds, FTAG);
+		return (0);
+	}
+
+	zap_cursor_init_serialized(&zc, ds->ds_dir->dd_pool->dp_meta_objset,
+	    dsl_dataset_phys(ds)->ds_userrefs_obj, cursor);
+	dsl_dataset_rele(ds, FTAG);
+
+	err = zap_cursor_retrieve(&zc, &za);
+	if (err != 0) {
+		zap_cursor_fini(&zc);
+		if (err != ENOENT) {
+			return (luaL_error(state,
+			    "unexpected error %d from zap_cursor_retrieve()",
+			    err));
+		}
+		return (0);
+	}
+	zap_cursor_advance(&zc);
+	cursor = zap_cursor_serialize(&zc);
+	zap_cursor_fini(&zc);
+
+	lua_pushnumber(state, cursor);
+	lua_replace(state, lua_upvalueindex(2));
+
+	(void) lua_pushstring(state, za.za_name);
+	(void) lua_pushnumber(state, za.za_first_integer);
+	return (2);
+}
+
+static int zcp_holds_list(lua_State *);
+static zcp_list_info_t zcp_holds_list_info = {
+	.name = "holds",
+	.func = zcp_holds_list,
+	.gc = NULL,
+	.pargs = {
+	    { .za_name = "snapshot", .za_lua_type = LUA_TSTRING},
+	    {NULL, 0}
+	},
+	.kwargs = {
+	    {NULL, 0}
+	}
+};
+
+/*
+ * Iterate over all the holds for a given dataset. Each iteration returns
+ * a hold's tag and its timestamp as an integer.
+ */
+static int
+zcp_holds_list(lua_State *state)
+{
+	const char *snapname = lua_tostring(state, 1);
+	dsl_pool_t *dp = zcp_run_info(state)->zri_pool;
+
+	dsl_dataset_t *ds = zcp_dataset_hold(state, dp, snapname, FTAG);
+	if (ds == NULL)
+		return (1); /* not reached; zcp_dataset_hold() longjmp'd */
+
+	boolean_t issnap = ds->ds_is_snapshot;
+	uint64_t dsobj = ds->ds_object;
+	uint64_t cursor = 0;
+	dsl_dataset_rele(ds, FTAG);
+
+	if (!issnap) {
+		return (zcp_argerror(state, 1, "%s is not a snapshot",
+		    snapname));
+	}
+
+	lua_pushnumber(state, dsobj);
+	lua_pushnumber(state, cursor);
+	lua_pushcclosure(state, &zcp_holds_iter, 2);
+	return (1);
+}
+
+static int
 zcp_list_func(lua_State *state)
 {
 	zcp_list_info_t *info = lua_touserdata(state, lua_upvalueindex(1));
@@ -485,9 +714,12 @@ zcp_load_list_lib(lua_State *state)
 	zcp_list_info_t *zcp_list_funcs[] = {
 		&zcp_children_list_info,
 		&zcp_snapshots_list_info,
+		&zcp_user_props_list_info,
 		&zcp_props_list_info,
 		&zcp_clones_list_info,
 		&zcp_system_props_list_info,
+		&zcp_bookmarks_list_info,
+		&zcp_holds_list_info,
 		NULL
 	};
 
diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run
index 23ea63335..f8f04d73e 100644
--- a/tests/runfiles/linux.run
+++ b/tests/runfiles/linux.run
@@ -82,8 +82,9 @@ tags = ['functional', 'channel_program', 'lua_core']
 tests = ['tst.destroy_fs', 'tst.destroy_snap', 'tst.get_count_and_limit',
     'tst.get_index_props', 'tst.get_mountpoint', 'tst.get_neg',
     'tst.get_number_props', 'tst.get_string_props', 'tst.get_type',
-    'tst.get_userquota', 'tst.get_written', 'tst.list_children',
-    'tst.list_clones', 'tst.list_snapshots', 'tst.list_system_props',
+    'tst.get_userquota', 'tst.get_written', 'tst.list_bookmarks',
+    'tst.list_children', 'tst.list_clones', 'tst.list_holds',
+    'tst.list_snapshots', 'tst.list_system_props',
     'tst.list_user_props', 'tst.parse_args_neg','tst.promote_conflict',
     'tst.promote_multiple', 'tst.promote_simple', 'tst.rollback_mult',
     'tst.rollback_one', 'tst.snapshot_destroy', 'tst.snapshot_neg',
diff --git a/tests/zfs-tests/include/libtest.shlib b/tests/zfs-tests/include/libtest.shlib
index 5d9e3b361..5d536fda3 100644
--- a/tests/zfs-tests/include/libtest.shlib
+++ b/tests/zfs-tests/include/libtest.shlib
@@ -742,6 +742,18 @@ function bkmarkexists
 }
 
 #
+# Return 0 if a hold exists; $? otherwise
+#
+# $1 - hold tag
+# $2 - snapshot name
+#
+function holdexists
+{
+	zfs holds "$2" | awk '{ print $2 }' | grep "$1" > /dev/null 2>&1
+	return $?
+}
+
+#
 # Set a property to a certain value on a dataset.
 # Sets a property of the dataset to the value as passed in.
 # @param:
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/Makefile.am b/tests/zfs-tests/tests/functional/channel_program/synctask_core/Makefile.am
index cc86a2db9..d9894f682 100644
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/Makefile.am
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/Makefile.am
@@ -13,8 +13,10 @@ dist_pkgdata_SCRIPTS = \
 	tst.get_type.ksh \
 	tst.get_userquota.ksh \
 	tst.get_written.ksh \
+	tst.list_bookmarks.ksh \
 	tst.list_children.ksh \
 	tst.list_clones.ksh \
+	tst.list_holds.ksh \
 	tst.list_snapshots.ksh \
 	tst.list_system_props.ksh \
 	tst.list_user_props.ksh \
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/cleanup.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/cleanup.ksh
index 281f639a4..59f69ca8b 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/cleanup.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/cleanup.ksh
@@ -17,3 +17,4 @@
 . $STF_SUITE/include/libtest.shlib
 
 default_cleanup
+destroy_pool testpool2
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/setup.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/setup.ksh
index 2516b6b8a..5837bf1a1 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/setup.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/setup.ksh
@@ -18,4 +18,8 @@
 
 DISK=${DISKS%% *}
 
-default_setup ${DISK}
+TESTPOOLDISK=${DISKS%% *}
+TESTPOOL2DISK=${DISKS##* }
+
+default_setup ${TESTPOOLDISK}
+create_pool testpool2 ${TESTPOOL2DISK}
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.list_bookmarks.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.list_bookmarks.ksh
new file mode 100755
index 000000000..7456177f7
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.list_bookmarks.ksh
@@ -0,0 +1,120 @@
+#!/bin/ksh -p
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright (c) 2017 by Delphix. All rights reserved.
+#
+
+. $STF_SUITE/tests/functional/channel_program/channel_common.kshlib
+
+#
+# DESCRIPTION:
+#       Listing zfs bookmarks should work correctly.
+#
+
+verify_runnable "global"
+
+TESTBOOK=$TESTPOOL/$TESTFS#testbook
+TESTBOOK1=$TESTBOOK-1
+TESTBOOK2=$TESTBOOK-2
+TESTBOOK3=$TESTBOOK-3
+
+function cleanup
+{
+	bkmarkexists $TESTBOOK && log_must zfs destroy $TESTBOOK
+	bkmarkexists $TESTBOOK1 && log_must zfs destroy $TESTBOOK1
+	bkmarkexists $TESTBOOK2 && log_must zfs destroy $TESTBOOK2
+	bkmarkexists $TESTBOOK3 && log_must zfs destroy $TESTBOOK3
+	destroy_snapshot
+}
+
+log_onexit cleanup
+
+create_snapshot
+
+# 0 bookmarks handled correctly
+log_must_program $TESTPOOL - <<-EOF
+	n = 0
+	for s in zfs.list.bookmarks("$TESTPOOL/$TESTFS") do
+		n = n + 1
+	end
+	assert(n == 0)
+	return 0
+EOF
+
+# Create a bookmark
+log_must zfs bookmark $TESTPOOL/$TESTFS@$TESTSNAP $TESTBOOK
+
+log_must_program $TESTPOOL - <<-EOF
+	n = 0
+	for s in zfs.list.bookmarks("$TESTPOOL/$TESTFS") do
+		assert(s == "$TESTBOOK")
+		n = n + 1
+	end
+	assert(n == 1)
+	return 0
+EOF
+
+log_must zfs bookmark $TESTPOOL/$TESTFS@$TESTSNAP $TESTBOOK1
+log_must zfs bookmark $TESTPOOL/$TESTFS@$TESTSNAP $TESTBOOK2
+log_must zfs bookmark $TESTPOOL/$TESTFS@$TESTSNAP $TESTBOOK3
+
+# All bookmarks appear exactly once
+log_must_program $TESTPOOL - <<-EOF
+	a = {}
+	a["$TESTBOOK"] = false
+	a["$TESTBOOK1"] = false
+	a["$TESTBOOK2"] = false
+	a["$TESTBOOK3"] = false
+	n = 0
+	for s in zfs.list.bookmarks("$TESTPOOL/$TESTFS") do
+		assert(not a[s])
+		a[s] = true
+		n = n + 1
+	end
+	assert(n == 4)
+	assert(a["$TESTBOOK"] and
+	    a["$TESTBOOK1"] and
+	    a["$TESTBOOK2"] and
+	    a["$TESTBOOK3"])
+	return 0
+EOF
+
+# Nonexistent input
+log_mustnot_program $TESTPOOL - <<-EOF
+	zfs.list.bookmarks("$TESTPOOL/nonexistent-fs")
+	return 0
+EOF
+log_mustnot_program $TESTPOOL - <<-EOF
+	zfs.list.bookmarks("nonexistent-pool/$TESTFS")
+	return 0
+EOF
+
+# Can't look in a different pool than the one specified on command line
+log_mustnot_program $TESTPOOL - <<-EOF
+	zfs.list.bookmarks("testpool2")
+	return 0
+EOF
+
+# Can't have bookmarks on snapshots, only on filesystems
+log_mustnot_program $TESTPOOL - <<-EOF
+	zfs.list.bookmarks("$TESTPOOL/$TESTFS@$TESTSNAP")
+	return 0
+EOF
+
+# Can't have bookmarks on bookmarks, only on filesystems
+log_mustnot_program $TESTPOOL - <<-EOF
+	zfs.list.bookmarks("$TESTBOOK")
+	return 0
+EOF
+
+log_pass "Listing zfs bookmarks should work correctly."
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.list_holds.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.list_holds.ksh
new file mode 100755
index 000000000..2a471bdec
--- /dev/null
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.list_holds.ksh
@@ -0,0 +1,121 @@
+#!/bin/ksh -p
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright (c) 2017 by Delphix. All rights reserved.
+#
+
+. $STF_SUITE/tests/functional/channel_program/channel_common.kshlib
+
+#
+# DESCRIPTION:
+#       Listing zfs holds should work correctly.
+#
+
+verify_runnable "global"
+
+TESTHOLD=testhold-tag
+TESTHOLD1=$TESTHOLD-1
+TESTHOLD2=$TESTHOLD-2
+TESTHOLD3=$TESTHOLD-3
+SNAP=$TESTPOOL/$TESTFS@$TESTSNAP
+
+function cleanup
+{
+	holdexists $TESTHOLD $SNAP && log_must zfs release $TESTHOLD $SNAP
+	holdexists $TESTHOLD1 $SNAP && log_must zfs release $TESTHOLD1 $SNAP
+	holdexists $TESTHOLD2 $SNAP && log_must zfs release $TESTHOLD2 $SNAP
+	holdexists $TESTHOLD3 $SNAP && log_must zfs release $TESTHOLD3 $SNAP
+	destroy_snapshot
+}
+
+log_onexit cleanup
+
+create_snapshot
+
+# 0 holds handled correctly
+log_must_program $TESTPOOL - <<-EOF
+	n = 0
+	for s in zfs.list.holds("$SNAP") do
+		n = n + 1
+	end
+	assert(n == 0)
+	return 0
+EOF
+
+# Create a hold
+log_must zfs hold $TESTHOLD $SNAP
+
+log_must_program $TESTPOOL - <<-EOF
+	n = 0
+	for s in zfs.list.holds("$SNAP") do
+		assert(s == "$TESTHOLD")
+		n = n + 1
+	end
+	assert(n == 1)
+	return 0
+EOF
+
+log_must zfs hold $TESTHOLD1 $SNAP
+log_must zfs hold $TESTHOLD2 $SNAP
+log_must zfs hold $TESTHOLD3 $SNAP
+
+# All holds appear exactly once
+log_must_program $TESTPOOL - <<-EOF
+	a = {}
+	a["$TESTHOLD"] = false
+	a["$TESTHOLD1"] = false
+	a["$TESTHOLD2"] = false
+	a["$TESTHOLD3"] = false
+	n = 0
+	for s in zfs.list.holds("$SNAP") do
+		assert(not a[s])
+		a[s] = true
+		n = n + 1
+	end
+	assert(n == 4)
+	assert(a["$TESTHOLD"] and
+	    a["$TESTHOLD1"] and
+	    a["$TESTHOLD2"] and
+	    a["$TESTHOLD3"])
+	return 0
+EOF
+
+# Nonexistent input
+log_mustnot_program $TESTPOOL - <<-EOF
+	zfs.list.holds("$TESTPOOL/nonexistent-fs@nonexistent-snap")
+	return 0
+EOF
+log_mustnot_program $TESTPOOL - <<-EOF
+	zfs.list.holds("nonexistent-pool/$TESTFS")
+	return 0
+EOF
+
+# Can't look in a different pool than the one specified on command line
+log_mustnot_program $TESTPOOL - <<-EOF
+	zfs.list.holds("testpool2")
+	return 0
+EOF
+
+# Can't have holds on filesystems
+log_mustnot_program $TESTPOOL - <<-EOF
+	zfs.list.holds("$TESTPOOL/$TESTFS")
+	return 0
+EOF
+
+# Can't have holds on bookmarks
+log_mustnot_program $TESTPOOL - <<-EOF
+	zfs.list.holds("$TESTPOOL/$TESTFS#bookmark")
+	return 0
+EOF
+
+log_pass "Listing zfs holds should work correctly."
diff --git a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.list_user_props.ksh b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.list_user_props.ksh
index 910dddc03..34fdbd56d 100755
--- a/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.list_user_props.ksh
+++ b/tests/zfs-tests/tests/functional/channel_program/synctask_core/tst.list_user_props.ksh
@@ -20,6 +20,9 @@
 # DESCRIPTION:
 #       Listing zfs user properties should work correctly.
 #
+#       Note, that this file tests both zfs.list.user_properties
+#       and it's alias zfs.list.properties.
+#
 
 verify_runnable "global"
 
@@ -39,6 +42,14 @@ TESTVAL4="TOZwOfACvQtmDyiq68elB3a3g9YYyxBjSnLtN3ZyQYNOAKykzIE2khKKOBncJiDx"
 # 0 properties handled correctly
 log_must_program $TESTPOOL - <<-EOF
 	n = 0
+	for p in zfs.list.user_properties("$TESTPOOL/$TESTFS") do
+		n = n + 1
+	end
+	assert(n == 0)
+	return 0
+EOF
+log_must_program $TESTPOOL - <<-EOF
+	n = 0
 	for p in zfs.list.properties("$TESTPOOL/$TESTFS") do
 		n = n + 1
 	end
@@ -51,6 +62,16 @@ log_must zfs set $TESTPROP="$TESTVAL" $TESTPOOL/$TESTFS
 
 log_must_program $TESTPOOL - <<-EOF
 	n = 0
+	for p,v in zfs.list.user_properties("$TESTPOOL/$TESTFS") do
+		assert(p == "$TESTPROP")
+		assert(v == "$TESTVAL")
+		n = n + 1
+	end
+	assert(n == 1)
+	return 0
+EOF
+log_must_program $TESTPOOL - <<-EOF
+	n = 0
 	for p,v in zfs.list.properties("$TESTPOOL/$TESTFS") do
 		assert(p == "$TESTPROP")
 		assert(v == "$TESTVAL")
@@ -80,6 +101,34 @@ log_must_program $TESTPOOL - <<-EOF
 	m["$TESTPROP3"] = "$TESTVAL3"
 	m["$TESTPROP4"] = "$TESTVAL4"
 	n = 0
+	for p,v in zfs.list.user_properties("$TESTPOOL/$TESTFS") do
+		assert(not a[p])
+		a[p] = true
+		assert(v == m[p])
+		n = n + 1
+	end
+	assert(n == 5)
+	assert(a["$TESTPROP"] and
+	    a["$TESTPROP1"] and
+	    a["$TESTPROP2"] and
+	    a["$TESTPROP3"] and
+	    a["$TESTPROP4"])
+	return 0
+EOF
+log_must_program $TESTPOOL - <<-EOF
+	a = {}
+	a["$TESTPROP"] = false
+	a["$TESTPROP1"] = false
+	a["$TESTPROP2"] = false
+	a["$TESTPROP3"] = false
+	a["$TESTPROP4"] = false
+	m = {}
+	m["$TESTPROP"] = "$TESTVAL"
+	m["$TESTPROP1"] = "$TESTVAL1"
+	m["$TESTPROP2"] = "$TESTVAL2"
+	m["$TESTPROP3"] = "$TESTVAL3"
+	m["$TESTPROP4"] = "$TESTVAL4"
+	n = 0
 	for p,v in zfs.list.properties("$TESTPOOL/$TESTFS") do
 		assert(not a[p])
 		a[p] = true