>Number: 144720 >Category: kern >Synopsis: [zfs][patch] zfs list improvements (vendor patches) >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: update >Submitter-Id: current-users >Arrival-Date: Sat Mar 13 22:40:00 UTC 2010 >Closed-Date: >Last-Modified: >Originator: Martin Matuska >Release: FreeBSD 8-STABLE amd64 >Organization: >Environment: System: FreeBSD 8-STABLE amd64 >Description: I suggest importing the following vendor changes from OpenSolaris to FreeBSD:
1. OpenSolaris onnv changeset #8802 - fix for OpenSolaris bug #6773366 (improves "zfs list" memory consumption) 2. OpenSolaris onnv changeset #9365 - depth option (-d) for zfs list and zfs get (OpenSolaris bug #6762432) 3. Partial fix from OpenSolaris onnv changeset #9396 - fix for OpenSolaris bug #6830813, assertion fail in libzfs/common/libzfs_dataset.c, introduced in changeset #8802 Explanation for these changes: 1. onnv changeset #8802 improves "zfs list" memory consumption by retrieving only the properties of requested datasets, this speeds up zfs list on systems with a large number of datasets 2. the -d property is a great simplification for scripting with ZFS, e.g. it is now possible to lists all snapshots of a single dataset and not also snapshots of all children datasets - this narrows down the number of returned data and speeds up "zfs list" once again 3. the fix is required for #9365 to work correctly with #8802 Additional comments: - all patches apply cleanly on the code (if applied in chronological order) - tested on FreeBSD 8 as well - modifications touch only the zfs command, so changes do not require a reboot, there are no changes in the filesystem or kernel strucures - Aggregated patch against HEAD is included in this pr. References: 1. http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6773366 2. http://www.opensolaris.org/os/community/arc/caselog/2009/171 http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6762432 3. http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6830813 Individual patches: http://mfsbsd.vx.sk/patches/8802.patch http://mfsbsd.vx.sk/patches/9365.patch http://mfsbsd.vx.sk/patches/9396-6830813.patch >How-To-Repeat: >Fix: Index: opensolaris/cmd/zfs/zfs_iter.c =================================================================== --- opensolaris/cmd/zfs/zfs_iter.c (revision 205133) +++ opensolaris/cmd/zfs/zfs_iter.c (working copy) @@ -19,7 +19,7 @@ * CDDL HEADER END */ /* - * Copyright 2008 Sun Microsystems, Inc. All rights reserved. + * Copyright 2009 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ @@ -53,11 +53,14 @@ } zfs_node_t; typedef struct callback_data { - uu_avl_t *cb_avl; - int cb_flags; - zfs_type_t cb_types; - zfs_sort_column_t *cb_sortcol; - zprop_list_t **cb_proplist; + uu_avl_t *cb_avl; + int cb_flags; + zfs_type_t cb_types; + zfs_sort_column_t *cb_sortcol; + zprop_list_t **cb_proplist; + int cb_depth_limit; + int cb_depth; + uint8_t cb_props_table[ZFS_NUM_PROPS]; } callback_data_t; uu_avl_pool_t *avl_pool; @@ -98,10 +101,17 @@ uu_avl_node_init(node, &node->zn_avlnode, avl_pool); if (uu_avl_find(cb->cb_avl, node, cb->cb_sortcol, &idx) == NULL) { - if (cb->cb_proplist && - zfs_expand_proplist(zhp, cb->cb_proplist) != 0) { - free(node); - return (-1); + if (cb->cb_proplist) { + if ((*cb->cb_proplist) && + !(*cb->cb_proplist)->pl_all) + zfs_prune_proplist(zhp, + cb->cb_props_table); + + if (zfs_expand_proplist(zhp, cb->cb_proplist) + != 0) { + free(node); + return (-1); + } } uu_avl_insert(cb->cb_avl, node, idx); dontclose = 1; @@ -113,11 +123,15 @@ /* * Recurse if necessary. */ - if (cb->cb_flags & ZFS_ITER_RECURSE) { + if (cb->cb_flags & ZFS_ITER_RECURSE && + ((cb->cb_flags & ZFS_ITER_DEPTH_LIMIT) == 0 || + cb->cb_depth < cb->cb_depth_limit)) { + cb->cb_depth++; if (zfs_get_type(zhp) == ZFS_TYPE_FILESYSTEM) (void) zfs_iter_filesystems(zhp, zfs_callback, data); if ((zfs_get_type(zhp) != ZFS_TYPE_SNAPSHOT) && include_snaps) (void) zfs_iter_snapshots(zhp, zfs_callback, data); + cb->cb_depth--; } if (!dontclose) @@ -325,10 +339,10 @@ int zfs_for_each(int argc, char **argv, int flags, zfs_type_t types, - zfs_sort_column_t *sortcol, zprop_list_t **proplist, + zfs_sort_column_t *sortcol, zprop_list_t **proplist, int limit, zfs_iter_f callback, void *data) { - callback_data_t cb; + callback_data_t cb = {0}; int ret = 0; zfs_node_t *node; uu_avl_walk_t *walk; @@ -346,6 +360,45 @@ cb.cb_flags = flags; cb.cb_proplist = proplist; cb.cb_types = types; + cb.cb_depth_limit = limit; + /* + * If cb_proplist is provided then in the zfs_handles created we + * retain only those properties listed in cb_proplist and sortcol. + * The rest are pruned. So, the caller should make sure that no other + * properties other than those listed in cb_proplist/sortcol are + * accessed. + * + * If cb_proplist is NULL then we retain all the properties. We + * always retain the zoned property, which some other properties + * need (userquota & friends), and the createtxg property, which + * we need to sort snapshots. + */ + if (cb.cb_proplist && *cb.cb_proplist) { + zprop_list_t *p = *cb.cb_proplist; + + while (p) { + if (p->pl_prop >= ZFS_PROP_TYPE && + p->pl_prop < ZFS_NUM_PROPS) { + cb.cb_props_table[p->pl_prop] = B_TRUE; + } + p = p->pl_next; + } + + while (sortcol) { + if (sortcol->sc_prop >= ZFS_PROP_TYPE && + sortcol->sc_prop < ZFS_NUM_PROPS) { + cb.cb_props_table[sortcol->sc_prop] = B_TRUE; + } + sortcol = sortcol->sc_next; + } + + cb.cb_props_table[ZFS_PROP_ZONED] = B_TRUE; + cb.cb_props_table[ZFS_PROP_CREATETXG] = B_TRUE; + } else { + (void) memset(cb.cb_props_table, B_TRUE, + sizeof (cb.cb_props_table)); + } + if ((cb.cb_avl = uu_avl_create(avl_pool, NULL, UU_DEFAULT)) == NULL) { (void) fprintf(stderr, gettext("internal error: out of memory\n")); Index: opensolaris/cmd/zfs/zfs_main.c =================================================================== --- opensolaris/cmd/zfs/zfs_main.c (revision 205133) +++ opensolaris/cmd/zfs/zfs_main.c (working copy) @@ -190,8 +190,8 @@ return (gettext("\tdestroy [-rRf] " "<filesystem|volume|snapshot>\n")); case HELP_GET: - return (gettext("\tget [-rHp] [-o field[,...]] " - "[-s source[,...]]\n" + return (gettext("\tget [-rHp] [-d max] " + "[-o field[,...]] [-s source[,...]]\n" "\t <\"all\" | property[,...]> " "[filesystem|volume|snapshot] ...\n")); case HELP_INHERIT: @@ -205,8 +205,8 @@ case HELP_UNJAIL: return (gettext("\tunjail <jailid> <filesystem>\n")); case HELP_LIST: - return (gettext("\tlist [-rH] [-o property[,...]] " - "[-t type[,...]] [-s property] ...\n" + return (gettext("\tlist [-rH][-d max] " + "[-o property[,...]] [-t type[,...]] [-s property] ...\n" "\t [-S property] ... " "[filesystem|volume|snapshot] ...\n")); case HELP_MOUNT: @@ -432,6 +432,27 @@ } +static int +parse_depth(char *opt, int *flags) +{ + char *tmp; + int depth; + + depth = (int)strtol(opt, &tmp, 0); + if (*tmp) { + (void) fprintf(stderr, + gettext("%s is not an integer\n"), optarg); + usage(B_FALSE); + } + if (depth < 0) { + (void) fprintf(stderr, + gettext("Depth can not be negative.\n")); + usage(B_FALSE); + } + *flags |= (ZFS_ITER_DEPTH_LIMIT|ZFS_ITER_RECURSE); + return (depth); +} + /* * zfs clone [-p] [-o prop=value] ... <snap> <fs | vol> * @@ -1119,6 +1140,7 @@ int i, c, flags = 0; char *value, *fields; int ret; + int limit = 0; zprop_list_t fake_name = { 0 }; /* @@ -1132,11 +1154,14 @@ cb.cb_type = ZFS_TYPE_DATASET; /* check options */ - while ((c = getopt(argc, argv, ":o:s:rHp")) != -1) { + while ((c = getopt(argc, argv, ":d:o:s:rHp")) != -1) { switch (c) { case 'p': cb.cb_literal = B_TRUE; break; + case 'd': + limit = parse_depth(optarg, &flags); + break; case 'r': flags |= ZFS_ITER_RECURSE; break; @@ -1267,7 +1292,7 @@ /* run for each object */ ret = zfs_for_each(argc, argv, flags, ZFS_TYPE_DATASET, NULL, - &cb.cb_proplist, get_callback, &cb); + &cb.cb_proplist, limit, get_callback, &cb); if (cb.cb_proplist == &fake_name) zprop_free_list(fake_name.pl_next); @@ -1380,10 +1405,10 @@ if (flags & ZFS_ITER_RECURSE) { ret = zfs_for_each(argc, argv, flags, ZFS_TYPE_DATASET, - NULL, NULL, inherit_recurse_cb, propname); + NULL, NULL, 0, inherit_recurse_cb, propname); } else { ret = zfs_for_each(argc, argv, flags, ZFS_TYPE_DATASET, - NULL, NULL, inherit_cb, propname); + NULL, NULL, 0, inherit_cb, propname); } return (ret); @@ -1578,7 +1603,7 @@ if (cb.cb_version == 0) cb.cb_version = ZPL_VERSION; ret = zfs_for_each(argc, argv, flags, ZFS_TYPE_FILESYSTEM, - NULL, NULL, upgrade_set_callback, &cb); + NULL, NULL, 0, upgrade_set_callback, &cb); (void) printf(gettext("%llu filesystems upgraded\n"), cb.cb_numupgraded); if (cb.cb_numsamegraded) { @@ -1596,14 +1621,14 @@ flags |= ZFS_ITER_RECURSE; ret = zfs_for_each(0, NULL, flags, ZFS_TYPE_FILESYSTEM, - NULL, NULL, upgrade_list_callback, &cb); + NULL, NULL, 0, upgrade_list_callback, &cb); found = cb.cb_foundone; cb.cb_foundone = B_FALSE; cb.cb_newer = B_TRUE; ret = zfs_for_each(0, NULL, flags, ZFS_TYPE_FILESYSTEM, - NULL, NULL, upgrade_list_callback, &cb); + NULL, NULL, 0, upgrade_list_callback, &cb); if (!cb.cb_foundone && !found) { (void) printf(gettext("All filesystems are " @@ -1615,11 +1640,12 @@ } /* - * list [-rH] [-o property[,property]...] [-t type[,type]...] + * list [-r][-d max] [-H] [-o property[,property]...] [-t type[,type]...] * [-s property [-s property]...] [-S property [-S property]...] * <dataset> ... * * -r Recurse over all children + * -d Limit recursion by depth. * -H Scripted mode; elide headers and separate columns by tabs * -o Control which fields to display. * -t Control which object types to display. @@ -1769,16 +1795,20 @@ char *fields = NULL; list_cbdata_t cb = { 0 }; char *value; + int limit = 0; int ret; zfs_sort_column_t *sortcol = NULL; int flags = ZFS_ITER_PROP_LISTSNAPS | ZFS_ITER_ARGS_CAN_BE_PATHS; /* check options */ - while ((c = getopt(argc, argv, ":o:rt:Hs:S:")) != -1) { + while ((c = getopt(argc, argv, ":d:o:rt:Hs:S:")) != -1) { switch (c) { case 'o': fields = optarg; break; + case 'd': + limit = parse_depth(optarg, &flags); + break; case 'r': flags |= ZFS_ITER_RECURSE; break; @@ -1869,7 +1899,7 @@ cb.cb_first = B_TRUE; ret = zfs_for_each(argc, argv, flags, types, sortcol, &cb.cb_proplist, - list_callback, &cb); + limit, list_callback, &cb); zprop_free_list(cb.cb_proplist); zfs_free_sort_columns(sortcol); @@ -2252,7 +2282,7 @@ } ret = zfs_for_each(argc - 2, argv + 2, NULL, - ZFS_TYPE_DATASET, NULL, NULL, set_callback, &cb); + ZFS_TYPE_DATASET, NULL, NULL, 0, set_callback, &cb); return (ret); } @@ -2886,7 +2916,7 @@ flags |= ZFS_ITER_RECURSE; error = zfs_for_each(argc, argv, flags, ZFS_TYPE_FILESYSTEM|ZFS_TYPE_VOLUME, NULL, - NULL, unallow_callback, (void *)zperms); + NULL, 0, unallow_callback, (void *)zperms); if (zperms) nvlist_free(zperms); Index: opensolaris/cmd/zfs/zfs_iter.h =================================================================== --- opensolaris/cmd/zfs/zfs_iter.h (revision 205133) +++ opensolaris/cmd/zfs/zfs_iter.h (working copy) @@ -19,7 +19,7 @@ * CDDL HEADER END */ /* - * Copyright 2008 Sun Microsystems, Inc. All rights reserved. + * Copyright 2009 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ @@ -41,9 +41,10 @@ #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) int zfs_for_each(int, char **, int options, zfs_type_t, - zfs_sort_column_t *, zprop_list_t **, zfs_iter_f, void *); + 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 *); Index: opensolaris/lib/libzfs/common/libzfs.h =================================================================== --- opensolaris/lib/libzfs/common/libzfs.h (revision 205133) +++ opensolaris/lib/libzfs/common/libzfs.h (working copy) @@ -369,6 +369,7 @@ } zprop_list_t; extern int zfs_expand_proplist(zfs_handle_t *, zprop_list_t **); +extern void zfs_prune_proplist(zfs_handle_t *, uint8_t *); #define ZFS_MOUNTPOINT_NONE "none" #define ZFS_MOUNTPOINT_LEGACY "legacy" Index: opensolaris/lib/libzfs/common/libzfs_dataset.c =================================================================== --- opensolaris/lib/libzfs/common/libzfs_dataset.c (revision 205133) +++ opensolaris/lib/libzfs/common/libzfs_dataset.c (working copy) @@ -20,7 +20,7 @@ */ /* - * Copyright 2008 Sun Microsystems, Inc. All rights reserved. + * Copyright 2009 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ @@ -2045,6 +2045,8 @@ verify(nvlist_lookup_uint64(nv, ZPROP_VALUE, &value) == 0); (void) nvlist_lookup_string(nv, ZPROP_SOURCE, source); } else { + verify(!zhp->zfs_props_table || + zhp->zfs_props_table[prop] == B_TRUE); value = zfs_prop_default_numeric(prop); *source = ""; } @@ -2064,6 +2066,8 @@ verify(nvlist_lookup_string(nv, ZPROP_VALUE, &value) == 0); (void) nvlist_lookup_string(nv, ZPROP_SOURCE, source); } else { + verify(!zhp->zfs_props_table || + zhp->zfs_props_table[prop] == B_TRUE); if ((value = (char *)zfs_prop_default_string(prop)) == NULL) value = ""; *source = ""; @@ -4267,6 +4271,30 @@ return (error); } +void +zfs_prune_proplist(zfs_handle_t *zhp, uint8_t *props) +{ + nvpair_t *curr; + + /* + * Keep a reference to the props-table against which we prune the + * properties. + */ + zhp->zfs_props_table = props; + + curr = nvlist_next_nvpair(zhp->zfs_props, NULL); + + while (curr) { + zfs_prop_t zfs_prop = zfs_name_to_prop(nvpair_name(curr)); + nvpair_t *next = nvlist_next_nvpair(zhp->zfs_props, curr); + + if (props[zfs_prop] == B_FALSE) + (void) nvlist_remove(zhp->zfs_props, + nvpair_name(curr), nvpair_type(curr)); + curr = next; + } +} + /* * Attach/detach the given filesystem to/from the given jail. */ Index: opensolaris/lib/libzfs/common/libzfs_impl.h =================================================================== --- opensolaris/lib/libzfs/common/libzfs_impl.h (revision 205133) +++ opensolaris/lib/libzfs/common/libzfs_impl.h (working copy) @@ -20,7 +20,7 @@ */ /* - * Copyright 2008 Sun Microsystems, Inc. All rights reserved. + * Copyright 2009 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ @@ -77,6 +77,7 @@ nvlist_t *zfs_user_props; boolean_t zfs_mntcheck; char *zfs_mntopts; + uint8_t *zfs_props_table; }; /* >Release-Note: >Audit-Trail: >Unformatted: _______________________________________________ freebsd-bugs@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-bugs To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"
