Illumos #1884, #3028, #3048, #3049, #3060, #3061, #3093

1884 Empty "used" field for zfs *space commands
3028 zfs {group,user}space -n prints (null) instead of numeric GID/UID
3048 zfs {user,group}space [-s|-S] is broken
3049 zfs {user,group}space -t doesn't really filter the results
3060 zfs {user,group}space -H output isn't tab-delimited
3061 zfs {user,group}space -o doesn't use specified fields order
3093 zfs {user,group}space's -i is noop

Reviewed by: Garry Mills <[email protected]>
Reviewed by: Eric Schrock <[email protected]>
Approved by: Richard Lowe <[email protected]>

References:
  illumos/illumos-gate@89f5d17b06fc4132c983112b24836a779a0ed736
  illumos changeset: 13803:b5e49d71ff0e
  https://www.illumos.org/issues/1884
  https://www.illumos.org/issues/3028
  https://www.illumos.org/issues/3048
  https://www.illumos.org/issues/3049
  https://www.illumos.org/issues/3060
  https://www.illumos.org/issues/3061
  https://www.illumos.org/issues/3093

Ported-by: Brian Behlendorf <[email protected]>
Closes #1194

1 files changed, 363 insertions, 391 deletions

diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c
index 797c7a591..818ab914c 100644
--- a/cmd/zfs/zfs_main.c
+++ b/cmd/zfs/zfs_main.c
@@ -288,13 +288,13 @@ get_usage(zfs_help_t idx)
 		    "\tunallow [-r] -s @setname [<perm|@setname>[,...]] "
 		    "<filesystem|volume>\n"));
 	case HELP_USERSPACE:
-		return (gettext("\tuserspace [-hniHp] [-o field[,...]] "
-		    "[-sS field] ... [-t type[,...]]\n"
-		    "\t    <filesystem|snapshot>\n"));
+		return (gettext("\tuserspace [-Hinp] [-o field[,...]] "
+		    "[-s field] ...\n\t[-S field] ... "
+		    "[-t type[,...]] <filesystem|snapshot>\n"));
 	case HELP_GROUPSPACE:
-		return (gettext("\tgroupspace [-hniHpU] [-o field[,...]] "
-		    "[-sS field] ... [-t type[,...]]\n"
-		    "\t    <filesystem|snapshot>\n"));
+		return (gettext("\tgroupspace [-Hinp] [-o field[,...]] "
+		    "[-s field] ...\n\t[-S field] ... "
+		    "[-t type[,...]] <filesystem|snapshot>\n"));
 	case HELP_HOLD:
 		return (gettext("\thold [-r] <tag> <snapshot> ...\n"));
 	case HELP_HOLDS:
@@ -2043,30 +2043,52 @@ zfs_do_upgrade(int argc, char **argv)
 	return (ret);
 }
 
-#define	USTYPE_USR_BIT (0)
-#define	USTYPE_GRP_BIT (1)
-#define	USTYPE_PSX_BIT (2)
-#define	USTYPE_SMB_BIT (3)
-
-#define	USTYPE_USR (1 << USTYPE_USR_BIT)
-#define	USTYPE_GRP (1 << USTYPE_GRP_BIT)
-
-#define	USTYPE_PSX (1 << USTYPE_PSX_BIT)
-#define	USTYPE_SMB (1 << USTYPE_SMB_BIT)
-
-#define	USTYPE_PSX_USR (USTYPE_PSX | USTYPE_USR)
-#define	USTYPE_SMB_USR (USTYPE_SMB | USTYPE_USR)
-#define	USTYPE_PSX_GRP (USTYPE_PSX | USTYPE_GRP)
-#define	USTYPE_SMB_GRP (USTYPE_SMB | USTYPE_GRP)
-#define	USTYPE_ALL (USTYPE_PSX_USR | USTYPE_SMB_USR \
-		| USTYPE_PSX_GRP | USTYPE_SMB_GRP)
+/*
+ * zfs userspace [-Hinp] [-o field[,...]] [-s field [-s field]...]
+ *               [-S field [-S field]...] [-t type[,...]] filesystem | snapshot
+ * zfs groupspace [-Hinp] [-o field[,...]] [-s field [-s field]...]
+ *                [-S field [-S field]...] [-t type[,...]] filesystem | snapshot
+ *
+ *	-H      Scripted mode; elide headers and separate columns by tabs.
+ *	-i	Translate SID to POSIX ID.
+ *	-n	Print numeric ID instead of user/group name.
+ *	-o      Control which fields to display.
+ *	-p	Use exact (parseable) numeric output.
+ *	-s      Specify sort columns, descending order.
+ *	-S      Specify sort columns, ascending order.
+ *	-t      Control which object types to display.
+ *
+ *	Displays space consumed by, and quotas on, each user in the specified
+ *	filesystem or snapshot.
+ */
 
+/* us_field_types, us_field_hdr and us_field_names should be kept in sync */
+enum us_field_types {
+	USFIELD_TYPE,
+	USFIELD_NAME,
+	USFIELD_USED,
+	USFIELD_QUOTA
+};
+static char *us_field_hdr[] = { "TYPE", "NAME", "USED", "QUOTA" };
+static char *us_field_names[] = { "type", "name", "used", "quota" };
+#define	USFIELD_LAST	(sizeof (us_field_names) / sizeof (char *))
 
-#define	USPROP_USED_BIT (0)
-#define	USPROP_QUOTA_BIT (1)
+#define	USTYPE_PSX_GRP	(1 << 0)
+#define	USTYPE_PSX_USR	(1 << 1)
+#define	USTYPE_SMB_GRP	(1 << 2)
+#define	USTYPE_SMB_USR	(1 << 3)
+#define	USTYPE_ALL	\
+	(USTYPE_PSX_GRP | USTYPE_PSX_USR | USTYPE_SMB_GRP | USTYPE_SMB_USR)
 
-#define	USPROP_USED (1 << USPROP_USED_BIT)
-#define	USPROP_QUOTA (1 << USPROP_QUOTA_BIT)
+static int us_type_bits[] = {
+	USTYPE_PSX_GRP,
+	USTYPE_PSX_USR,
+	USTYPE_SMB_GRP,
+	USTYPE_SMB_USR,
+	USTYPE_ALL
+};
+static char *us_type_names[] = { "posixgroup", "posxiuser", "smbgroup",
+	"smbuser", "all" };
 
 typedef struct us_node {
 	nvlist_t	*usn_nvl;
@@ -2075,37 +2097,49 @@ typedef struct us_node {
 } us_node_t;
 
 typedef struct us_cbdata {
-	nvlist_t		**cb_nvlp;
-	uu_avl_pool_t		*cb_avl_pool;
-	uu_avl_t		*cb_avl;
-	boolean_t		cb_numname;
-	boolean_t		cb_nicenum;
-	boolean_t		cb_sid2posix;
-	zfs_userquota_prop_t	cb_prop;
-	zfs_sort_column_t	*cb_sortcol;
-	size_t			cb_max_typelen;
-	size_t			cb_max_namelen;
-	size_t			cb_max_usedlen;
-	size_t			cb_max_quotalen;
+	nvlist_t	**cb_nvlp;
+	uu_avl_pool_t	*cb_avl_pool;
+	uu_avl_t	*cb_avl;
+	boolean_t	cb_numname;
+	boolean_t	cb_nicenum;
+	boolean_t	cb_sid2posix;
+	zfs_userquota_prop_t cb_prop;
+	zfs_sort_column_t *cb_sortcol;
+	size_t		cb_width[USFIELD_LAST];
 } us_cbdata_t;
 
+static boolean_t us_populated = B_FALSE;
+
 typedef struct {
 	zfs_sort_column_t *si_sortcol;
-	boolean_t si_num_name;
-	boolean_t si_parsable;
+	boolean_t	si_numname;
 } us_sort_info_t;
 
 static int
+us_field_index(char *field)
+{
+	int i;
+
+	for (i = 0; i < USFIELD_LAST; i++) {
+		if (strcmp(field, us_field_names[i]) == 0)
+			return (i);
+	}
+
+	return (-1);
+}
+
+static int
 us_compare(const void *larg, const void *rarg, void *unused)
 {
 	const us_node_t *l = larg;
 	const us_node_t *r = rarg;
-	int rc = 0;
 	us_sort_info_t *si = (us_sort_info_t *)unused;
 	zfs_sort_column_t *sortcol = si->si_sortcol;
-	boolean_t num_name = si->si_num_name;
+	boolean_t numname = si->si_numname;
 	nvlist_t *lnvl = l->usn_nvl;
 	nvlist_t *rnvl = r->usn_nvl;
+	int rc = 0;
+	boolean_t lvb, rvb;
 
 	for (; sortcol != NULL; sortcol = sortcol->sc_next) {
 		char *lvstr = "";
@@ -2124,17 +2158,17 @@ us_compare(const void *larg, const void *rarg, void *unused)
 			(void) nvlist_lookup_uint32(lnvl, propname, &lv32);
 			(void) nvlist_lookup_uint32(rnvl, propname, &rv32);
 			if (rv32 != lv32)
-				rc = (rv32 > lv32) ? 1 : -1;
+				rc = (rv32 < lv32) ? 1 : -1;
 			break;
 		case ZFS_PROP_NAME:
 			propname = "name";
-			if (num_name) {
-				(void) nvlist_lookup_uint32(lnvl, propname,
-				    &lv32);
-				(void) nvlist_lookup_uint32(rnvl, propname,
-				    &rv32);
-				if (rv32 != lv32)
-					rc = (rv32 > lv32) ? 1 : -1;
+			if (numname) {
+				(void) nvlist_lookup_uint64(lnvl, propname,
+				    &lv64);
+				(void) nvlist_lookup_uint64(rnvl, propname,
+				    &rv64);
+				if (rv64 != lv64)
+					rc = (rv64 < lv64) ? 1 : -1;
 			} else {
 				(void) nvlist_lookup_string(lnvl, propname,
 				    &lvstr);
@@ -2143,22 +2177,24 @@ us_compare(const void *larg, const void *rarg, void *unused)
 				rc = strcmp(lvstr, rvstr);
 			}
 			break;
-
 		case ZFS_PROP_USED:
 		case ZFS_PROP_QUOTA:
-			if (ZFS_PROP_USED == prop)
+			if (!us_populated)
+				break;
+			if (prop == ZFS_PROP_USED)
 				propname = "used";
 			else
 				propname = "quota";
 			(void) nvlist_lookup_uint64(lnvl, propname, &lv64);
 			(void) nvlist_lookup_uint64(rnvl, propname, &rv64);
 			if (rv64 != lv64)
-				rc = (rv64 > lv64) ? 1 : -1;
+				rc = (rv64 < lv64) ? 1 : -1;
+			break;
 		default:
 			break;
 		}
 
-		if (rc) {
+		if (rc != 0) {
 			if (rc < 0)
 				return (reverse ? 1 : -1);
 			else
@@ -2166,7 +2202,17 @@ us_compare(const void *larg, const void *rarg, void *unused)
 		}
 	}
 
-	return (rc);
+	/*
+	 * If entries still seem to be the same, check if they are of the same
+	 * type (smbentity is added only if we are doing SID to POSIX ID
+	 * translation where we can have duplicate type/name combinations).
+	 */
+	if (nvlist_lookup_boolean_value(lnvl, "smbentity", &lvb) == 0 &&
+	    nvlist_lookup_boolean_value(rnvl, "smbentity", &rvb) == 0 &&
+	    lvb != rvb)
+		return (lvb < rvb ? -1 : 1);
+
+	return (0);
 }
 
 static inline const char *
@@ -2186,9 +2232,6 @@ us_type2str(unsigned field_type)
 	}
 }
 
-/*
- * zfs userspace
- */
 static int
 userspace_cb(void *arg, const char *domain, uid_t rid, uint64_t space)
 {
@@ -2196,7 +2239,6 @@ userspace_cb(void *arg, const char *domain, uid_t rid, uint64_t space)
 	zfs_userquota_prop_t prop = cb->cb_prop;
 	char *name = NULL;
 	char *propname;
-	char namebuf[32];
 	char sizebuf[32];
 	us_node_t *node;
 	uu_avl_pool_t *avl_pool = cb->cb_avl_pool;
@@ -2205,36 +2247,34 @@ userspace_cb(void *arg, const char *domain, uid_t rid, uint64_t space)
 	nvlist_t *props;
 	us_node_t *n;
 	zfs_sort_column_t *sortcol = cb->cb_sortcol;
-	unsigned type;
+	unsigned type = 0;
 	const char *typestr;
 	size_t namelen;
 	size_t typelen;
 	size_t sizelen;
+	int typeidx, nameidx, sizeidx;
 	us_sort_info_t sortinfo = { sortcol, cb->cb_numname };
+	boolean_t smbentity = B_FALSE;
 
-	if (domain == NULL || domain[0] == '\0') {
-		/* POSIX */
-		if (prop == ZFS_PROP_GROUPUSED || prop == ZFS_PROP_GROUPQUOTA) {
-			type = USTYPE_PSX_GRP;
-			struct group *g = getgrgid(rid);
-			if (g)
-				name = g->gr_name;
-		} else {
-			type = USTYPE_PSX_USR;
-			struct passwd *p = getpwuid(rid);
-			if (p)
-				name = p->pw_name;
-		}
-	} else {
+	if (nvlist_alloc(&props, NV_UNIQUE_NAME, 0) != 0)
+		nomem();
+	node = safe_malloc(sizeof (us_node_t));
+	uu_avl_node_init(node, &node->usn_avlnode, avl_pool);
+	node->usn_nvl = props;
+
+	if (domain != NULL && domain[0] != '\0') {
 #ifdef HAVE_IDMAP
-		char sid[ZFS_MAXNAMELEN+32];
+		/* SMB */
+		char sid[ZFS_MAXNAMELEN + 32];
 		uid_t id;
 		uint64_t classes;
-		int err = 0;
+		int err;
 		directory_error_t e;
 
+		smbentity = B_TRUE;
+
 		(void) snprintf(sid, sizeof (sid), "%s-%u", domain, rid);
-		/* SMB */
+
 		if (prop == ZFS_PROP_GROUPUSED || prop == ZFS_PROP_GROUPQUOTA) {
 			type = USTYPE_SMB_GRP;
 			err = sid_to_id(sid, B_FALSE, &id);
@@ -2245,219 +2285,147 @@ userspace_cb(void *arg, const char *domain, uid_t rid, uint64_t space)
 
 		if (err == 0) {
 			rid = id;
-
-			e = directory_name_from_sid(NULL, sid, &name, &classes);
-			if (e != NULL) {
-				directory_error_free(e);
-				return (NULL);
+			if (!cb->cb_sid2posix) {
+				e = directory_name_from_sid(NULL, sid, &name,
+				    &classes);
+				if (e != NULL) {
+					directory_error_free(e);
+					return (1);
+				}
+				if (name == NULL)
+					name = sid;
 			}
-
-			if (name == NULL)
-				name = sid;
 		}
 #else
+		nvlist_free(props);
+		free(node);
+
 		return (-1);
 #endif /* HAVE_IDMAP */
 	}
 
-/*
- *	if (prop == ZFS_PROP_GROUPUSED || prop == ZFS_PROP_GROUPQUOTA)
- *		ug = "group";
- *	else
- *		ug = "user";
- */
-
-	if (prop == ZFS_PROP_USERUSED || prop == ZFS_PROP_GROUPUSED)
-		propname = "used";
-	else
-		propname = "quota";
-
-	(void) snprintf(namebuf, sizeof (namebuf), "%u", rid);
-	if (name == NULL)
-		name = namebuf;
-
-	if (cb->cb_nicenum)
-		zfs_nicenum(space, sizebuf, sizeof (sizebuf));
-	else
-		(void) sprintf(sizebuf, "%llu", (u_longlong_t)space);
+	if (cb->cb_sid2posix || domain == NULL || domain[0] == '\0') {
+		/* POSIX or -i */
+		if (prop == ZFS_PROP_GROUPUSED || prop == ZFS_PROP_GROUPQUOTA) {
+			type = USTYPE_PSX_GRP;
+			if (!cb->cb_numname) {
+				struct group *g;
 
-	node = safe_malloc(sizeof (us_node_t));
-	uu_avl_node_init(node, &node->usn_avlnode, avl_pool);
+				if ((g = getgrgid(rid)) != NULL)
+					name = g->gr_name;
+			}
+		} else {
+			type = USTYPE_PSX_USR;
+			if (!cb->cb_numname) {
+				struct passwd *p;
 
-	if (nvlist_alloc(&props, NV_UNIQUE_NAME, 0) != 0) {
-		free(node);
-		return (-1);
+				if ((p = getpwuid(rid)) != NULL)
+					name = p->pw_name;
+			}
+		}
 	}
 
+	/*
+	 * Make sure that the type/name combination is unique when doing
+	 * SID to POSIX ID translation (hence changing the type from SMB to
+	 * POSIX).
+	 */
+	if (cb->cb_sid2posix &&
+	    nvlist_add_boolean_value(props, "smbentity", smbentity) != 0)
+		nomem();
+
+	/* Calculate/update width of TYPE field */
+	typestr = us_type2str(type);
+	typelen = strlen(gettext(typestr));
+	typeidx = us_field_index("type");
+	if (typelen > cb->cb_width[typeidx])
+		cb->cb_width[typeidx] = typelen;
 	if (nvlist_add_uint32(props, "type", type) != 0)
 		nomem();
 
-	if (cb->cb_numname) {
-		if (nvlist_add_uint32(props, "name", rid) != 0)
+	/* Calculate/update width of NAME field */
+	if ((cb->cb_numname && cb->cb_sid2posix) || name == NULL) {
+		if (nvlist_add_uint64(props, "name", rid) != 0)
 			nomem();
-		namelen = strlen(namebuf);
+		namelen = snprintf(NULL, 0, "%u", rid);
 	} else {
 		if (nvlist_add_string(props, "name", name) != 0)
 			nomem();
 		namelen = strlen(name);
 	}
+	nameidx = us_field_index("name");
+	if (namelen > cb->cb_width[nameidx])
+		cb->cb_width[nameidx] = namelen;
 
-	typestr = us_type2str(type);
-	typelen = strlen(gettext(typestr));
-	if (typelen > cb->cb_max_typelen)
-		cb->cb_max_typelen  = typelen;
-
-	if (namelen > cb->cb_max_namelen)
-		cb->cb_max_namelen  = namelen;
-
-	sizelen = strlen(sizebuf);
-	if (0 == strcmp(propname, "used")) {
-		if (sizelen > cb->cb_max_usedlen)
-			cb->cb_max_usedlen  = sizelen;
-	} else {
-		if (sizelen > cb->cb_max_quotalen)
-			cb->cb_max_quotalen  = sizelen;
-	}
-
-	node->usn_nvl = props;
-
-	n = uu_avl_find(avl, node, &sortinfo, &idx);
-	if (n == NULL)
+	/*
+	 * Check if this type/name combination is in the list and update it;
+	 * otherwise add new node to the list.
+	 */
+	if ((n = uu_avl_find(avl, node, &sortinfo, &idx)) == NULL) {
 		uu_avl_insert(avl, node, idx);
-	else {
+	} else {
 		nvlist_free(props);
 		free(node);
 		node = n;
 		props = node->usn_nvl;
 	}
 
+	/* Calculate/update width of USED/QUOTA fields */
+	if (cb->cb_nicenum)
+		zfs_nicenum(space, sizebuf, sizeof (sizebuf));
+	else
+		(void) snprintf(sizebuf, sizeof (sizebuf), "%llu",
+		    (u_longlong_t)space);
+	sizelen = strlen(sizebuf);
+	if (prop == ZFS_PROP_USERUSED || prop == ZFS_PROP_GROUPUSED) {
+		propname = "used";
+		if (!nvlist_exists(props, "quota"))
+			(void) nvlist_add_uint64(props, "quota", 0);
+	} else {
+		propname = "quota";
+		if (!nvlist_exists(props, "used"))
+			(void) nvlist_add_uint64(props, "used", 0);
+	}
+	sizeidx = us_field_index(propname);
+	if (sizelen > cb->cb_width[sizeidx])
+		cb->cb_width[sizeidx] = sizelen;
+
 	if (nvlist_add_uint64(props, propname, space) != 0)
 		nomem();
 
 	return (0);
 }
 
-static inline boolean_t
-usprop_check(zfs_userquota_prop_t p, unsigned types, unsigned props)
-{
-	unsigned type;
-	unsigned prop;
-
-	switch (p) {
-	case ZFS_PROP_USERUSED:
-		type = USTYPE_USR;
-		prop = USPROP_USED;
-		break;
-	case ZFS_PROP_USERQUOTA:
-		type = USTYPE_USR;
-		prop = USPROP_QUOTA;
-		break;
-	case ZFS_PROP_GROUPUSED:
-		type = USTYPE_GRP;
-		prop = USPROP_USED;
-		break;
-	case ZFS_PROP_GROUPQUOTA:
-		type = USTYPE_GRP;
-		prop = USPROP_QUOTA;
-		break;
-	default: /* ALL */
-		return (B_TRUE);
-	};
-
-	return (type & types && prop & props);
-}
-
-#define	USFIELD_TYPE (1 << 0)
-#define	USFIELD_NAME (1 << 1)
-#define	USFIELD_USED (1 << 2)
-#define	USFIELD_QUOTA (1 << 3)
-#define	USFIELD_ALL (USFIELD_TYPE | USFIELD_NAME | USFIELD_USED | USFIELD_QUOTA)
-
-static int
-parsefields(unsigned *fieldsp, char **names, unsigned *bits, size_t len)
-{
-	char *field = optarg;
-	char *delim;
-
-	do {
-		int i;
-		boolean_t found = B_FALSE;
-		delim = strchr(field, ',');
-		if (delim != NULL)
-			*delim = '\0';
-
-		for (i = 0; i < len; i++)
-			if (0 == strcmp(field, names[i])) {
-				found = B_TRUE;
-				*fieldsp |= bits[i];
-				break;
-			}
-
-		if (!found) {
-			(void) fprintf(stderr, gettext("invalid type '%s'"
-			    "for -t option\n"), field);
-			return (-1);
-		}
-
-		field = delim + 1;
-	} while (delim);
-
-	return (0);
-}
-
-
-static char *type_names[] = { "posixuser", "smbuser", "posixgroup", "smbgroup",
-	"all" };
-static unsigned type_bits[] = {
-	USTYPE_PSX_USR,
-	USTYPE_SMB_USR,
-	USTYPE_PSX_GRP,
-	USTYPE_SMB_GRP,
-	USTYPE_ALL
-};
-
-static char *us_field_names[] = { "type", "name", "used", "quota" };
-static unsigned us_field_bits[] = {
-	USFIELD_TYPE,
-	USFIELD_NAME,
-	USFIELD_USED,
-	USFIELD_QUOTA
-};
-
 static void
-print_us_node(boolean_t scripted, boolean_t parseable, unsigned fields,
-		size_t type_width, size_t name_width, size_t used_width,
-		size_t quota_width, us_node_t *node)
+print_us_node(boolean_t scripted, boolean_t parsable, int *fields, int types,
+    size_t *width, us_node_t *node)
 {
 	nvlist_t *nvl = node->usn_nvl;
-	nvpair_t *nvp = NULL;
 	char valstr[ZFS_MAXNAMELEN];
 	boolean_t first = B_TRUE;
-	boolean_t quota_found = B_FALSE;
+	int cfield = 0;
+	int field;
+	uint32_t ustype;
 
-	if (fields & USFIELD_QUOTA && !nvlist_exists(nvl, "quota"))
-		if (nvlist_add_string(nvl, "quota", "none") != 0)
-			nomem();
+	/* Check type */
+	(void) nvlist_lookup_uint32(nvl, "type", &ustype);
+	if (!(ustype & types))
+		return;
 
-	while ((nvp = nvlist_next_nvpair(nvl, nvp)) != NULL) {
-		char *pname = nvpair_name(nvp);
-		data_type_t type = nvpair_type(nvp);
-		uint32_t val32 = 0;
-		uint64_t val64 = 0;
+	while ((field = fields[cfield]) != USFIELD_LAST) {
+		nvpair_t *nvp = NULL;
+		data_type_t type;
+		uint32_t val32;
+		uint64_t val64;
 		char *strval = NULL;
-		unsigned field = 0;
-		unsigned width = 0;
-		int i;
-		for (i = 0; i < 4; i++) {
-			if (0 == strcmp(pname, us_field_names[i])) {
-				field = us_field_bits[i];
+
+		while ((nvp = nvlist_next_nvpair(nvl, nvp)) != NULL) {
+			if (strcmp(nvpair_name(nvp),
+			    us_field_names[field]) == 0)
 				break;
-			}
 		}
 
-		if (!(field & fields))
-			continue;
-
+		type = nvpair_type(nvp);
 		switch (type) {
 		case DATA_TYPE_UINT32:
 			(void) nvpair_value_uint32(nvp, &val32);
@@ -2469,20 +2437,12 @@ print_us_node(boolean_t scripted, boolean_t parseable, unsigned fields,
 			(void) nvpair_value_string(nvp, &strval);
 			break;
 		default:
-			(void) fprintf(stderr, "Invalid data type\n");
-		}
-
-		if (!first) {
-			if (scripted)
-				(void) printf("\t");
-			else
-				(void) printf("  ");
+			(void) fprintf(stderr, "invalid data type\n");
 		}
 
 		switch (field) {
 		case USFIELD_TYPE:
 			strval = (char *)us_type2str(val32);
-			width = type_width;
 			break;
 		case USFIELD_NAME:
 			if (type == DATA_TYPE_UINT64) {
@@ -2490,81 +2450,75 @@ print_us_node(boolean_t scripted, boolean_t parseable, unsigned fields,
 				    (u_longlong_t) val64);
 				strval = valstr;
 			}
-			width = name_width;
 			break;
 		case USFIELD_USED:
 		case USFIELD_QUOTA:
 			if (type == DATA_TYPE_UINT64) {
-				(void) nvpair_value_uint64(nvp, &val64);
-				if (parseable)
+				if (parsable) {
 					(void) sprintf(valstr, "%llu",
 					    (u_longlong_t) val64);
-				else
+				} else {
 					zfs_nicenum(val64, valstr,
 					    sizeof (valstr));
-				strval = valstr;
-			}
-
-			if (field == USFIELD_USED)
-				width = used_width;
-			else {
-				quota_found = B_FALSE;
-				width = quota_width;
+				}
+				if (field == USFIELD_QUOTA &&
+				    strcmp(valstr, "0") == 0)
+					strval = "none";
+				else
+					strval = valstr;
 			}
-
 			break;
 		}
 
-		if (field == USFIELD_QUOTA && !quota_found)
-			(void) printf("%*s", width, strval);
-		else {
-			if (type == DATA_TYPE_STRING)
-				(void) printf("%-*s", width, strval);
+		if (!first) {
+			if (scripted)
+				(void) printf("\t");
 			else
-				(void) printf("%*s", width, strval);
+				(void) printf("  ");
 		}
+		if (scripted)
+			(void) printf("%s", strval);
+		else if (field == USFIELD_TYPE || field == USFIELD_NAME)
+			(void) printf("%-*s", (int) width[field], strval);
+		else
+			(void) printf("%*s", (int) width[field], strval);
 
 		first = B_FALSE;
-
+		cfield++;
 	}
 
 	(void) printf("\n");
 }
 
 static void
-print_us(boolean_t scripted, boolean_t parsable, unsigned fields,
-		unsigned type_width, unsigned name_width, unsigned used_width,
-		unsigned quota_width, boolean_t rmnode, uu_avl_t *avl)
+print_us(boolean_t scripted, boolean_t parsable, int *fields, int types,
+    size_t *width, boolean_t rmnode, uu_avl_t *avl)
 {
-	static char *us_field_hdr[] = { "TYPE", "NAME", "USED", "QUOTA" };
 	us_node_t *node;
 	const char *col;
-	int i;
-	int width[4] = { type_width, name_width, used_width, quota_width };
+	int cfield = 0;
+	int field;
 
 	if (!scripted) {
 		boolean_t first = B_TRUE;
-		for (i = 0; i < 4; i++) {
-			unsigned field = us_field_bits[i];
-			if (!(field & fields))
-				continue;
 
-			col = gettext(us_field_hdr[i]);
-			if (field == USFIELD_TYPE || field == USFIELD_NAME)
-				(void) printf(first?"%-*s":"  %-*s", width[i],
-				    col);
-			else
-				(void) printf(first?"%*s":"  %*s", width[i],
-				    col);
+		while ((field = fields[cfield]) != USFIELD_LAST) {
+			col = gettext(us_field_hdr[field]);
+			if (field == USFIELD_TYPE || field == USFIELD_NAME) {
+				(void) printf(first ? "%-*s" : "  %-*s",
+				    (int) width[field], col);
+			} else {
+				(void) printf(first ? "%*s" : "  %*s",
+				    (int) width[field], col);
+			}
 			first = B_FALSE;
+			cfield++;
 		}
 		(void) printf("\n");
 	}
 
-	for (node = uu_avl_first(avl); node != NULL;
-	    node = uu_avl_next(avl, node)) {
-		print_us_node(scripted, parsable, fields, type_width,
-		    name_width, used_width, used_width, node);
+	for (node = uu_avl_first(avl); node; node = uu_avl_next(avl, node)) {
+		print_us_node(scripted, parsable, fields, types, width, node);
 		if (rmnode)
 			nvlist_free(node->usn_nvl);
 	}
@@ -2578,32 +2532,36 @@ zfs_do_userspace(int argc, char **argv)
 	uu_avl_pool_t *avl_pool;
 	uu_avl_t *avl_tree;
 	uu_avl_walk_t *walk;
-
-	char *cmd;
+	char *delim;
+	char deffields[] = "type,name,used,quota";
+	char *ofield = NULL;
+	char *tfield = NULL;
+	int cfield = 0;
+	int fields[256];
+	int i;
 	boolean_t scripted = B_FALSE;
 	boolean_t prtnum = B_FALSE;
-	boolean_t parseable = B_FALSE;
+	boolean_t parsable = B_FALSE;
 	boolean_t sid2posix = B_FALSE;
 	int error = 0;
 	int c;
-	zfs_sort_column_t *default_sortcol = NULL;
 	zfs_sort_column_t *sortcol = NULL;
-	unsigned types = USTYPE_PSX_USR | USTYPE_SMB_USR;
-	unsigned fields = 0;
-	unsigned props = USPROP_USED | USPROP_QUOTA;
+	int types = USTYPE_PSX_USR | USTYPE_SMB_USR;
 	us_cbdata_t cb;
 	us_node_t *node;
-	boolean_t resort_avl = B_FALSE;
+	us_node_t *rmnode;
+	uu_list_pool_t *listpool;
+	uu_list_t *list;
+	uu_avl_index_t idx = 0;
+	uu_list_index_t idx2 = 0;
 
 	if (argc < 2)
 		usage(B_FALSE);
 
-	cmd = argv[0];
-	if (0 == strcmp(cmd, "groupspace"))
-		/* toggle default group types */
+	if (strcmp(argv[0], "groupspace") == 0)
+		/* Toggle default group types */
 		types = USTYPE_PSX_GRP | USTYPE_SMB_GRP;
 
-	/* check options */
 	while ((c = getopt(argc, argv, "nHpo:s:S:t:i")) != -1) {
 		switch (c) {
 		case 'n':
@@ -2613,32 +2571,22 @@ zfs_do_userspace(int argc, char **argv)
 			scripted = B_TRUE;
 			break;
 		case 'p':
-			parseable = B_TRUE;
+			parsable = B_TRUE;
 			break;
 		case 'o':
-			if (parsefields(&fields, us_field_names, us_field_bits,
-			    4) != 0)
-				return (1);
+			ofield = optarg;
 			break;
 		case 's':
-			if (zfs_add_sort_column(&sortcol, optarg,
-			    B_FALSE) != 0) {
-				(void) fprintf(stderr,
-				    gettext("invalid property '%s'\n"), optarg);
-				usage(B_FALSE);
-			}
-			break;
 		case 'S':
 			if (zfs_add_sort_column(&sortcol, optarg,
-			    B_TRUE) != 0) {
+			    c == 's' ? B_FALSE : B_TRUE) != 0) {
 				(void) fprintf(stderr,
-				    gettext("invalid property '%s'\n"), optarg);
+				    gettext("invalid field '%s'\n"), optarg);
 				usage(B_FALSE);
 			}
 			break;
 		case 't':
-			if (parsefields(&types, type_names, type_bits, 5))
-				return (1);
+			tfield = optarg;
 			break;
 		case 'i':
 			sid2posix = B_TRUE;
@@ -2658,104 +2606,128 @@ zfs_do_userspace(int argc, char **argv)
 	argc -= optind;
 	argv += optind;
 
-	/* ok, now we have sorted by default colums (type,name) avl tree */
-	if (sortcol) {
-		zfs_sort_column_t *sc;
-		for (sc = sortcol; sc; sc = sc->sc_next) {
-			if (sc->sc_prop == ZFS_PROP_QUOTA) {
-				resort_avl = B_TRUE;
-				break;
-			}
-		}
+	if (argc < 1) {
+		(void) fprintf(stderr, gettext("missing dataset name\n"));
+		usage(B_FALSE);
+	}
+	if (argc > 1) {
+		(void) fprintf(stderr, gettext("too many arguments\n"));
+		usage(B_FALSE);
 	}
 
-	if (!fields)
-		fields = USFIELD_ALL;
+	/* Use default output fields if not specified using -o */
+	if (ofield == NULL)
+		ofield = deffields;
+	do {
+		if ((delim = strchr(ofield, ',')) != NULL)
+			*delim = '\0';
+		if ((fields[cfield++] = us_field_index(ofield)) == -1) {
+			(void) fprintf(stderr, gettext("invalid type '%s' "
+			    "for -o option\n"), ofield);
+			return (-1);
+		}
+		if (delim != NULL)
+			ofield = delim + 1;
+	} while (delim != NULL);
+	fields[cfield] = USFIELD_LAST;
+
+	/* Override output types (-t option) */
+	if (tfield != NULL) {
+		types = 0;
+
+		do {
+			boolean_t found = B_FALSE;
+
+			if ((delim = strchr(tfield, ',')) != NULL)
+				*delim = '\0';
+			for (i = 0; i < sizeof (us_type_bits) / sizeof (int);
+			    i++) {
+				if (strcmp(tfield, us_type_names[i]) == 0) {
+					found = B_TRUE;
+					types |= us_type_bits[i];
+					break;
+				}
+			}
+			if (!found) {
+				(void) fprintf(stderr, gettext("invalid type "
+				    "'%s' for -t option\n"), tfield);
+				return (-1);
+			}
+			if (delim != NULL)
+				tfield = delim + 1;
+		} while (delim != NULL);
+	}
 
-	if ((zhp = zfs_open(g_zfs, argv[argc-1], ZFS_TYPE_DATASET)) == NULL)
+	if ((zhp = zfs_open(g_zfs, argv[0], ZFS_TYPE_DATASET)) == NULL)
 		return (1);
 
 	if ((avl_pool = uu_avl_pool_create("us_avl_pool", sizeof (us_node_t),
-	    offsetof(us_node_t, usn_avlnode),
-	    us_compare, UU_DEFAULT)) == NULL)
+	    offsetof(us_node_t, usn_avlnode), us_compare, UU_DEFAULT)) == NULL)
 		nomem();
 	if ((avl_tree = uu_avl_create(avl_pool, NULL, UU_DEFAULT)) == NULL)
 		nomem();
 
-	if (sortcol && !resort_avl)
-		cb.cb_sortcol = sortcol;
-	else {
-		(void) zfs_add_sort_column(&default_sortcol, "type", B_FALSE);
-		(void) zfs_add_sort_column(&default_sortcol, "name", B_FALSE);
-		cb.cb_sortcol = default_sortcol;
-	}
+	/* Always add default sorting columns */
+	(void) zfs_add_sort_column(&sortcol, "type", B_FALSE);
+	(void) zfs_add_sort_column(&sortcol, "name", B_FALSE);
+
+	cb.cb_sortcol = sortcol;
 	cb.cb_numname = prtnum;
-	cb.cb_nicenum = !parseable;
+	cb.cb_nicenum = !parsable;
 	cb.cb_avl_pool = avl_pool;
 	cb.cb_avl = avl_tree;
 	cb.cb_sid2posix = sid2posix;
-	cb.cb_max_typelen = strlen(gettext("TYPE"));
-	cb.cb_max_namelen = strlen(gettext("NAME"));
-	cb.cb_max_usedlen = strlen(gettext("USED"));
-	cb.cb_max_quotalen = strlen(gettext("QUOTA"));
+
+	for (i = 0; i < USFIELD_LAST; i++)
+		cb.cb_width[i] = strlen(gettext(us_field_hdr[i]));
 
 	for (p = 0; p < ZFS_NUM_USERQUOTA_PROPS; p++) {
-		if (!usprop_check(p, types, props))
+		if (((p == ZFS_PROP_USERUSED || p == ZFS_PROP_USERQUOTA) &&
+		    !(types & (USTYPE_PSX_USR | USTYPE_SMB_USR))) ||
+		    ((p == ZFS_PROP_GROUPUSED || p == ZFS_PROP_GROUPQUOTA) &&
+		    !(types & (USTYPE_PSX_GRP | USTYPE_SMB_GRP))))
 			continue;
-
 		cb.cb_prop = p;
 		error = zfs_userspace(zhp, p, userspace_cb, &cb);
 		if (error)
 			break;
 	}
 
+	/* Sort the list */
+	us_populated = B_TRUE;
+	listpool = uu_list_pool_create("tmplist", sizeof (us_node_t),
+	    offsetof(us_node_t, usn_listnode), NULL, UU_DEFAULT);
+	list = uu_list_create(listpool, NULL, UU_DEFAULT);
 
-	if (resort_avl) {
-		us_node_t *node;
-		us_node_t *rmnode;
-		uu_list_pool_t *listpool;
-		uu_list_t *list;
-		uu_avl_index_t idx = 0;
-		uu_list_index_t idx2 = 0;
-		listpool = uu_list_pool_create("tmplist", sizeof (us_node_t),
-		    offsetof(us_node_t, usn_listnode), NULL,
-		    UU_DEFAULT);
-		list = uu_list_create(listpool, NULL, UU_DEFAULT);
-
-		node = uu_avl_first(avl_tree);
-		uu_list_node_init(node, &node->usn_listnode, listpool);
-		while (node != NULL) {
-			rmnode = node;
-			node = uu_avl_next(avl_tree, node);
-			uu_avl_remove(avl_tree, rmnode);
-			if (uu_list_find(list, rmnode, NULL, &idx2) == NULL) {
-				uu_list_insert(list, rmnode, idx2);
-			}
-		}
+	node = uu_avl_first(avl_tree);
+	uu_list_node_init(node, &node->usn_listnode, listpool);
 
-		for (node = uu_list_first(list); node != NULL;
-		    node = uu_list_next(list, node)) {
-			us_sort_info_t sortinfo = { sortcol, cb.cb_numname };
-			if (uu_avl_find(avl_tree, node, &sortinfo, &idx) ==
-			    NULL)
-			uu_avl_insert(avl_tree, node, idx);
-		}
+	while (node != NULL) {
+		rmnode = node;
+		node = uu_avl_next(avl_tree, node);
+		uu_avl_remove(avl_tree, rmnode);
+		if (uu_list_find(list, rmnode, NULL, &idx2) == NULL)
+			uu_list_insert(list, rmnode, idx2);
+	}
+
+	for (node = uu_list_first(list); node != NULL;
+	    node = uu_list_next(list, node)) {
+		us_sort_info_t sortinfo = { sortcol, cb.cb_numname };
 
-		uu_list_destroy(list);
+		if (uu_avl_find(avl_tree, node, &sortinfo, &idx) == NULL)
+			uu_avl_insert(avl_tree, node, idx);
 	}
 
-	/* print & free node`s nvlist memory */
-	print_us(scripted, parseable, fields, cb.cb_max_typelen,
-	    cb.cb_max_namelen, cb.cb_max_usedlen,
-	    cb.cb_max_quotalen, B_TRUE, cb.cb_avl);
+	uu_list_destroy(list);
+	uu_list_pool_destroy(listpool);
 
-	if (sortcol)
-		zfs_free_sort_columns(sortcol);
-	zfs_free_sort_columns(default_sortcol);
+	/* Print and free node nvlist memory */
+	print_us(scripted, parsable, fields, types, cb.cb_width, B_TRUE,
+	    cb.cb_avl);
 
-	/*
-	 * Finally, clean up the AVL tree.
-	 */
+	zfs_free_sort_columns(sortcol);
+
+	/* Clean up the AVL tree */
 	if ((walk = uu_avl_walk_start(cb.cb_avl, UU_WALK_ROBUST)) == NULL)
 		nomem();