Print this page
1869 "zfs holds" is O(n^2)
@@ -5360,11 +5360,14 @@
return (zfs_do_hold_rele_impl(argc, argv, B_FALSE));
}
typedef struct holds_cbdata {
boolean_t cb_recursive;
- const char *cb_snapname;
+ const char **cb_snapnames;
+ char **cb_argv_filt;
+ size_t cb_argc_filt;
+ size_t cb_idx;
nvlist_t **cb_nvlp;
size_t cb_max_namelen;
size_t cb_max_taglen;
} holds_cbdata_t;
@@ -5429,21 +5432,51 @@
nvlist_t *nvl = NULL;
nvpair_t *nvp = NULL;
const char *zname = zfs_get_name(zhp);
size_t znamelen = strnlen(zname, ZFS_MAXNAMELEN);
- if (cbp->cb_recursive) {
- const char *snapname;
- char *delim = strchr(zname, '@');
- if (delim == NULL)
+ /*
+ * If not a snapshot then we are running recursive and it
+ * might be one of the arguments. In this case we have to bump
+ * the index so we know which snapname to match the
+ * descendants of this argument against.
+ *
+ * In any case we skip this zfs handle because we know it is
+ * not a hold.
+ */
+ if (zfs_get_type(zhp) != ZFS_TYPE_SNAPSHOT) {
+ if ((cbp->cb_idx < cbp->cb_argc_filt)
+ && (strcmp(cbp->cb_argv_filt[cbp->cb_idx], zname) == 0)) {
+ cbp->cb_idx++;
+ }
return (0);
+ }
- snapname = delim + 1;
- if (strcmp(cbp->cb_snapname, snapname))
+ if (cbp->cb_recursive == B_TRUE) {
+ char *delim = strchr(zname, '@');
+ const char *snapname = delim + 1;
+
+ /*
+ * The snapshot name of this descendent doesn't match,
+ * skip it.
+ */
+ if (strcmp(cbp->cb_snapnames[cbp->cb_idx - 1], snapname) != 0) {
return (0);
}
+ }
+ /*
+ * In this case we know two things are true.
+ *
+ * 1. That zhp is a snapshot.
+ *
+ * 2. If a recursive listing then zhp is descendent of the
+ * user argument with a snapshot name that matches.
+ *
+ * In this case, gegrab the holds on the snapshot and add them
+ * to list.
+ */
if (zfs_get_holds(zhp, &nvl) != 0)
return (-1);
if (znamelen > cbp->cb_max_namelen)
cbp->cb_max_namelen = znamelen;
@@ -5466,11 +5499,13 @@
static int
zfs_do_holds(int argc, char **argv)
{
int errors = 0;
int c;
- int i;
+ char **argv_filt;
+ int argc_filt = 0;
+ const char **snapnames;
boolean_t scripted = B_FALSE;
boolean_t recursive = B_FALSE;
const char *opts = "rH";
nvlist_t *nvl;
@@ -5495,63 +5530,129 @@
optopt);
usage(B_FALSE);
}
}
- if (recursive) {
- types |= ZFS_TYPE_FILESYSTEM | ZFS_TYPE_VOLUME;
- flags |= ZFS_ITER_RECURSE;
- }
-
argc -= optind;
argv += optind;
/* check number of arguments */
if (argc < 1)
usage(B_FALSE);
if (nvlist_alloc(&nvl, NV_UNIQUE_NAME, 0) != 0)
nomem();
- for (i = 0; i < argc; ++i) {
+ if ((argv_filt = malloc(sizeof (char *) * argc)) == NULL)
+ nomem();
+
+ if ((snapnames = malloc(sizeof (char *) * argc)) == NULL)
+ nomem();
+
+ cb.cb_nvlp = &nvl;
+ cb.cb_recursive = B_FALSE;
+ cb.cb_idx = 0;
+
+ if (recursive == B_TRUE) {
+ types |= ZFS_TYPE_FILESYSTEM | ZFS_TYPE_VOLUME;
+ flags |= ZFS_ITER_RECURSE;
+ cb.cb_recursive = B_TRUE;
+ cb.cb_snapnames = malloc(sizeof (char *) * argc);
+
+ if (cb.cb_snapnames == NULL)
+ nomem();
+ }
+
+ /*
+ * Filter out non-snapshot arguments. Store pointers to valid
+ * arguments in argv_filt. The total count of valid arguments
+ * is stored in argc_filt.
+ */
+ for (int i = 0; i < argc; i++) {
char *snapshot = argv[i];
- const char *delim;
- const char *snapname;
+ const char *delim = strchr(snapshot, '@');
- delim = strchr(snapshot, '@');
if (delim == NULL) {
(void) fprintf(stderr,
gettext("'%s' is not a snapshot\n"), snapshot);
++errors;
continue;
}
- snapname = delim + 1;
- if (recursive)
+
+ /*
+ * If running a recusive (-r) listing then the
+ * arguments to zfs_for_each() must each be stripped
+ * of their snapshot suffix. Furthermore, when
+ * descending down for a particular argument, only
+ * snapshots with the same name as the original
+ * argument should be returned. Since we just stripped
+ * the snapname from the arguments they need to be
+ * kept track of somewhere else. This is the purpose
+ * of snapnames. As zfs_for_each() iterates the list
+ * the callback uses cb_argv_filt and cb_snapnames to
+ * make sure that only descendents of the same
+ * snapname are listed.
+ *
+ * If this is confusing then it only means you are
+ * human. The behavior of "zfs holds -r" is very odd.
+ *
+ * For example, given a filesystem layout like so:
+ *
+ * rpool/foo
+ * rpool/foo@1
+ * rpool/foo/bar
+ * rpool/foo/bar@1
+ * rpool/foo/bar@2
+ * rpool/baz
+ * rpool/baz@1
+ * rpool/baz@2
+ * rpool/baz/blah
+ * rpool/baz/blah@2
+ *
+ * "zfs holds -r rpool/foo@1 rpool/baz@2" should list
+ * holds for:
+ *
+ * rpool/foo@1
+ * rpool/foo/bar@1
+ * rpool/baz@2
+ * rpool/baz/blah@2
+ */
+ if (recursive == B_TRUE) {
+ const char *snapname = delim + 1;
snapshot[delim - snapshot] = '\0';
+ snapnames[argc_filt] = snapname;
+ }
- cb.cb_recursive = recursive;
- cb.cb_snapname = snapname;
- cb.cb_nvlp = &nvl;
+ argv_filt[argc_filt++] = snapshot;
+ }
+
+ if (argc_filt == 0)
+ return (0 != errors);
+
+ cb.cb_argv_filt = argv_filt;
+ cb.cb_argc_filt = argc_filt;
+ cb.cb_snapnames = snapnames;
/*
* 1. collect holds data, set format options
*/
- ret = zfs_for_each(argc, argv, flags, types, NULL, NULL, limit,
- holds_callback, &cb);
+ ret = zfs_for_each(argc_filt, argv_filt, flags, types, NULL, NULL,
+ limit, holds_callback, &cb);
if (ret != 0)
++errors;
- }
/*
* 2. print holds data
*/
print_holds(scripted, cb.cb_max_namelen, cb.cb_max_taglen, nvl);
if (nvlist_empty(nvl))
(void) printf(gettext("no datasets available\n"));
nvlist_free(nvl);
+ free(snapnames);
+ free(argv_filt);
return (0 != errors);
}
#define CHECK_SPINNER 30