Minor device creation/removal (Kernel)

With the update to onnv_141 how minor devices were created and
removed for ZVOL was substantially changed.  The updated system
is much more tightly integrated with Solaris's /dev/ filesystem.
This is great for Solaris but bad for Linux.

On the kernel side the ZFS_IOC_{CREATE,REMOVE}_MINOR ioctl
entry points have been re-added.  They now call directly in
to the ZVOL to create the needed minor node and add the sysfs
entried for udev.

Also as part of this change I've decided it would really be
best if all the zvols were in a /dev/zvol directory like on
Solaris.  Organizationally this makes sense and on the code
side it allows us to know a block device is a zvol simply by
where it is located in /dev/.  Unless Solaris there still is
to ./dsk or ./rdsk as part of the path.

module/zcommon/include/sys/fs/zfs.h

@@ -686,13 +686,17 @@ typedef struct ddt_histogram {
 	ddt_stat_t	ddh_stat[64];	/* power-of-two histogram buckets */
 } ddt_histogram_t;
 
-#define	ZVOL_DRIVER		"zvol"
-#define	ZFS_DRIVER		"zfs"
-#define	ZFS_DEV			"/dev/zfs"
-#define ZVOL_MAJOR		230
-#define ZVOL_MINOR_BITS		4
-#define ZVOL_MINOR_MASK		((1U << ZVOL_MINOR_BITS) - 1)
-#define ZVOL_MINORS		(1 << 4)
+#define	ZVOL_DRIVER	"zvol"
+#define	ZFS_DRIVER	"zfs"
+#define	ZFS_DEV		"/dev/zfs"
+
+/* general zvol path */
+#define	ZVOL_DIR	"/dev/zvol"
+
+#define	ZVOL_MAJOR		230
+#define	ZVOL_MINOR_BITS		4
+#define	ZVOL_MINOR_MASK		((1U << ZVOL_MINOR_BITS) - 1)
+#define	ZVOL_MINORS		(1 << 4)
 
 #define	ZVOL_PROP_NAME		"name"
 #define	ZVOL_DEFAULT_BLOCKSIZE	8192
@@ -726,6 +730,8 @@ typedef enum zfs_ioc {
 	ZFS_IOC_DATASET_LIST_NEXT,
 	ZFS_IOC_SNAPSHOT_LIST_NEXT,
 	ZFS_IOC_SET_PROP,
+	ZFS_IOC_CREATE_MINOR,
+	ZFS_IOC_REMOVE_MINOR,
 	ZFS_IOC_CREATE,
 	ZFS_IOC_DESTROY,
 	ZFS_IOC_ROLLBACK,

module/zfs/include/sys/zvol.h

@@ -42,7 +42,7 @@ 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 void zvol_remove_minors(const char *);
 extern int zvol_set_volsize(const char *, uint64_t);
 extern int zvol_set_volblocksize(const char *, uint64_t);
 

module/zfs/vdev.c

@@ -1072,12 +1072,6 @@ vdev_open_child(void *arg)
 boolean_t
 vdev_uses_zvols(vdev_t *vd)
 {
-/*
- * NOTE: Disabled because under Linux I've choosen not to put all the zvols
- * in their own directory.  This could be changed or this code can be updated
- * to perhap run an ioctl() on the vdev path to determine if it is a zvol.
- */
-#if 0
 	int c;
 
 	if (vd->vdev_path && strncmp(vd->vdev_path, ZVOL_DIR,
@@ -1086,7 +1080,6 @@ vdev_uses_zvols(vdev_t *vd)
 	for (c = 0; c < vd->vdev_children; c++)
 		if (vdev_uses_zvols(vd->vdev_child[c]))
 			return (B_TRUE);
-#endif
 	return (B_FALSE);
 }
 

module/zfs/zfs_ioctl.c

@@ -1112,8 +1112,9 @@ zfs_ioc_pool_destroy(zfs_cmd_t *zc)
 {
 	int error;
 	zfs_log_history(zc);
-	(void) zvol_remove_minors(zc->zc_name);
 	error = spa_destroy(zc->zc_name);
+	if (error == 0)
+		zvol_remove_minors(zc->zc_name);
 	return (error);
 }
 
@@ -1144,7 +1145,7 @@ zfs_ioc_pool_import(zfs_cmd_t *zc)
 		error = spa_import(zc->zc_name, config, props);
 
 	if (error == 0)
-		error = zvol_create_minors(zc->zc_name);
+		zvol_create_minors(zc->zc_name);
 
 	if (zc->zc_nvlist_dst != 0)
 		(void) put_nvlist(zc, config);
@@ -1165,10 +1166,9 @@ zfs_ioc_pool_export(zfs_cmd_t *zc)
 	boolean_t hardforce = (boolean_t)zc->zc_guid;
 
 	zfs_log_history(zc);
-	error = zvol_remove_minors(zc->zc_name);
+	error = spa_export(zc->zc_name, NULL, force, hardforce);
 	if (error == 0)
-		error = spa_export(zc->zc_name, NULL, force, hardforce);
-
+		zvol_remove_minors(zc->zc_name);
 	return (error);
 }
 
@@ -2438,6 +2438,30 @@ zfs_ioc_pool_get_props(zfs_cmd_t *zc)
 	return (error);
 }
 
+/*
+ * inputs:
+ * zc_name              name of volume
+ *
+ * outputs:             none
+ */
+static int
+zfs_ioc_create_minor(struct file *filp, zfs_cmd_t *zc)
+{
+	return (zvol_create_minor(zc->zc_name));
+}
+
+/*
+ * inputs:
+ * zc_name              name of volume
+ *
+ * outputs:             none
+ */
+static int
+zfs_ioc_remove_minor(struct file *filp, zfs_cmd_t *zc)
+{
+	return (zvol_remove_minor(zc->zc_name));
+}
+
 /*
  * inputs:
  * zc_name		name of filesystem
@@ -2834,18 +2858,9 @@ zfs_ioc_create(zfs_cmd_t *zc)
 	if (error == 0) {
 		error = zfs_set_prop_nvlist(zc->zc_name, ZPROP_SRC_LOCAL,
 		    nvprops, NULL);
-		if (error != 0) {
+		if (error != 0)
 			(void) dmu_objset_destroy(zc->zc_name, B_FALSE);
-			goto out;
-		}
-
-		if (type == DMU_OST_ZVOL) {
-			error = zvol_create_minor(zc->zc_name);
-			if (error != 0)
-				(void) dmu_objset_destroy(zc->zc_name, B_FALSE);
-		}
 	}
-out:
 	nvlist_free(nvprops);
 	return (error);
 }
@@ -2968,7 +2983,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)
-		err = zvol_remove_minor(zc->zc_name);
+		(void) zvol_remove_minor(zc->zc_name);
 	return (err);
 }
 
@@ -3068,7 +3083,6 @@ 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 ||
@@ -3082,16 +3096,12 @@ zfs_ioc_rename(zfs_cmd_t *zc)
 	 */
 	if (!recursive && strchr(zc->zc_name, '@') != NULL &&
 	    zc->zc_objset_type == DMU_OST_ZFS) {
-		err = zfs_unmount_snap(zc->zc_name, NULL);
-		if (err)
-			return (err);
-	}
-	if (zc->zc_objset_type == DMU_OST_ZVOL) {
-		err = zvol_remove_minor(zc->zc_name);
+		int 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);
 	return (dmu_objset_rename(zc->zc_name, zc->zc_value, recursive));
 }
 
@@ -4315,6 +4325,10 @@ static zfs_ioc_vec_t zfs_ioc_vec[] = {
 	{ zfs_ioc_snapshot_list_next, zfs_secpolicy_read, DATASET_NAME, B_FALSE,
 	    B_TRUE },
 	{ zfs_ioc_set_prop, zfs_secpolicy_none, DATASET_NAME, B_TRUE, B_TRUE },
+	{ zfs_ioc_create_minor, zfs_secpolicy_config, DATASET_NAME, B_FALSE,
+	    B_FALSE },
+	{ zfs_ioc_remove_minor, zfs_secpolicy_config, DATASET_NAME, B_FALSE,
+	    B_FALSE },
 	{ zfs_ioc_create, zfs_secpolicy_create, DATASET_NAME, B_TRUE, B_TRUE },
 	{ zfs_ioc_destroy, zfs_secpolicy_destroy, DATASET_NAME, B_TRUE,
 	    B_TRUE},

module/zfs/zvol.c

@@ -57,6 +57,7 @@ static char *zvol_tag = "zvol_tag";
  * The in-core state of each volume.
  */
 typedef struct zvol_state {
+	char			zv_name[DISK_NAME_LEN];	/* name */
 	uint64_t		zv_volsize;	/* advertised space */
 	uint64_t		zv_volblocksize;/* volume block size */
 	objset_t		*zv_objset;	/* objset handle */
@@ -130,7 +131,7 @@ zvol_find_by_name(const char *name)
 	ASSERT(MUTEX_HELD(&zvol_state_lock));
 	for (zv = list_head(&zvol_state_list); zv != NULL;
 	     zv = list_next(&zvol_state_list, zv)) {
-		if (!strncmp(zv->zv_disk->disk_name, name, DISK_NAME_LEN))
+		if (!strncmp(zv->zv_name, name, DISK_NAME_LEN))
 			return zv;
 	}
 
@@ -757,11 +758,10 @@ zvol_first_open(zvol_state_t *zv)
 	objset_t *os;
 	uint64_t volsize;
 	int error;
-	uint64_t readonly;
+	uint64_t ro;
 
 	/* lie and say we're read-only */
-	error = dmu_objset_own(zv->zv_disk->disk_name,
-	    DMU_OST_ZVOL, B_TRUE, zvol_tag, &os);
+	error = dmu_objset_own(zv->zv_name, DMU_OST_ZVOL, 1, zvol_tag, &os);
 	if (error)
 		return (-error);
 
@@ -782,9 +782,8 @@ zvol_first_open(zvol_state_t *zv)
 	zv->zv_volsize = volsize;
 	zv->zv_zilog = zil_open(os, zvol_get_data);
 
-	VERIFY(dsl_prop_get_integer(zv->zv_disk->disk_name,
-	    "readonly", &readonly, NULL) == 0);
-	if (readonly || dmu_objset_is_snapshot(os)) {
+	VERIFY(dsl_prop_get_integer(zv->zv_name, "readonly", &ro, NULL) == 0);
+	if (ro || dmu_objset_is_snapshot(os)) {
                 set_disk_ro(zv->zv_disk, 1);
 	        zv->zv_flags |= ZVOL_RDONLY;
 	} else {
@@ -1031,6 +1030,7 @@ zvol_alloc(dev_t dev, const char *name)
 	zv->zv_queue->queuedata = zv;
 	zv->zv_dev = dev;
 	zv->zv_open_count = 0;
+	strlcpy(zv->zv_name, name, DISK_NAME_LEN);
 
 	mutex_init(&zv->zv_znode.z_range_lock, NULL, MUTEX_DEFAULT, NULL);
 	avl_create(&zv->zv_znode.z_range_avl, zfs_range_compare,
@@ -1043,7 +1043,7 @@ zvol_alloc(dev_t dev, const char *name)
 	zv->zv_disk->fops = &zvol_ops;
 	zv->zv_disk->private_data = zv;
 	zv->zv_disk->queue = zv->zv_queue;
-	strlcpy(zv->zv_disk->disk_name, name, DISK_NAME_LEN);
+	snprintf(zv->zv_disk->disk_name, DISK_NAME_LEN, "zvol/%s", name);
 
 	return zv;
 
@@ -1173,31 +1173,25 @@ zvol_create_minors_cb(spa_t *spa, uint64_t dsobj,
 	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)
+/*
+ * Create minors for specified pool, if pool is NULL create minors
+ * for all available pools.
+ */
+int
+zvol_create_minors(const char *pool)
 {
 	spa_t *spa = NULL;
 	int error = 0;
 
 	if (pool) {
-		error = dmu_objset_find_spa(NULL, pool, func, arg,
-		    DS_FIND_CHILDREN | DS_FIND_SNAPSHOTS);
+		error = dmu_objset_find_spa(NULL, pool, zvol_create_minors_cb,
+		    NULL, 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);
+			error = dmu_objset_find_spa(NULL,
+			    spa_name(spa), zvol_create_minors_cb, NULL,
+			    DS_FIND_CHILDREN | DS_FIND_SNAPSHOTS);
 			if (error)
 				break;
 		}
@@ -1208,21 +1202,31 @@ zvol_cr_minors_common(const char *pool,
 }
 
 /*
- * Create minors for specified pool, or if NULL create all minors.
- */
-int
-zvol_create_minors(const char *pool)
-{
-	return zvol_cr_minors_common(pool, zvol_create_minors_cb, NULL);
-}
-
-/*
- * Remove minors for specified pool, or if NULL remove all minors.
+ * Remove minors for specified pool, if pool is NULL remove all minors.
  */
-int
+void
 zvol_remove_minors(const char *pool)
 {
-	return zvol_cr_minors_common(pool, zvol_remove_minors_cb, NULL);
+	zvol_state_t *zv, *zv_next;
+	char *str;
+
+	str = kmem_zalloc(DISK_NAME_LEN, KM_SLEEP);
+	if (pool) {
+		(void) strncpy(str, pool, strlen(pool));
+		(void) strcat(str, "/");
+	}
+
+	mutex_enter(&zvol_state_lock);
+	for (zv = list_head(&zvol_state_list); zv != NULL; zv = zv_next) {
+		zv_next = list_next(&zvol_state_list, zv);
+
+		if (pool == NULL || !strncmp(str, zv->zv_name, strlen(str))) {
+			zvol_remove(zv);
+			zvol_free(zv);
+		}
+	}
+	mutex_exit(&zvol_state_lock);
+	kmem_free(str, DISK_NAME_LEN);
 }
 
 int
@@ -1262,12 +1266,10 @@ zvol_init(void)
 void
 zvol_fini(void)
 {
-	(void) zvol_remove_minors(NULL);
-
+	zvol_remove_minors(NULL);
 	blk_unregister_region(MKDEV(zvol_major, 0), 1UL << MINORBITS);
 	unregister_blkdev(zvol_major, ZVOL_DRIVER);
 	taskq_destroy(zvol_taskq);
-
 	mutex_destroy(&zvol_state_lock);
 	list_destroy(&zvol_state_list);
 }