zvol: call zvol_first_open upon the creating of zvol geom
Andriy Gapon
avg at FreeBSD.org
Sun Feb 3 17:35:59 UTC 2013
Please review the following proposed ZFS/ZVOL<->GEOM changes.
Thank you.
commit 9315659d772b8ebbb9b4bcf16f2d5f5bb8a766bd
Author: Andriy Gapon <avg at icyb.net.ua>
Date: Fri Sep 14 00:34:38 2012 +0300
zvol: call zvol_first_open upon the creating of zvol geom
... and symmetrically call zvol_last_close when the geom is going away.
Also, do not needlessly call zvol_size_changed in zvol_first_open.
There is no need to blindly mimic Solaris behavior when FreeBSD GEOM
interfacing can be more efficient (and proper).
Also, this simplifies locking in zvol_open/zvol_close and fixes
the really broken behavior in zvol_geom_access of dropping the
g_topology_lock.
This change introduces a requirement that a zvol's 'minor' (geom) should be
destroyed before the zvol itself (as a dataset) can go.
Otherwise the minor holds the zvol's dataset busy and thus it can not
be destroyed nor its pool can be exported.
diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
index 10443fb..8465ea5 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
@@ -1291,9 +1291,10 @@ zfs_ioc_pool_destroy(zfs_cmd_t *zc)
{
int error;
zfs_log_history(zc);
+ zvol_remove_minors(zc->zc_name);
error = spa_destroy(zc->zc_name);
- if (error == 0)
- zvol_remove_minors(zc->zc_name);
+ if (error != 0)
+ zvol_create_minors(zc->zc_name);
return (error);
}
@@ -1344,9 +1345,10 @@ zfs_ioc_pool_export(zfs_cmd_t *zc)
boolean_t hardforce = (boolean_t)zc->zc_guid;
zfs_log_history(zc);
+ zvol_remove_minors(zc->zc_name);
error = spa_export(zc->zc_name, NULL, force, hardforce);
- if (error == 0)
- zvol_remove_minors(zc->zc_name);
+ if (error != 0)
+ zvol_create_minors(zc->zc_name);
return (error);
}
@@ -3272,9 +3274,11 @@ zfs_ioc_destroy(zfs_cmd_t *zc)
return (err);
}
- err = dmu_objset_destroy(zc->zc_name, zc->zc_defer_destroy);
- if (zc->zc_objset_type == DMU_OST_ZVOL && err == 0)
+ if (zc->zc_objset_type == DMU_OST_ZVOL)
(void) zvol_remove_minor(zc->zc_name);
+ err = dmu_objset_destroy(zc->zc_name, zc->zc_defer_destroy);
+ if (err != 0)
+ (void) zvol_create_minor(zc->zc_name);
return (err);
}
diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
index e5d159b..845702a 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
@@ -151,6 +151,9 @@ static int zvol_dumpify(zvol_state_t *zv);
static int zvol_dump_fini(zvol_state_t *zv);
static int zvol_dump_init(zvol_state_t *zv, boolean_t resize);
+static int zvol_first_open(zvol_state_t *zv);
+static void zvol_last_close(zvol_state_t *zv);
+
static zvol_state_t *zvol_geom_create(const char *name);
static void zvol_geom_run(zvol_state_t *zv);
static void zvol_geom_destroy(zvol_state_t *zv);
@@ -579,6 +582,9 @@ zvol_create_minor(const char *name)
zvol_minors++;
+ error = zvol_first_open(zv);
+ ASSERT(error == 0);
+
mutex_exit(&spa_namespace_lock);
zvol_geom_run(zv);
@@ -605,6 +611,8 @@ zvol_remove_zv(zvol_state_t *zv)
if (zv->zv_total_opens != 0)
return (EBUSY);
+ zvol_last_close(zv);
+
ZFS_LOG(1, "ZVOL %s destroyed.", zv->zv_name);
#ifdef sun
@@ -667,8 +675,9 @@ zvol_first_open(zvol_state_t *zv)
}
zv->zv_volsize = volsize;
zv->zv_zilog = zil_open(os, zvol_get_data);
+#ifdef sun
zvol_size_changed(zv);
-
+#endif
VERIFY(dsl_prop_get_integer(zv->zv_name, "readonly", &readonly,
NULL) == 0);
if (readonly || dmu_objset_is_snapshot(os) ||
@@ -887,42 +896,12 @@ out:
/*ARGSUSED*/
static int
-zvol_open(struct g_provider *pp, int flag, int count)
+zvol_open(zvol_state_t *zv, int flag, int count)
{
- zvol_state_t *zv;
int err = 0;
- boolean_t locked = B_FALSE;
-
- /*
- * Protect against recursively entering spa_namespace_lock
- * when spa_open() is used for a pool on a (local) ZVOL(s).
- * This is needed since we replaced upstream zfsdev_state_lock
- * with spa_namespace_lock in the ZVOL code.
- * We are using the same trick as spa_open().
- * Note that calls in zvol_first_open which need to resolve
- * pool name to a spa object will enter spa_open()
- * recursively, but that function already has all the
- * necessary protection.
- */
- if (!MUTEX_HELD(&spa_namespace_lock)) {
- mutex_enter(&spa_namespace_lock);
- locked = B_TRUE;
- }
- zv = pp->private;
- if (zv == NULL) {
- if (locked)
- mutex_exit(&spa_namespace_lock);
- return (ENXIO);
- }
+ ASSERT(zv != NULL);
- if (zv->zv_total_opens == 0)
- err = zvol_first_open(zv);
- if (err) {
- if (locked)
- mutex_exit(&spa_namespace_lock);
- return (err);
- }
if ((flag & FWRITE) && (zv->zv_flags & ZVOL_RDONLY)) {
err = EROFS;
goto out;
@@ -942,38 +921,17 @@ zvol_open(struct g_provider *pp, int flag, int count)
#endif
zv->zv_total_opens += count;
- if (locked)
- mutex_exit(&spa_namespace_lock);
- return (err);
out:
- if (zv->zv_total_opens == 0)
- zvol_last_close(zv);
- if (locked)
- mutex_exit(&spa_namespace_lock);
return (err);
}
/*ARGSUSED*/
static int
-zvol_close(struct g_provider *pp, int flag, int count)
+zvol_close(zvol_state_t *zv, int flag, int count)
{
- zvol_state_t *zv;
- int error = 0;
- boolean_t locked = B_FALSE;
- /* See comment in zvol_open(). */
- if (!MUTEX_HELD(&spa_namespace_lock)) {
- mutex_enter(&spa_namespace_lock);
- locked = B_TRUE;
- }
-
- zv = pp->private;
- if (zv == NULL) {
- if (locked)
- mutex_exit(&spa_namespace_lock);
- return (ENXIO);
- }
+ ASSERT(zv != NULL);
if (zv->zv_flags & ZVOL_EXCL) {
ASSERT(zv->zv_total_opens == 1);
@@ -991,12 +949,7 @@ zvol_close(struct g_provider *pp, int flag, int count)
*/
zv->zv_total_opens -= count;
- if (zv->zv_total_opens == 0)
- zvol_last_close(zv);
-
- if (locked)
- mutex_exit(&spa_namespace_lock);
- return (error);
+ return (0);
}
static void
@@ -2052,6 +2005,7 @@ zvol_geom_destroy(zvol_state_t *zv)
static int
zvol_geom_access(struct g_provider *pp, int acr, int acw, int ace)
{
+ zvol_state_t *zv;
int count, error, flags;
g_topology_assert();
@@ -2090,12 +2044,14 @@ zvol_geom_access(struct g_provider *pp, int acr, int
acw, int ace)
if (acw != 0)
flags |= FWRITE;
- g_topology_unlock();
+ zv = pp->private;
+ if (zv == NULL)
+ return (ENXIO);
+
if (count > 0)
- error = zvol_open(pp, flags, count);
+ error = zvol_open(zv, flags, count);
else
- error = zvol_close(pp, flags, -count);
- g_topology_lock();
+ error = zvol_close(zv, flags, -count);
return (error);
}
--
Andriy Gapon
More information about the zfs-devel
mailing list