From 122e5b44e1fac136df6929a2929fd2723d291120 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Fri, 2 Jul 2010 09:24:18 -0700 Subject: [PATCH 1/2] Add missing mutex_exit(&zvol_state_lock) With the recent ZVOL update zvol_set_volblocksize() accidentally lost its mutex_exit(). This was noticed when zvol_create_minor() blocked on the zvol_state_lock while it was holding the spa_namespace_lock(). This caused everything to get blocked up and hung the system. --- module/zfs/zvol.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/module/zfs/zvol.c b/module/zfs/zvol.c index ffbb29251c..955b75517a 100644 --- a/module/zfs/zvol.c +++ b/module/zfs/zvol.c @@ -342,11 +342,15 @@ zvol_set_volblocksize(const char *name, uint64_t volblocksize) mutex_enter(&zvol_state_lock); zv = zvol_find_by_name(name); - if (zv == NULL) - return (ENXIO); + if (zv == NULL) { + error = ENXIO; + goto out; + } - if (get_disk_ro(zv->zv_disk) || (zv->zv_flags & ZVOL_RDONLY)) - return (EROFS); + if (get_disk_ro(zv->zv_disk) || (zv->zv_flags & ZVOL_RDONLY)) { + error = EROFS; + goto out; + } tx = dmu_tx_create(zv->zv_objset); dmu_tx_hold_bonus(tx, ZVOL_OBJ); @@ -362,6 +366,8 @@ zvol_set_volblocksize(const char *name, uint64_t volblocksize) if (error == 0) zv->zv_volblocksize = volblocksize; } +out: + mutex_exit(&zvol_state_lock); return (error); } From 45cb33f64f1fa7e30844cf9952ebf10b050088a1 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Fri, 2 Jul 2010 12:21:39 -0700 Subject: [PATCH 2/2] 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 | 3 +- module/zfs/zfs_ioctl.c | 25 +++++++--- module/zfs/zvol.c | 94 ++++++++++++++++++++++------------- scripts/zconfig.sh | 48 ++++++++++++++++++ 4 files changed, 127 insertions(+), 43 deletions(-) diff --git a/module/zfs/include/sys/zvol.h b/module/zfs/include/sys/zvol.h index ff3e7af9bb..ed12c923de 100644 --- a/module/zfs/include/sys/zvol.h +++ b/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); diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index fb1dc03637..a41c2762bd 100644 --- a/module/zfs/zfs_ioctl.c +++ b/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)); } diff --git a/module/zfs/zvol.c b/module/zfs/zvol.c index 955b75517a..2e16e60c23 100644 --- a/module/zfs/zvol.c +++ b/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); diff --git a/scripts/zconfig.sh b/scripts/zconfig.sh index 55e4c46453..7429669b61 100755 --- a/scripts/zconfig.sh +++ b/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