Ensure devices are always created/removed

Devices were only being created at module load time or when a
dataset was created.  Similiar devices were not always being
removed at all the correct times.  This patch updates all the
places where devices should either be created or removed.  I'm
reasonably sure I got them all but if theres a case I missed
we can catch it with a follow up patch.

        module load/unload
        zfs create/remove
        zpool import/export
        zpool destroy

This patch also adds a simple regression test to zconfig.sh
to ensure zpool import/export is basically working properly.
This test specifically checks that devices are created
properly, removed after export, created after import, and
removed as a consequence of a zpool destroy.

module/zfs/include/sys/zvol.h

@@ -40,10 +40,11 @@ extern int zvol_check_volblocksize(uint64_t volblocksize);
 extern int zvol_get_stats(objset_t *os, nvlist_t *nv);
 extern void zvol_create_cb(objset_t *os, void *arg, cred_t *cr, dmu_tx_t *tx);
 extern int zvol_create_minor(const char *);
+extern int zvol_create_minors(const char *);
 extern int zvol_remove_minor(const char *);
+extern int zvol_remove_minors(const char *);
 extern int zvol_set_volsize(const char *, uint64_t);
 extern int zvol_set_volblocksize(const char *, uint64_t);
-extern void zvol_remove_minors(const char *);
 
 extern int zvol_init(void);
 extern void zvol_fini(void);

module/zfs/zfs_ioctl.c

@@ -1112,9 +1112,9 @@ zfs_ioc_pool_destroy(zfs_cmd_t *zc)
 {
 	int error;
 	zfs_log_history(zc);
-	error = spa_destroy(zc->zc_name);
+	error = zvol_remove_minors(zc->zc_name);
 	if (error == 0)
-		zvol_remove_minors(zc->zc_name);
+		error = spa_destroy(zc->zc_name);
 	return (error);
 }
 
@@ -1144,6 +1144,9 @@ zfs_ioc_pool_import(zfs_cmd_t *zc)
 	else
 		error = spa_import(zc->zc_name, config, props);
 
+	if (error == 0)
+		error = zvol_create_minors(zc->zc_name);
+
 	if (zc->zc_nvlist_dst != 0)
 		(void) put_nvlist(zc, config);
 
@@ -1163,9 +1166,10 @@ zfs_ioc_pool_export(zfs_cmd_t *zc)
 	boolean_t hardforce = (boolean_t)zc->zc_guid;
 
 	zfs_log_history(zc);
-	error = spa_export(zc->zc_name, NULL, force, hardforce);
+	error = zvol_remove_minors(zc->zc_name);
 	if (error == 0)
-		zvol_remove_minors(zc->zc_name);
+		error = spa_export(zc->zc_name, NULL, force, hardforce);
+
 	return (error);
 }
 
@@ -2965,7 +2969,7 @@ zfs_ioc_destroy(zfs_cmd_t *zc)
 
 	err = dmu_objset_destroy(zc->zc_name, zc->zc_defer_destroy);
 	if (zc->zc_objset_type == DMU_OST_ZVOL && err == 0)
-		(void) zvol_remove_minor(zc->zc_name);
+		err = zvol_remove_minor(zc->zc_name);
 	return (err);
 }
 
@@ -3065,6 +3069,7 @@ static int
 zfs_ioc_rename(zfs_cmd_t *zc)
 {
 	boolean_t recursive = zc->zc_cookie & 1;
+	int err;
 
 	zc->zc_value[sizeof (zc->zc_value) - 1] = '\0';
 	if (dataset_namecheck(zc->zc_value, NULL, NULL) != 0 ||
@@ -3078,12 +3083,16 @@ zfs_ioc_rename(zfs_cmd_t *zc)
 	 */
 	if (!recursive && strchr(zc->zc_name, '@') != NULL &&
 	    zc->zc_objset_type == DMU_OST_ZFS) {
-		int err = zfs_unmount_snap(zc->zc_name, NULL);
+		err = zfs_unmount_snap(zc->zc_name, NULL);
 		if (err)
 			return (err);
 	}
-	if (zc->zc_objset_type == DMU_OST_ZVOL)
-		(void) zvol_remove_minor(zc->zc_name);
+	if (zc->zc_objset_type == DMU_OST_ZVOL) {
+		err = zvol_remove_minor(zc->zc_name);
+		if (err)
+			return (err);
+	}
+
 	return (dmu_objset_rename(zc->zc_name, zc->zc_value, recursive));
 }
 

module/zfs/zvol.c

@@ -1133,25 +1133,6 @@ out:
 	return (-error);
 }
 
-static int
-zvol_create_minors_cb(spa_t *spa, uint64_t dsobj,
-		      const char *dsname, void *arg)
-{
-	return zvol_create_minor(dsname);
-}
-
-static void
-zvol_create_minors(void)
-{
-	spa_t *spa = NULL;
-
-	mutex_enter(&spa_namespace_lock);
-	while ((spa = spa_next(spa)) != NULL)
-		(void) dmu_objset_find_spa(NULL, spa_name(spa), zvol_create_minors_cb,
-		    NULL, DS_FIND_CHILDREN | DS_FIND_SNAPSHOTS);
-	mutex_exit(&spa_namespace_lock);
-}
-
 /*
  * Remove a block device minor node for the specified volume.
  */
@@ -1179,26 +1160,69 @@ zvol_remove_minor(const char *name)
 out:
 	mutex_exit(&zvol_state_lock);
 
-	return (-error);
+	return (error);
+}
+
+static int
+zvol_create_minors_cb(spa_t *spa, uint64_t dsobj,
+		      const char *dsname, void *arg)
+{
+	if (strchr(dsname, '/') == NULL)
+		return 0;
+
+	return zvol_create_minor(dsname);
+}
+
+static int
+zvol_remove_minors_cb(spa_t *spa, uint64_t dsobj,
+		      const char *dsname, void *arg)
+{
+	if (strchr(dsname, '/') == NULL)
+		return 0;
+
+	return zvol_remove_minor(dsname);
+}
+
+static int
+zvol_cr_minors_common(const char *pool,
+    int func(spa_t *, uint64_t, const char *, void *), void *arg)
+{
+	spa_t *spa = NULL;
+	int error = 0;
+
+	if (pool) {
+		error = dmu_objset_find_spa(NULL, pool, func, arg,
+		    DS_FIND_CHILDREN | DS_FIND_SNAPSHOTS);
+	} else {
+		mutex_enter(&spa_namespace_lock);
+		while ((spa = spa_next(spa)) != NULL) {
+			error = dmu_objset_find_spa(NULL, spa_name(spa),
+			    func, arg, DS_FIND_CHILDREN | DS_FIND_SNAPSHOTS);
+			if (error)
+				break;
+		}
+		mutex_exit(&spa_namespace_lock);
+	}
+
+	return error;
 }
 
 /*
- * Remove all minors from the system.  This is only called from
- * zvol_fini() which means the module reference count must have
- * dropped to zero and none of the zvol devices may be open.
+ * Create minors for specified pool, or if NULL create all minors.
  */
-void
-zvol_remove_minors(const char *name)
+int
+zvol_create_minors(const char *pool)
 {
-	zvol_state_t *zv;
+	return zvol_cr_minors_common(pool, zvol_create_minors_cb, NULL);
+}
 
-	mutex_enter(&zvol_state_lock);
-	while ((zv = list_head(&zvol_state_list)) != NULL) {
-		ASSERT3U(zv->zv_open_count, ==, 0);
-		zvol_remove(zv);
-		zvol_free(zv);
-	}
-	mutex_exit(&zvol_state_lock);
+/*
+ * Remove minors for specified pool, or if NULL remove all minors.
+ */
+int
+zvol_remove_minors(const char *pool)
+{
+	return zvol_cr_minors_common(pool, zvol_remove_minors_cb, NULL);
 }
 
 int
@@ -1230,7 +1254,7 @@ zvol_init(void)
 	list_create(&zvol_state_list, sizeof (zvol_state_t),
 	            offsetof(zvol_state_t, zv_next));
 
-	zvol_create_minors();
+	(void) zvol_create_minors(NULL);
 
 	return (0);
 }
@@ -1238,6 +1262,8 @@ zvol_init(void)
 void
 zvol_fini(void)
 {
+	(void) zvol_remove_minors(NULL);
+
 	blk_unregister_region(MKDEV(zvol_major, 0), 1UL << MINORBITS);
 	unregister_blkdev(zvol_major, ZVOL_DRIVER);
 	taskq_destroy(zvol_taskq);

scripts/zconfig.sh

@@ -161,4 +161,52 @@ EOF
 }
 zconfig_test3
 
+# zpool import/export check
+zconfig_test4() {
+	POOL_NAME=test4
+	ZVOL_NAME1=fish1
+	ZVOL_NAME2=fish2
+	FULL_NAME1=${POOL_NAME}/${ZVOL_NAME1}
+	FULL_NAME2=${POOL_NAME}/${ZVOL_NAME2}
+	TMP_CACHE=`mktemp -p /tmp zpool.cache.XXXXXXXX`
+
+	echo -n "test 4 - zpool import/export: "
+
+	# Create a pool and volume.
+	${ZFS_SH} zfs="spa_config_path=${TMP_CACHE}" || fail 1
+	${ZPOOL_CREATE_SH} -p ${POOL_NAME} -c lo-raidz2 || fail 2
+	${ZFS} create -V 100M ${FULL_NAME1} || fail 3
+	${ZFS} create -V 100M ${FULL_NAME2} || fail 4
+
+	# Verify the devices were created
+	stat /dev/${FULL_NAME1} &>/dev/null || fail 5
+	stat /dev/${FULL_NAME2} &>/dev/null || fail 6
+
+	# Export the pool
+	${ZPOOL} export ${POOL_NAME} || fail 7
+
+	# Verify the devices were removed
+	stat /dev/${FULL_NAME1} &>/dev/null && fail 8
+	stat /dev/${FULL_NAME2} &>/dev/null && fail 9
+
+	# Import the pool
+	${ZPOOL} import ${POOL_NAME} || fail 10
+
+	# Verify the devices were created
+	stat /dev/${FULL_NAME1} &>/dev/null || fail 11
+	stat /dev/${FULL_NAME2} &>/dev/null || fail 12
+
+	# Destroy the pool and consequently the devices
+	${ZPOOL_CREATE_SH} -p ${POOL_NAME} -c lo-raidz2 -d || fail 15
+
+	# Verify the devices were removed
+	stat /dev/${FULL_NAME1} &>/dev/null && fail 16
+	stat /dev/${FULL_NAME2} &>/dev/null && fail 17
+
+	${ZFS_SH} -u || fail 18
+
+	pass
+}
+zconfig_test4
+
 exit 0