3744 zfs shouldn't ignore errors unmounting snapshots
Submitted by:   Will Andrews <willa@spectralogic.com>
Reviewed by:    Matthew Ahrens <mahrens@delphix.com>

@@ -3366,45 +3366,48 @@
  * The dp_config_rwlock must not be held when calling this, because the
  * unmount may need to write out data.
  *
  * This function is best-effort.  Callers must deal gracefully if it
  * remains mounted (or is remounted after this call).
+ *
+ * Returns 0 if the argument is not a snapshot, or it is not currently a
+ * filesystem, or we were able to unmount it.  Returns error code otherwise.
  */
-void
+int
 zfs_unmount_snap(const char *snapname)
 {
         vfs_t *vfsp;
         zfsvfs_t *zfsvfs;
+        int err;
 
         if (strchr(snapname, '@') == NULL)
-                return;
+                return (0);
 
         vfsp = zfs_get_vfs(snapname);
         if (vfsp == NULL)
-                return;
+                return (0);
 
         zfsvfs = vfsp->vfs_data;
         ASSERT(!dsl_pool_config_held(dmu_objset_pool(zfsvfs->z_os)));
 
-        if (vn_vfswlock(vfsp->vfs_vnodecovered) != 0) {
-                VFS_RELE(vfsp);
-                return;
-        }
+        err = vn_vfswlock(vfsp->vfs_vnodecovered);
         VFS_RELE(vfsp);
+        if (err != 0)
+                return (err);
 
         /*
          * Always force the unmount for snapshots.
          */
         (void) dounmount(vfsp, MS_FORCE, kcred);
+        return (0);
 }
 
 /* ARGSUSED */
 static int
 zfs_unmount_snap_cb(const char *snapname, void *arg)
 {
-        zfs_unmount_snap(snapname);
-        return (0);
+        return (zfs_unmount_snap(snapname));
 }
 
 /*
  * When a clone is destroyed, its origin may also need to be destroyed,
  * in which case it must be unmounted.  This routine will do that unmount

@@ -3423,11 +3426,11 @@
         ds = dmu_objset_ds(os);
         if (dsl_dir_is_clone(ds->ds_dir) && DS_IS_DEFER_DESTROY(ds->ds_prev)) {
                 char originname[MAXNAMELEN];
                 dsl_dataset_name(ds->ds_prev, originname);
                 dmu_objset_rele(os, FTAG);
-                zfs_unmount_snap(originname);
+                (void) zfs_unmount_snap(originname);
         } else {
                 dmu_objset_rele(os, FTAG);
         }
 }
 

@@ -3441,11 +3444,11 @@
  *
  */
 static int
 zfs_ioc_destroy_snaps(const char *poolname, nvlist_t *innvl, nvlist_t *outnvl)
 {
-        int poollen;
+        int error, poollen;
         nvlist_t *snaps;
         nvpair_t *pair;
         boolean_t defer;
 
         if (nvlist_lookup_nvlist(innvl, "snaps", &snaps) != 0)

@@ -3462,11 +3465,13 @@
                  */
                 if (strncmp(name, poolname, poollen) != 0 ||
                     (name[poollen] != '/' && name[poollen] != '@'))
                         return (SET_ERROR(EXDEV));
 
-                zfs_unmount_snap(name);
+                error = zfs_unmount_snap(name);
+                if (error)
+                        return (SET_ERROR(error));
         }
 
         return (dsl_destroy_snapshots_nvl(snaps, defer, outnvl));
 }
 

@@ -3480,12 +3485,16 @@
  */
 static int
 zfs_ioc_destroy(zfs_cmd_t *zc)
 {
         int err;
-        if (strchr(zc->zc_name, '@') && zc->zc_objset_type == DMU_OST_ZFS)
-                zfs_unmount_snap(zc->zc_name);
+
+        if (zc->zc_objset_type == DMU_OST_ZFS) {
+                err = zfs_unmount_snap(zc->zc_name);
+                if (err)
+                        return (err);
+        }
 
         if (strchr(zc->zc_name, '@'))
                 err = dsl_destroy_snapshot(zc->zc_name, zc->zc_defer_destroy);
         else
                 err = dsl_destroy_head(zc->zc_name);

@@ -3527,12 +3536,11 @@
 {
         const char *snapname = arg;
         char fullname[MAXNAMELEN];
 
         (void) snprintf(fullname, sizeof (fullname), "%s@%s", fsname, snapname);
-        zfs_unmount_snap(fullname);
-        return (0);
+        return (zfs_unmount_snap(fullname));
 }
 
 /*
  * inputs:
  * zc_name      old name of dataset

@@ -4987,18 +4995,22 @@
 /* ARGSUSED */
 static int
 zfs_ioc_release(const char *pool, nvlist_t *holds, nvlist_t *errlist)
 {
         nvpair_t *pair;
+        int err;
 
         /*
          * The release may cause the snapshot to be destroyed; make sure it
          * is not mounted.
          */
         for (pair = nvlist_next_nvpair(holds, NULL); pair != NULL;
-            pair = nvlist_next_nvpair(holds, pair))
-                zfs_unmount_snap(nvpair_name(pair));
+            pair = nvlist_next_nvpair(holds, pair)) {
+                err = zfs_unmount_snap(nvpair_name(pair));
+                if (err)
+                        return (err);
+        }
 
         return (dsl_dataset_user_release(holds, errlist));
 }
 
 /*