Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 3 Oct 2014 14:49:49 +0000 (UTC)
From:      Steven Hartland <smh@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r272474 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Message-ID:  <201410031449.s93EnnR9009098@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: smh
Date: Fri Oct  3 14:49:48 2014
New Revision: 272474
URL: https://svnweb.freebsd.org/changeset/base/272474

Log:
  Fix various issues with zvols
  
  When performing snapshot renames we could deadlock due to the locking
  in zvol_rename_minors. In order to avoid this use the same workaround
  as zvol_open in zvol_rename_minors.
  
  Add missing zvol_rename_minors to dsl_dataset_promote_sync.
  
  Protect against invalid index into zv_name in zvol_remove_minors.
  
  Replace zvol_remove_minor calls with zvol_remove_minors to ensure
  any potential children are also renamed.
  
  Don't fail zvol_create_minors if zvol_create_minor returns EEXIST.
  
  Restore the valid pool check in zfs_ioc_destroy_snaps to ensure we
  don't call zvol_remove_minors when zfs_unmount_snap fails.
  
  PR:		193803
  MFC after:	1 week
  Sponsored by:	Multiplay

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c	Fri Oct  3 12:20:37 2014	(r272473)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dataset.c	Fri Oct  3 14:49:48 2014	(r272474)
@@ -2257,6 +2257,9 @@ dsl_dataset_promote_sync(void *arg, dmu_
 	dsl_dir_t *odd = NULL;
 	uint64_t oldnext_obj;
 	int64_t delta;
+#if defined(__FreeBSD__) && defined(_KERNEL)
+	char *oldname, *newname;
+#endif
 
 	VERIFY0(promote_hold(ddpa, dp, FTAG));
 	hds = ddpa->ddpa_clone;
@@ -2322,6 +2325,14 @@ dsl_dataset_promote_sync(void *arg, dmu_
 		    dd->dd_phys->dd_clones, origin_head->ds_object, tx));
 	}
 
+#if defined(__FreeBSD__) && defined(_KERNEL)
+	/* Take the spa_namespace_lock early so zvol renames don't deadlock. */
+	mutex_enter(&spa_namespace_lock);
+
+	oldname = kmem_alloc(MAXPATHLEN, KM_SLEEP);
+	newname = kmem_alloc(MAXPATHLEN, KM_SLEEP);
+#endif
+
 	/* move snapshots to this dir */
 	for (snap = list_head(&ddpa->shared_snaps); snap;
 	    snap = list_next(&ddpa->shared_snaps, snap)) {
@@ -2356,6 +2367,12 @@ dsl_dataset_promote_sync(void *arg, dmu_
 		VERIFY0(dsl_dir_hold_obj(dp, dd->dd_object,
 		    NULL, ds, &ds->ds_dir));
 
+#if defined(__FreeBSD__) && defined(_KERNEL)
+		dsl_dataset_name(ds, newname);
+		zfsvfs_update_fromname(oldname, newname);
+		zvol_rename_minors(oldname, newname);
+#endif
+
 		/* move any clone references */
 		if (ds->ds_phys->ds_next_clones_obj &&
 		    spa_version(dp->dp_spa) >= SPA_VERSION_DIR_CLONES) {
@@ -2393,6 +2410,12 @@ dsl_dataset_promote_sync(void *arg, dmu_
 		ASSERT(!dsl_prop_hascb(ds));
 	}
 
+#if defined(__FreeBSD__) && defined(_KERNEL)
+	mutex_exit(&spa_namespace_lock);
+
+	kmem_free(newname, MAXPATHLEN);
+	kmem_free(oldname, MAXPATHLEN);
+#endif
 	/*
 	 * Change space accounting.
 	 * Note, pa->*usedsnap and dd_used_breakdown[SNAP] will either

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c	Fri Oct  3 12:20:37 2014	(r272473)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c	Fri Oct  3 14:49:48 2014	(r272474)
@@ -3540,6 +3540,7 @@ zfs_destroy_unmount_origin(const char *f
 static int
 zfs_ioc_destroy_snaps(const char *poolname, nvlist_t *innvl, nvlist_t *outnvl)
 {
+	int error, poollen;
 	nvlist_t *snaps;
 	nvpair_t *pair;
 	boolean_t defer;
@@ -3548,13 +3549,24 @@ zfs_ioc_destroy_snaps(const char *poolna
 		return (SET_ERROR(EINVAL));
 	defer = nvlist_exists(innvl, "defer");
 
+	poollen = strlen(poolname);
 	for (pair = nvlist_next_nvpair(snaps, NULL); pair != NULL;
 	    pair = nvlist_next_nvpair(snaps, pair)) {
 		const char *name = nvpair_name(pair);
 
-		(void) zfs_unmount_snap(name);
+		/*
+		 * The snap must be in the specified pool to prevent the
+		 * invalid removal of zvol minors below.
+		 */
+		if (strncmp(name, poolname, poollen) != 0 ||
+		    (name[poollen] != '/' && name[poollen] != '@'))
+			return (SET_ERROR(EXDEV));
+
+		error = zfs_unmount_snap(name);
+		if (error != 0)
+			return (error);
 #if defined(__FreeBSD__)
-		(void) zvol_remove_minor(name);
+		zvol_remove_minors(name);
 #endif
 	}
 
@@ -3679,7 +3691,11 @@ zfs_ioc_destroy(zfs_cmd_t *zc)
 	else
 		err = dsl_destroy_head(zc->zc_name);
 	if (zc->zc_objset_type == DMU_OST_ZVOL && err == 0)
+#ifdef __FreeBSD__
+		zvol_remove_minors(zc->zc_name);
+#else
 		(void) zvol_remove_minor(zc->zc_name);
+#endif
 	return (err);
 }
 

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c	Fri Oct  3 12:20:37 2014	(r272473)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c	Fri Oct  3 14:49:48 2014	(r272474)
@@ -882,7 +882,8 @@ zvol_remove_minors(const char *name)
 	LIST_FOREACH_SAFE(zv, &all_zvols, zv_links, tzv) {
 		if (strcmp(zv->zv_name, name) == 0 ||
 		    (strncmp(zv->zv_name, name, namelen) == 0 &&
-		     zv->zv_name[namelen] == '/')) {
+		    strlen(zv->zv_name) > namelen && (zv->zv_name[namelen] == '/' ||
+		    zv->zv_name[namelen] == '@'))) {
 			(void) zvol_remove_zv(zv);
 		}
 	}
@@ -2570,9 +2571,10 @@ zvol_create_minors(const char *name)
 	if (dmu_objset_type(os) == DMU_OST_ZVOL) {
 		dsl_dataset_long_hold(os->os_dsl_dataset, FTAG);
 		dsl_pool_rele(dmu_objset_pool(os), FTAG);
-		if ((error = zvol_create_minor(name)) == 0)
+		error = zvol_create_minor(name);
+		if (error == 0 || error == EEXIST) {
 			error = zvol_create_snapshots(os, name);
-		else {
+		} else {
 			printf("ZFS WARNING: Unable to create ZVOL %s (error=%d).\n",
 			    name, error);
 		}
@@ -2673,12 +2675,17 @@ zvol_rename_minors(const char *oldname, 
 	size_t oldnamelen, newnamelen;
 	zvol_state_t *zv;
 	char *namebuf;
+	boolean_t locked = B_FALSE;
 
 	oldnamelen = strlen(oldname);
 	newnamelen = strlen(newname);
 
 	DROP_GIANT();
-	mutex_enter(&spa_namespace_lock);
+	/* See comment in zvol_open(). */
+	if (!MUTEX_HELD(&spa_namespace_lock)) {
+		mutex_enter(&spa_namespace_lock);
+		locked = B_TRUE;
+	}
 
 	LIST_FOREACH(zv, &all_zvols, zv_links) {
 		if (strcmp(zv->zv_name, oldname) == 0) {
@@ -2693,7 +2700,8 @@ zvol_rename_minors(const char *oldname, 
 		}
 	}
 
-	mutex_exit(&spa_namespace_lock);
+	if (locked)
+		mutex_exit(&spa_namespace_lock);
 	PICKUP_GIANT();
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201410031449.s93EnnR9009098>