6938335 zfs send -R can still miss renamed snapshots

6955879 panic in dmu_objset_stats while running nfs IOs.
6928104 zfs send/rename race can leak snapshot holds
6953835 mem leak in dsl_dataset_user_release_tmp()
6915117 zfs_iter_snapshots() should deal with midstream snapshot renames

4 files changed, 121 insertions, 188 deletions

diff --git a/usr/src/lib/libzfs/common/libzfs.h b/usr/src/lib/libzfs/common/libzfs.h
index 8969b44c01..5df5d9d1f7 100644
--- a/usr/src/lib/libzfs/common/libzfs.h
+++ b/usr/src/lib/libzfs/common/libzfs.h
@@ -533,12 +533,8 @@ extern int zfs_send(zfs_handle_t *zhp, const char *fromsnap, const char *tosnap,
 
 extern int zfs_promote(zfs_handle_t *);
 extern int zfs_hold(zfs_handle_t *, const char *, const char *, boolean_t,
-    boolean_t, boolean_t, int);
-extern int zfs_hold_range(zfs_handle_t *, const char *, const char *,
-    const char *, boolean_t, boolean_t, snapfilter_cb_t, void *, int);
+    boolean_t, boolean_t, int, uint64_t, uint64_t);
 extern int zfs_release(zfs_handle_t *, const char *, const char *, boolean_t);
-extern int zfs_release_range(zfs_handle_t *, const char *, const char *,
-    const char *, boolean_t);
 extern uint64_t zvol_volsize_to_reservation(uint64_t, nvlist_t *);
 
 typedef int (*zfs_userspace_cb_t)(void *arg, const char *domain,
diff --git a/usr/src/lib/libzfs/common/libzfs_dataset.c b/usr/src/lib/libzfs/common/libzfs_dataset.c
index f863b8f369..b23c45f354 100644
--- a/usr/src/lib/libzfs/common/libzfs_dataset.c
+++ b/usr/src/lib/libzfs/common/libzfs_dataset.c
@@ -3927,11 +3927,13 @@ zfs_userspace(zfs_handle_t *zhp, zfs_userquota_prop_t type,
 int
 zfs_hold(zfs_handle_t *zhp, const char *snapname, const char *tag,
     boolean_t recursive, boolean_t temphold, boolean_t enoent_ok,
-    int cleanup_fd)
+    int cleanup_fd, uint64_t dsobj, uint64_t createtxg)
 {
 	zfs_cmd_t zc = { 0 };
 	libzfs_handle_t *hdl = zhp->zfs_hdl;
 
+	ASSERT(!recursive || dsobj == 0);
+
 	(void) strlcpy(zc.zc_name, zhp->zfs_name, sizeof (zc.zc_name));
 	(void) strlcpy(zc.zc_value, snapname, sizeof (zc.zc_value));
 	if (strlcpy(zc.zc_string, tag, sizeof (zc.zc_string))
@@ -3940,6 +3942,8 @@ zfs_hold(zfs_handle_t *zhp, const char *snapname, const char *tag,
 	zc.zc_cookie = recursive;
 	zc.zc_temphold = temphold;
 	zc.zc_cleanup_fd = cleanup_fd;
+	zc.zc_sendobj = dsobj;
+	zc.zc_createtxg = createtxg;
 
 	if (zfs_ioctl(hdl, ZFS_IOC_HOLD, &zc) != 0) {
 		char errbuf[ZFS_MAXNAMELEN+32];
@@ -3969,7 +3973,7 @@ zfs_hold(zfs_handle_t *zhp, const char *snapname, const char *tag,
 			return (zfs_error(hdl, EZFS_REFTAG_HOLD, errbuf));
 		case ENOENT:
 			if (enoent_ok)
-				return (0);
+				return (ENOENT);
 			/* FALLTHROUGH */
 		default:
 			return (zfs_standard_error_fmt(hdl, errno, errbuf));
@@ -3979,107 +3983,6 @@ zfs_hold(zfs_handle_t *zhp, const char *snapname, const char *tag,
 	return (0);
 }
 
-struct hold_range_arg {
-	zfs_handle_t	*origin;
-	const char	*fromsnap;
-	const char	*tosnap;
-	char		lastsnapheld[ZFS_MAXNAMELEN];
-	const char	*tag;
-	boolean_t	temphold;
-	boolean_t	seento;
-	boolean_t	seenfrom;
-	boolean_t	holding;
-	boolean_t	recursive;
-	snapfilter_cb_t	*filter_cb;
-	void		*filter_cb_arg;
-	int		cleanup_fd;
-};
-
-static int
-zfs_hold_range_one(zfs_handle_t *zhp, void *arg)
-{
-	struct hold_range_arg *hra = arg;
-	const char *thissnap;
-	int error;
-
-	thissnap = strchr(zfs_get_name(zhp), '@') + 1;
-
-	if (hra->fromsnap && !hra->seenfrom &&
-	    strcmp(hra->fromsnap, thissnap) == 0)
-		hra->seenfrom = B_TRUE;
-
-	/* snap is older or newer than the desired range, ignore it */
-	if (hra->seento || !hra->seenfrom) {
-		zfs_close(zhp);
-		return (0);
-	}
-
-	if (!hra->seento && strcmp(hra->tosnap, thissnap) == 0)
-		hra->seento = B_TRUE;
-
-	if (hra->filter_cb != NULL &&
-	    hra->filter_cb(zhp, hra->filter_cb_arg) == B_FALSE) {
-		zfs_close(zhp);
-		return (0);
-	}
-
-	if (hra->holding) {
-		/* We could be racing with destroy, so ignore ENOENT. */
-		error = zfs_hold(hra->origin, thissnap, hra->tag,
-		    hra->recursive, hra->temphold, B_TRUE, hra->cleanup_fd);
-		if (error == 0) {
-			(void) strlcpy(hra->lastsnapheld, zfs_get_name(zhp),
-			    sizeof (hra->lastsnapheld));
-		}
-	} else {
-		error = zfs_release(hra->origin, thissnap, hra->tag,
-		    hra->recursive);
-	}
-
-	zfs_close(zhp);
-	return (error);
-}
-
-/*
- * Add a user hold on the set of snapshots starting with fromsnap up to
- * and including tosnap. If we're unable to to acquire a particular hold,
- * undo any holds up to that point.
- */
-int
-zfs_hold_range(zfs_handle_t *zhp, const char *fromsnap, const char *tosnap,
-    const char *tag, boolean_t recursive, boolean_t temphold,
-    snapfilter_cb_t filter_cb, void *cbarg, int cleanup_fd)
-{
-	struct hold_range_arg arg = { 0 };
-	int error;
-
-	arg.origin = zhp;
-	arg.fromsnap = fromsnap;
-	arg.tosnap = tosnap;
-	arg.tag = tag;
-	arg.temphold = temphold;
-	arg.holding = B_TRUE;
-	arg.recursive = recursive;
-	arg.seenfrom = (fromsnap == NULL);
-	arg.filter_cb = filter_cb;
-	arg.filter_cb_arg = cbarg;
-	arg.cleanup_fd = cleanup_fd;
-
-	error = zfs_iter_snapshots_sorted(zhp, zfs_hold_range_one, &arg);
-
-	/*
-	 * Make sure we either hold the entire range or none. If we're
-	 * using cleanup-on-exit, we'll let the closing of the cleanup_fd
-	 * do the work for us.
-	 */
-	if (error && arg.lastsnapheld[0] != '\0' &&
-	    (cleanup_fd == -1 || !temphold)) {
-		(void) zfs_release_range(zhp, fromsnap,
-		    (const char *)arg.lastsnapheld, tag, recursive);
-	}
-	return (error);
-}
-
 int
 zfs_release(zfs_handle_t *zhp, const char *snapname, const char *tag,
     boolean_t recursive)
@@ -4121,27 +4024,6 @@ zfs_release(zfs_handle_t *zhp, const char *snapname, const char *tag,
 	return (0);
 }
 
-/*
- * Release a user hold from the set of snapshots starting with fromsnap
- * up to and including tosnap.
- */
-int
-zfs_release_range(zfs_handle_t *zhp, const char *fromsnap, const char *tosnap,
-    const char *tag, boolean_t recursive)
-{
-	struct hold_range_arg arg = { 0 };
-
-	arg.origin = zhp;
-	arg.fromsnap = fromsnap;
-	arg.tosnap = tosnap;
-	arg.tag = tag;
-	arg.recursive = recursive;
-	arg.seenfrom = (fromsnap == NULL);
-	arg.cleanup_fd = -1;
-
-	return (zfs_iter_snapshots_sorted(zhp, zfs_hold_range_one, &arg));
-}
-
 uint64_t
 zvol_volsize_to_reservation(uint64_t volsize, nvlist_t *props)
 {
diff --git a/usr/src/lib/libzfs/common/libzfs_sendrecv.c b/usr/src/lib/libzfs/common/libzfs_sendrecv.c
index 0762451554..3093ab974d 100644
--- a/usr/src/lib/libzfs/common/libzfs_sendrecv.c
+++ b/usr/src/lib/libzfs/common/libzfs_sendrecv.c
@@ -782,14 +782,30 @@ static int
 zfs_sort_snaps(zfs_handle_t *zhp, void *data)
 {
 	avl_tree_t *avl = data;
-	zfs_node_t *node = zfs_alloc(zhp->zfs_hdl, sizeof (zfs_node_t));
+	zfs_node_t *node;
+	zfs_node_t search;
 
+	search.zn_handle = zhp;
+	node = avl_find(avl, &search, NULL);
+	if (node) {
+		/*
+		 * If this snapshot was renamed while we were creating the
+		 * AVL tree, it's possible that we already inserted it under
+		 * its old name. Remove the old handle before adding the new
+		 * one.
+		 */
+		zfs_close(node->zn_handle);
+		avl_remove(avl, node);
+		free(node);
+	}
+
+	node = zfs_alloc(zhp->zfs_hdl, sizeof (zfs_node_t));
 	node->zn_handle = zhp;
 	avl_add(avl, node);
+
 	return (0);
 }
 
-/* ARGSUSED */
 static int
 zfs_snapshot_compare(const void *larg, const void *rarg)
 {
@@ -844,6 +860,7 @@ typedef struct send_dump_data {
 	const char *fromsnap;
 	const char *tosnap;
 	char prevsnap[ZFS_MAXNAMELEN];
+	uint64_t prevsnap_obj;
 	boolean_t seenfrom, seento, replicate, doall, fromorigin;
 	boolean_t verbose;
 	int outfd;
@@ -853,6 +870,8 @@ typedef struct send_dump_data {
 	snapfilter_cb_t *filter_cb;
 	void *filter_cb_arg;
 	nvlist_t *debugnv;
+	char holdtag[ZFS_MAXNAMELEN];
+	int cleanup_fd;
 } send_dump_data_t;
 
 /*
@@ -860,23 +879,21 @@ typedef struct send_dump_data {
  * NULL) to the file descriptor specified by outfd.
  */
 static int
-dump_ioctl(zfs_handle_t *zhp, const char *fromsnap, boolean_t fromorigin,
-    int outfd, boolean_t enoent_ok, boolean_t *got_enoent, nvlist_t *debugnv)
+dump_ioctl(zfs_handle_t *zhp, const char *fromsnap, uint64_t fromsnap_obj,
+    boolean_t fromorigin, int outfd, nvlist_t *debugnv)
 {
 	zfs_cmd_t zc = { 0 };
 	libzfs_handle_t *hdl = zhp->zfs_hdl;
 	nvlist_t *thisdbg;
 
 	assert(zhp->zfs_type == ZFS_TYPE_SNAPSHOT);
-	assert(fromsnap == NULL || fromsnap[0] == '\0' || !fromorigin);
+	assert(fromsnap_obj == 0 || !fromorigin);
 
 	(void) strlcpy(zc.zc_name, zhp->zfs_name, sizeof (zc.zc_name));
-	if (fromsnap)
-		(void) strlcpy(zc.zc_value, fromsnap, sizeof (zc.zc_value));
 	zc.zc_cookie = outfd;
 	zc.zc_obj = fromorigin;
-
-	*got_enoent = B_FALSE;
+	zc.zc_sendobj = zfs_prop_get_int(zhp, ZFS_PROP_OBJSETID);
+	zc.zc_fromobj = fromsnap_obj;
 
 	VERIFY(0 == nvlist_alloc(&thisdbg, NV_UNIQUE_NAME, 0));
 	if (fromsnap && fromsnap[0] != '\0') {
@@ -904,10 +921,6 @@ dump_ioctl(zfs_handle_t *zhp, const char *fromsnap, boolean_t fromorigin,
 			return (zfs_error(hdl, EZFS_CROSSTARGET, errbuf));
 
 		case ENOENT:
-			if (enoent_ok) {
-				*got_enoent = B_TRUE;
-				return (0);
-			}
 			if (zfs_dataset_exists(hdl, zc.zc_name,
 			    ZFS_TYPE_SNAPSHOT)) {
 				zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
@@ -943,12 +956,47 @@ dump_ioctl(zfs_handle_t *zhp, const char *fromsnap, boolean_t fromorigin,
 }
 
 static int
+hold_for_send(zfs_handle_t *zhp, send_dump_data_t *sdd)
+{
+	zfs_handle_t *pzhp;
+	int error = 0;
+	char *thissnap;
+
+	assert(zhp->zfs_type == ZFS_TYPE_SNAPSHOT);
+
+	/*
+	 * zfs_send() only opens a cleanup_fd for sends that need it,
+	 * e.g. replication and doall.
+	 */
+	if (sdd->cleanup_fd == -1)
+		return (0);
+
+	thissnap = strchr(zhp->zfs_name, '@') + 1;
+	*(thissnap - 1) = '\0';
+	pzhp = zfs_open(zhp->zfs_hdl, zhp->zfs_name, ZFS_TYPE_DATASET);
+	*(thissnap - 1) = '@';
+
+	/*
+	 * It's OK if the parent no longer exists.  The send code will
+	 * handle that error.
+	 */
+	if (pzhp) {
+		error = zfs_hold(pzhp, thissnap, sdd->holdtag,
+		    B_FALSE, B_TRUE, B_TRUE, sdd->cleanup_fd,
+		    zfs_prop_get_int(zhp, ZFS_PROP_OBJSETID),
+		    zfs_prop_get_int(zhp, ZFS_PROP_CREATETXG));
+		zfs_close(pzhp);
+	}
+
+	return (error);
+}
+
+static int
 dump_snapshot(zfs_handle_t *zhp, void *arg)
 {
 	send_dump_data_t *sdd = arg;
-	const char *thissnap;
+	char *thissnap;
 	int err;
-	boolean_t got_enoent;
 	boolean_t isfromsnap, istosnap;
 	boolean_t exclude = B_FALSE;
 
@@ -957,10 +1005,17 @@ dump_snapshot(zfs_handle_t *zhp, void *arg)
 	    strcmp(sdd->fromsnap, thissnap) == 0);
 
 	if (!sdd->seenfrom && isfromsnap) {
-		sdd->seenfrom = B_TRUE;
-		(void) strcpy(sdd->prevsnap, thissnap);
+		err = hold_for_send(zhp, sdd);
+		if (err == 0) {
+			sdd->seenfrom = B_TRUE;
+			(void) strcpy(sdd->prevsnap, thissnap);
+			sdd->prevsnap_obj = zfs_prop_get_int(zhp,
+			    ZFS_PROP_OBJSETID);
+		} else if (err == ENOENT) {
+			err = 0;
+		}
 		zfs_close(zhp);
-		return (0);
+		return (err);
 	}
 
 	if (sdd->seento || !sdd->seenfrom) {
@@ -1001,7 +1056,7 @@ dump_snapshot(zfs_handle_t *zhp, void *arg)
 	    sdd->filter_cb(zhp, sdd->filter_cb_arg) == B_FALSE)) {
 		/*
 		 * This snapshot is filtered out.  Don't send it, and don't
-		 * set prevsnap, so it will be as if this snapshot didn't
+		 * set prevsnap_obj, so it will be as if this snapshot didn't
 		 * exist, and the next accepted snapshot will be sent as
 		 * an incremental from the last accepted one, or as the
 		 * first (and full) snapshot in the case of a replication,
@@ -1011,20 +1066,26 @@ dump_snapshot(zfs_handle_t *zhp, void *arg)
 		return (0);
 	}
 
+	err = hold_for_send(zhp, sdd);
+	if (err) {
+		if (err == ENOENT)
+			err = 0;
+		zfs_close(zhp);
+		return (err);
+	}
+
 	/* send it */
 	if (sdd->verbose) {
 		(void) fprintf(stderr, "sending from @%s to %s\n",
 		    sdd->prevsnap, zhp->zfs_name);
 	}
 
-	err = dump_ioctl(zhp, sdd->prevsnap,
+	err = dump_ioctl(zhp, sdd->prevsnap, sdd->prevsnap_obj,
 	    sdd->prevsnap[0] == '\0' && (sdd->fromorigin || sdd->replicate),
-	    sdd->outfd, B_TRUE, &got_enoent, sdd->debugnv);
+	    sdd->outfd, sdd->debugnv);
 
-	if (got_enoent)
-		err = 0;
-	else
-		(void) strcpy(sdd->prevsnap, thissnap);
+	(void) strcpy(sdd->prevsnap, thissnap);
+	sdd->prevsnap_obj = zfs_prop_get_int(zhp, ZFS_PROP_OBJSETID);
 	zfs_close(zhp);
 	return (err);
 }
@@ -1064,6 +1125,7 @@ dump_filesystem(zfs_handle_t *zhp, void *arg)
 	}
 
 	sdd->seenfrom = sdd->seento = sdd->prevsnap[0] = 0;
+	sdd->prevsnap_obj = 0;
 	if (sdd->fromsnap == NULL || missingfrom)
 		sdd->seenfrom = B_TRUE;
 
@@ -1202,7 +1264,6 @@ zfs_send(zfs_handle_t *zhp, const char *fromsnap, const char *tosnap,
 	int err;
 	nvlist_t *fss = NULL;
 	avl_tree_t *fsavl = NULL;
-	char holdtag[128];
 	static uint64_t holdseq;
 	int spa_version;
 	boolean_t holdsnaps = B_FALSE;
@@ -1210,15 +1271,6 @@ zfs_send(zfs_handle_t *zhp, const char *fromsnap, const char *tosnap,
 	int pipefd[2];
 	dedup_arg_t dda = { 0 };
 	int featureflags = 0;
-	int cleanup_fd = -1;
-
-	if (zhp->zfs_type == ZFS_TYPE_FILESYSTEM) {
-		uint64_t version;
-		version = zfs_prop_get_int(zhp, ZFS_PROP_VERSION);
-		if (version >= ZPL_VERSION_SA) {
-			featureflags |= DMU_BACKUP_FEATURE_SA_SPILL;
-		}
-	}
 
 	(void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN,
 	    "cannot send '%s'"), zhp->zfs_name);
@@ -1229,8 +1281,17 @@ zfs_send(zfs_handle_t *zhp, const char *fromsnap, const char *tosnap,
 		return (zfs_error(zhp->zfs_hdl, EZFS_NOENT, errbuf));
 	}
 
+	if (zhp->zfs_type == ZFS_TYPE_FILESYSTEM) {
+		uint64_t version;
+		version = zfs_prop_get_int(zhp, ZFS_PROP_VERSION);
+		if (version >= ZPL_VERSION_SA) {
+			featureflags |= DMU_BACKUP_FEATURE_SA_SPILL;
+		}
+	}
+
 	if (zfs_spa_version(zhp, &spa_version) == 0 &&
-	    spa_version >= SPA_VERSION_USERREFS)
+	    spa_version >= SPA_VERSION_USERREFS &&
+	    (flags.doall || flags.replicate))
 		holdsnaps = B_TRUE;
 
 	if (flags.dedup) {
@@ -1259,22 +1320,6 @@ zfs_send(zfs_handle_t *zhp, const char *fromsnap, const char *tosnap,
 		size_t buflen = 0;
 		zio_cksum_t zc = { 0 };
 
-		if (holdsnaps) {
-			++holdseq;
-			(void) snprintf(holdtag, sizeof (holdtag),
-			    ".send-%d-%llu", getpid(), (u_longlong_t)holdseq);
-			cleanup_fd = open(ZFS_DEV, O_RDWR|O_EXCL);
-			if (cleanup_fd < 0) {
-				err = errno;
-				goto stderr_out;
-			}
-			err = zfs_hold_range(zhp, fromsnap, tosnap,
-			    holdtag, flags.replicate, B_TRUE, filter_func,
-			    cb_arg, cleanup_fd);
-			if (err)
-				goto err_out;
-		}
-
 		if (flags.replicate || flags.props) {
 			nvlist_t *hdrnv;
 
@@ -1364,6 +1409,18 @@ zfs_send(zfs_handle_t *zhp, const char *fromsnap, const char *tosnap,
 	sdd.filter_cb_arg = cb_arg;
 	if (debugnvp)
 		sdd.debugnv = *debugnvp;
+	if (holdsnaps) {
+		++holdseq;
+		(void) snprintf(sdd.holdtag, sizeof (sdd.holdtag),
+		    ".send-%d-%llu", getpid(), (u_longlong_t)holdseq);
+		sdd.cleanup_fd = open(ZFS_DEV, O_RDWR|O_EXCL);
+		if (sdd.cleanup_fd < 0) {
+			err = errno;
+			goto stderr_out;
+		}
+	} else {
+		sdd.cleanup_fd = -1;
+	}
 	err = dump_filesystems(zhp, &sdd);
 	fsavl_destroy(fsavl);
 	nvlist_free(fss);
@@ -1373,9 +1430,9 @@ zfs_send(zfs_handle_t *zhp, const char *fromsnap, const char *tosnap,
 		(void) pthread_join(tid, NULL);
 	}
 
-	if (cleanup_fd != -1) {
-		VERIFY(0 == close(cleanup_fd));
-		cleanup_fd = -1;
+	if (sdd.cleanup_fd != -1) {
+		VERIFY(0 == close(sdd.cleanup_fd));
+		sdd.cleanup_fd = -1;
 	}
 
 	if (flags.replicate || flags.doall || flags.props) {
@@ -1397,8 +1454,8 @@ zfs_send(zfs_handle_t *zhp, const char *fromsnap, const char *tosnap,
 stderr_out:
 	err = zfs_standard_error(zhp->zfs_hdl, err, errbuf);
 err_out:
-	if (cleanup_fd != -1)
-		VERIFY(0 == close(cleanup_fd));
+	if (sdd.cleanup_fd != -1)
+		VERIFY(0 == close(sdd.cleanup_fd));
 	if (flags.dedup) {
 		(void) pthread_cancel(tid);
 		(void) pthread_join(tid, NULL);
diff --git a/usr/src/lib/libzfs/common/mapfile-vers b/usr/src/lib/libzfs/common/mapfile-vers
index b7ca66d6fd..e9487759ea 100644
--- a/usr/src/lib/libzfs/common/mapfile-vers
+++ b/usr/src/lib/libzfs/common/mapfile-vers
@@ -75,7 +75,6 @@ SYMBOL_VERSION SUNWprivate_1.1 {
 	zfs_get_type;
 	zfs_history_event_names;
 	zfs_hold;
-	zfs_hold_range;
 	zfs_is_mounted;
 	zfs_is_shared;
 	zfs_is_shared_nfs;
@@ -121,7 +120,6 @@ SYMBOL_VERSION SUNWprivate_1.1 {
 	zfs_receive;
 	zfs_refresh_properties;
 	zfs_release;
-	zfs_release_range;
 	zfs_rename;
 	zfs_rollback;
 	zfs_send;