Skip to content

Commit

Permalink
Illumos openzfs#3744
Browse files Browse the repository at this point in the history
3744 zfs shouldn't ignore errors unmounting snapshots
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Christopher Siden <christopher.siden@delphix.com>

References:
  https://www.illumos.org/issues/3744
  illumos/illumos-gate@fc7a6e3

Ported-by: Richard Yao <ryao@gentoo.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#1775

Porting notes:

1. There is no clear way to distinguish between a failure when we
   tried to unmount the snapdir of a zvol (which does not exist)
   and the failure when we try to unmount a snapdir of a dataset,
   so the changes to zfs_unmount_snap() were dropped in favor of
   an altered Linux function that unconditionally returns 0.
  • Loading branch information
wca authored and unya committed Dec 13, 2013
1 parent 3cb58d4 commit ec746b1
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 15 deletions.
2 changes: 1 addition & 1 deletion include/sys/zfs_ioctl.h
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ extern int zfs_secpolicy_snapshot_perms(const char *name, cred_t *cr);
extern int zfs_secpolicy_rename_perms(const char *from,
const char *to, cred_t *cr);
extern int zfs_secpolicy_destroy_perms(const char *name, cred_t *cr);
extern void zfs_unmount_snap(const char *);
extern int zfs_unmount_snap(const char *);
extern void zfs_destroy_unmount_origin(const char *);

enum zfsdev_state_type {
Expand Down
2 changes: 1 addition & 1 deletion module/zfs/dsl_userhold.c
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ dsl_dataset_user_release_tmp(dsl_pool_t *dp, uint64_t dsobj, const char *htag)
dsl_dataset_name(ds, name);
dsl_dataset_rele(ds, FTAG);
dsl_pool_config_exit(dp, FTAG);
zfs_unmount_snap(name);
(void) zfs_unmount_snap(name);
} else {
dsl_pool_config_exit(dp, FTAG);
}
Expand Down
43 changes: 30 additions & 13 deletions module/zfs/zfs_ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -3348,8 +3348,16 @@ zfs_ioc_log_history(const char *unused, nvlist_t *innvl, nvlist_t *outnvl)
*
* This function is best-effort. Callers must deal gracefully if it
* remains mounted (or is remounted after this call).
*
* XXX: This function should detect a failure to unmount a snapdir of a dataset
* and return the appropriate error code when it is mounted. Its Illumos and
* FreeBSD counterparts do this. We do not do this on Linux because there is no
* clear way to access the mount information that FreeBSD and Illumos use to
* distinguish between things with mounted snapshot directories, and things
* without mounted snapshot directories, which include zvols. Returning a
* failure for the latter causes `zfs destroy` to fail on zvol snapshots.
*/
void
int
zfs_unmount_snap(const char *snapname)
{
zfs_sb_t *zsb = NULL;
Expand All @@ -3358,7 +3366,7 @@ zfs_unmount_snap(const char *snapname)
char *ptr;

if ((ptr = strchr(snapname, '@')) == NULL)
return;
return (0);

dsname = kmem_alloc(ptr - snapname + 1, KM_SLEEP);
strlcpy(dsname, snapname, ptr - snapname + 1);
Expand All @@ -3373,15 +3381,14 @@ zfs_unmount_snap(const char *snapname)
kmem_free(dsname, ptr - snapname + 1);
strfree(fullname);

return;
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));
}

/*
Expand All @@ -3404,7 +3411,7 @@ zfs_destroy_unmount_origin(const char *fsname)
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);
}
Expand All @@ -3421,7 +3428,7 @@ zfs_destroy_unmount_origin(const char *fsname)
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;
Expand All @@ -3442,8 +3449,10 @@ zfs_ioc_destroy_snaps(const char *poolname, nvlist_t *innvl, nvlist_t *outnvl)
(name[poollen] != '/' && name[poollen] != '@'))
return (SET_ERROR(EXDEV));

zfs_unmount_snap(name);
(void) zvol_remove_minor(name);
error = zfs_unmount_snap(name);
if (error != 0)
return (error);
}

return (dsl_destroy_snapshots_nvl(snaps, defer, outnvl));
Expand All @@ -3461,8 +3470,12 @@ 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 != 0)
return (err);
}

if (strchr(zc->zc_name, '@'))
err = dsl_destroy_snapshot(zc->zc_name, zc->zc_defer_destroy);
Expand Down Expand Up @@ -3510,7 +3523,7 @@ recursive_unmount(const char *fsname, void *arg)
fullname = kmem_asprintf("%s@%s", fsname, snapname);
zfs_unmount_snap(fullname);
strfree(fullname);
return (0);
return (zfs_unmount_snap(fullname));
}

/*
Expand Down Expand Up @@ -4855,14 +4868,18 @@ 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 != 0)
return (err);
}

return (dsl_dataset_user_release(holds, errlist));
}
Expand Down

0 comments on commit ec746b1

Please sign in to comment.