Revert "Fix "snapdev" property inheritance behaviour"

This reverts commit 959f56b99366c8727647b5b19fb3d47555c96cf3.
An issue was uncovered by the new zvol_misc_snapdev test case
which needs to be investigated and resolved.

Reviewed-by: loli10K <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #6174 
Issue #6131 

2 files changed, 57 insertions, 67 deletions

diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c
index 4fda36c7f..c6a532123 100644
--- a/module/zfs/zfs_ioctl.c
+++ b/module/zfs/zfs_ioctl.c
@@ -2761,65 +2761,66 @@ zfs_ioc_inherit_prop(zfs_cmd_t *zc)
 	zprop_source_t source = (received
 	    ? ZPROP_SRC_NONE		/* revert to received value, if any */
 	    : ZPROP_SRC_INHERITED);	/* explicitly inherit */
-	nvlist_t *dummy;
-	nvpair_t *pair;
-	zprop_type_t type;
-	int err;
 
-	if (!received) {
+	if (received) {
+		nvlist_t *dummy;
+		nvpair_t *pair;
+		zprop_type_t type;
+		int err;
+
 		/*
-		 * Only check this in the non-received case. We want to allow
-		 * 'inherit -S' to revert non-inheritable properties like quota
-		 * and reservation to the received or default values even though
-		 * they are not considered inheritable.
+		 * zfs_prop_set_special() expects properties in the form of an
+		 * nvpair with type info.
 		 */
-		if (prop != ZPROP_INVAL && !zfs_prop_inheritable(prop))
-			return (SET_ERROR(EINVAL));
-	}
+		if (prop == ZPROP_INVAL) {
+			if (!zfs_prop_user(propname))
+				return (SET_ERROR(EINVAL));
 
-	if (prop == ZPROP_INVAL) {
-		if (!zfs_prop_user(propname))
+			type = PROP_TYPE_STRING;
+		} else if (prop == ZFS_PROP_VOLSIZE ||
+		    prop == ZFS_PROP_VERSION) {
 			return (SET_ERROR(EINVAL));
+		} else {
+			type = zfs_prop_get_type(prop);
+		}
 
-		type = PROP_TYPE_STRING;
-	} else if (prop == ZFS_PROP_VOLSIZE || prop == ZFS_PROP_VERSION) {
-		return (SET_ERROR(EINVAL));
-	} else {
-		type = zfs_prop_get_type(prop);
-	}
-
-	/*
-	 * zfs_prop_set_special() expects properties in the form of an
-	 * nvpair with type info.
-	 */
-	dummy = fnvlist_alloc();
+		VERIFY(nvlist_alloc(&dummy, NV_UNIQUE_NAME, KM_SLEEP) == 0);
 
-	switch (type) {
-	case PROP_TYPE_STRING:
-		VERIFY(0 == nvlist_add_string(dummy, propname, ""));
-		break;
-	case PROP_TYPE_NUMBER:
-	case PROP_TYPE_INDEX:
-		VERIFY(0 == nvlist_add_uint64(dummy, propname, 0));
-		break;
-	default:
-		err = SET_ERROR(EINVAL);
-		goto errout;
-	}
+		switch (type) {
+		case PROP_TYPE_STRING:
+			VERIFY(0 == nvlist_add_string(dummy, propname, ""));
+			break;
+		case PROP_TYPE_NUMBER:
+		case PROP_TYPE_INDEX:
+			VERIFY(0 == nvlist_add_uint64(dummy, propname, 0));
+			break;
+		default:
+			nvlist_free(dummy);
+			return (SET_ERROR(EINVAL));
+		}
 
-	pair = nvlist_next_nvpair(dummy, NULL);
-	if (pair == NULL) {
-		err = SET_ERROR(EINVAL);
-	} else {
+		pair = nvlist_next_nvpair(dummy, NULL);
+		if (pair == NULL) {
+			nvlist_free(dummy);
+			return (SET_ERROR(EINVAL));
+		}
 		err = zfs_prop_set_special(zc->zc_name, source, pair);
-		if (err == -1) /* property is not "special", needs handling */
-			err = dsl_prop_inherit(zc->zc_name, zc->zc_value,
-			    source);
+		nvlist_free(dummy);
+		if (err != -1)
+			return (err); /* special property already handled */
+	} else {
+		/*
+		 * Only check this in the non-received case. We want to allow
+		 * 'inherit -S' to revert non-inheritable properties like quota
+		 * and reservation to the received or default values even though
+		 * they are not considered inheritable.
+		 */
+		if (prop != ZPROP_INVAL && !zfs_prop_inheritable(prop))
+			return (SET_ERROR(EINVAL));
 	}
 
-errout:
-	nvlist_free(dummy);
-	return (err);
+	/* property name has been validated by zfs_secpolicy_inherit_prop() */
+	return (dsl_prop_inherit(zc->zc_name, zc->zc_value, source));
 }
 
 static int
diff --git a/module/zfs/zvol.c b/module/zfs/zvol.c
index 33b1f4321..121f75e4e 100644
--- a/module/zfs/zvol.c
+++ b/module/zfs/zvol.c
@@ -2216,18 +2216,20 @@ zvol_set_snapdev_check(void *arg, dmu_tx_t *tx)
 	return (error);
 }
 
-/* ARGSUSED */
 static int
 zvol_set_snapdev_sync_cb(dsl_pool_t *dp, dsl_dataset_t *ds, void *arg)
 {
+	zvol_set_snapdev_arg_t *zsda = arg;
 	char dsname[MAXNAMELEN];
 	zvol_task_t *task;
-	uint64_t snapdev;
 
 	dsl_dataset_name(ds, dsname);
-	if (dsl_prop_get_int_ds(ds, "snapdev", &snapdev) != 0)
-		return (0);
-	task = zvol_task_alloc(ZVOL_ASYNC_SET_SNAPDEV, dsname, NULL, snapdev);
+	dsl_prop_set_sync_impl(ds, zfs_prop_to_name(ZFS_PROP_SNAPDEV),
+	    zsda->zsda_source, sizeof (zsda->zsda_value), 1,
+	    &zsda->zsda_value, zsda->zsda_tx);
+
+	task = zvol_task_alloc(ZVOL_ASYNC_SET_SNAPDEV, dsname,
+	    NULL, zsda->zsda_value);
 	if (task == NULL)
 		return (0);
 
@@ -2237,11 +2239,7 @@ zvol_set_snapdev_sync_cb(dsl_pool_t *dp, dsl_dataset_t *ds, void *arg)
 }
 
 /*
- * Traverse all child datasets and apply snapdev appropriately.
- * We call dsl_prop_set_sync_impl() here to set the value only on the toplevel
- * dataset and read the effective "snapdev" on every child in the callback
- * function: this is because the value is not guaranteed to be the same in the
- * whole dataset hierarchy.
+ * Traverse all child snapshot datasets and apply snapdev appropriately.
  */
 static void
 zvol_set_snapdev_sync(void *arg, dmu_tx_t *tx)
@@ -2249,19 +2247,10 @@ zvol_set_snapdev_sync(void *arg, dmu_tx_t *tx)
 	zvol_set_snapdev_arg_t *zsda = arg;
 	dsl_pool_t *dp = dmu_tx_pool(tx);
 	dsl_dir_t *dd;
-	dsl_dataset_t *ds;
-	int error;
 
 	VERIFY0(dsl_dir_hold(dp, zsda->zsda_name, FTAG, &dd, NULL));
 	zsda->zsda_tx = tx;
 
-	error = dsl_dataset_hold(dp, zsda->zsda_name, FTAG, &ds);
-	if (error == 0) {
-		dsl_prop_set_sync_impl(ds, zfs_prop_to_name(ZFS_PROP_SNAPDEV),
-		    zsda->zsda_source, sizeof (zsda->zsda_value), 1,
-		    &zsda->zsda_value, zsda->zsda_tx);
-		dsl_dataset_rele(ds, FTAG);
-	}
 	dmu_objset_find_dp(dp, dd->dd_object, zvol_set_snapdev_sync_cb,
 	    zsda, DS_FIND_CHILDREN);