diff --git a/include/libzfs_impl.h b/include/libzfs_impl.h index 959ba85b7..568103f4b 100644 --- a/include/libzfs_impl.h +++ b/include/libzfs_impl.h @@ -22,6 +22,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2011, 2015 by Delphix. All rights reserved. + * Copyright (c) 2018 Datto Inc. */ #ifndef _LIBZFS_IMPL_H @@ -146,6 +147,10 @@ int zprop_expand_list(libzfs_handle_t *hdl, zprop_list_t **plp, * mounted. */ #define CL_GATHER_MOUNT_ALWAYS 1 +/* + * changelist_gather() flag to force it to iterate on mounted datasets only + */ +#define CL_GATHER_ITER_MOUNTED 2 typedef struct prop_changelist prop_changelist_t; diff --git a/lib/libzfs/libzfs_changelist.c b/lib/libzfs/libzfs_changelist.c index ae9f1d179..3101febc1 100644 --- a/lib/libzfs/libzfs_changelist.c +++ b/lib/libzfs/libzfs_changelist.c @@ -553,39 +553,50 @@ change_one(zfs_handle_t *zhp, void *data) return (0); } -/*ARGSUSED*/ static int -compare_mountpoints(const void *a, const void *b, void *unused) +compare_props(const void *a, const void *b, zfs_prop_t prop) { const prop_changenode_t *ca = a; const prop_changenode_t *cb = b; - char mounta[MAXPATHLEN]; - char mountb[MAXPATHLEN]; + char propa[MAXPATHLEN]; + char propb[MAXPATHLEN]; - boolean_t hasmounta, hasmountb; + boolean_t haspropa, haspropb; + haspropa = (zfs_prop_get(ca->cn_handle, prop, propa, sizeof (propa), + NULL, NULL, 0, B_FALSE) == 0); + haspropb = (zfs_prop_get(cb->cn_handle, prop, propb, sizeof (propb), + NULL, NULL, 0, B_FALSE) == 0); + + if (!haspropa && haspropb) + return (-1); + else if (haspropa && !haspropb) + return (1); + else if (!haspropa && !haspropb) + return (0); + else + return (strcmp(propb, propa)); +} + +/*ARGSUSED*/ +static int +compare_mountpoints(const void *a, const void *b, void *unused) +{ /* * When unsharing or unmounting filesystems, we need to do it in * mountpoint order. This allows the user to have a mountpoint * hierarchy that is different from the dataset hierarchy, and still - * allow it to be changed. However, if either dataset doesn't have a - * mountpoint (because it is a volume or a snapshot), we place it at the - * end of the list, because it doesn't affect our change at all. + * allow it to be changed. */ - hasmounta = (zfs_prop_get(ca->cn_handle, ZFS_PROP_MOUNTPOINT, mounta, - sizeof (mounta), NULL, NULL, 0, B_FALSE) == 0); - hasmountb = (zfs_prop_get(cb->cn_handle, ZFS_PROP_MOUNTPOINT, mountb, - sizeof (mountb), NULL, NULL, 0, B_FALSE) == 0); + return (compare_props(a, b, ZFS_PROP_MOUNTPOINT)); +} - if (!hasmounta && hasmountb) - return (-1); - else if (hasmounta && !hasmountb) - return (1); - else if (!hasmounta && !hasmountb) - return (0); - else - return (strcmp(mountb, mounta)); +/*ARGSUSED*/ +static int +compare_dataset_names(const void *a, const void *b, void *unused) +{ + return (compare_props(a, b, ZFS_PROP_NAME)); } /* @@ -630,7 +641,7 @@ changelist_gather(zfs_handle_t *zhp, zfs_prop_t prop, int gather_flags, clp->cl_pool = uu_avl_pool_create("changelist_pool", sizeof (prop_changenode_t), offsetof(prop_changenode_t, cn_treenode), - compare_mountpoints, 0); + legacy ? compare_dataset_names : compare_mountpoints, 0); if (clp->cl_pool == NULL) { assert(uu_error() == UU_ERROR_NO_MEMORY); (void) zfs_error(zhp->zfs_hdl, EZFS_NOMEM, "internal error"); @@ -687,7 +698,7 @@ changelist_gather(zfs_handle_t *zhp, zfs_prop_t prop, int gather_flags, clp->cl_shareprop = ZFS_PROP_SHARENFS; if (clp->cl_prop == ZFS_PROP_MOUNTPOINT && - (clp->cl_gflags & CL_GATHER_MOUNT_ALWAYS) == 0) { + (clp->cl_gflags & CL_GATHER_ITER_MOUNTED)) { /* * Instead of iterating through all of the dataset children we * gather mounted dataset children from MNTTAB diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index b6dd4f166..5767b2ee3 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -30,6 +30,7 @@ * Copyright 2017 Nexenta Systems, Inc. * Copyright 2016 Igor Kozhukhov * Copyright 2017-2018 RackTop Systems. + * Copyright (c) 2018 Datto Inc. */ #include @@ -4548,7 +4549,8 @@ zfs_rename(zfs_handle_t *zhp, const char *target, boolean_t recursive, goto error; } } else if (zhp->zfs_type != ZFS_TYPE_SNAPSHOT) { - if ((cl = changelist_gather(zhp, ZFS_PROP_NAME, 0, + if ((cl = changelist_gather(zhp, ZFS_PROP_NAME, + CL_GATHER_ITER_MOUNTED, force_unmount ? MS_FORCE : 0)) == NULL) return (-1); diff --git a/lib/libzfs/libzfs_mount.c b/lib/libzfs/libzfs_mount.c index 252112e24..23e45d0d3 100644 --- a/lib/libzfs/libzfs_mount.c +++ b/lib/libzfs/libzfs_mount.c @@ -25,6 +25,7 @@ * Copyright (c) 2014, 2015 by Delphix. All rights reserved. * Copyright 2016 Igor Kozhukhov * Copyright 2017 RackTop Systems. + * Copyright (c) 2018 Datto Inc. */ /* @@ -699,7 +700,8 @@ zfs_unmountall(zfs_handle_t *zhp, int flags) prop_changelist_t *clp; int ret; - clp = changelist_gather(zhp, ZFS_PROP_MOUNTPOINT, 0, flags); + clp = changelist_gather(zhp, ZFS_PROP_MOUNTPOINT, + CL_GATHER_ITER_MOUNTED, 0); if (clp == NULL) return (-1); diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index 776aff4d4..d33c55d9c 100644 --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -166,7 +166,8 @@ tests = ['zfs_get_001_pos', 'zfs_get_002_pos', 'zfs_get_003_pos', tags = ['functional', 'cli_root', 'zfs_get'] [tests/functional/cli_root/zfs_inherit] -tests = ['zfs_inherit_001_neg', 'zfs_inherit_002_neg', 'zfs_inherit_003_pos'] +tests = ['zfs_inherit_001_neg', 'zfs_inherit_002_neg', 'zfs_inherit_003_pos', + 'zfs_inherit_mountpoint'] tags = ['functional', 'cli_root', 'zfs_inherit'] [tests/functional/cli_root/zfs_load-key] @@ -218,7 +219,7 @@ tests = ['zfs_rename_001_pos', 'zfs_rename_002_pos', 'zfs_rename_003_pos', 'zfs_rename_007_pos', 'zfs_rename_008_pos', 'zfs_rename_009_neg', 'zfs_rename_010_neg', 'zfs_rename_011_pos', 'zfs_rename_012_neg', 'zfs_rename_013_pos', 'zfs_rename_014_neg', 'zfs_rename_encrypted_child', - 'zfs_rename_to_encrypted'] + 'zfs_rename_to_encrypted', 'zfs_rename_mountpoint'] tags = ['functional', 'cli_root', 'zfs_rename'] [tests/functional/cli_root/zfs_reservation] diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_inherit/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/zfs_inherit/Makefile.am index 104ee069c..95a51ec75 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_inherit/Makefile.am +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_inherit/Makefile.am @@ -4,4 +4,5 @@ dist_pkgdata_SCRIPTS = \ setup.ksh \ zfs_inherit_001_neg.ksh \ zfs_inherit_002_neg.ksh \ - zfs_inherit_003_pos.ksh + zfs_inherit_003_pos.ksh \ + zfs_inherit_mountpoint.ksh diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_inherit/zfs_inherit_mountpoint.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_inherit/zfs_inherit_mountpoint.ksh new file mode 100755 index 000000000..9c1251533 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_inherit/zfs_inherit_mountpoint.ksh @@ -0,0 +1,62 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy is of the CDDL is also available via the Internet +# at http://www.illumos.org/license/CDDL. +# +# CDDL HEADER END +# + +# +# Copyright (c) 2018 Datto Inc. +# + +. $STF_SUITE/include/libtest.shlib + +# +# DESCRIPTION: +# zfs inherit should inherit mountpoint on mountpoint=none children +# +# STRATEGY: +# 1. Create a set of nested datasets with mountpoint=none +# 2. Verify datasets aren't mounted +# 3. Inherit mountpoint and verify all datasets are now mounted +# + +verify_runnable "both" + +function inherit_cleanup +{ + log_must zfs destroy -fR $TESTPOOL/inherit_test +} + +log_onexit inherit_cleanup + + +log_must zfs create -o mountpoint=none $TESTPOOL/inherit_test +log_must zfs create $TESTPOOL/inherit_test/child + +if ismounted $TESTPOOL/inherit_test; then + log_fail "$TESTPOOL/inherit_test is mounted" +fi +if ismounted $TESTPOOL/inherit_test/child; then + log_fail "$TESTPOOL/inherit_test/child is mounted" +fi + +log_must zfs inherit mountpoint $TESTPOOL/inherit_test + +if ! ismounted $TESTPOOL/inherit_test; then + log_fail "$TESTPOOL/inherit_test is not mounted" +fi +if ! ismounted $TESTPOOL/inherit_test/child; then + log_fail "$TESTPOOL/inherit_test/child is not mounted" +fi + +log_pass "Verified mountpoint for mountpoint=none children inherited." diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_rename/Makefile.am b/tests/zfs-tests/tests/functional/cli_root/zfs_rename/Makefile.am index 974538dd9..406e27881 100644 --- a/tests/zfs-tests/tests/functional/cli_root/zfs_rename/Makefile.am +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_rename/Makefile.am @@ -17,7 +17,8 @@ dist_pkgdata_SCRIPTS = \ zfs_rename_013_pos.ksh \ zfs_rename_014_neg.ksh \ zfs_rename_encrypted_child.ksh \ - zfs_rename_to_encrypted.ksh + zfs_rename_to_encrypted.ksh \ + zfs_rename_mountpoint.ksh dist_pkgdata_DATA = \ zfs_rename.cfg \ diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_mountpoint.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_mountpoint.ksh new file mode 100755 index 000000000..4d2b94dc8 --- /dev/null +++ b/tests/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_mountpoint.ksh @@ -0,0 +1,88 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy is of the CDDL is also available via the Internet +# at http://www.illumos.org/license/CDDL. +# +# CDDL HEADER END +# + +# +# Copyright (c) 2018 Datto Inc. +# + +. $STF_SUITE/include/libtest.shlib + +# +# DESCRIPTION: +# zfs rename should rename datasets even for mountpoint=none children +# +# STRATEGY: +# 1. Create a set of nested datasets with mountpoint=none +# 2. Verify datasets aren't mounted except for the parent +# 3. Rename mountpoint and verify all child datasets are renamed +# + +verify_runnable "both" + +function rename_cleanup +{ + log_note zfs destroy -fR $TESTPOOL/rename_test + log_note zfs destroy -fR $TESTPOOL/renamed +} + +log_onexit rename_cleanup + + +log_must zfs create $TESTPOOL/rename_test +log_must zfs create $TESTPOOL/rename_test/ds +log_must zfs create -o mountpoint=none $TESTPOOL/rename_test/child +log_must zfs create $TESTPOOL/rename_test/child/grandchild + +if ! ismounted $TESTPOOL/rename_test; then + log_fail "$TESTPOOL/rename_test is not mounted" +fi +if ! ismounted $TESTPOOL/rename_test/ds; then + log_fail "$TESTPOOL/rename_test/ds is not mounted" +fi +if ismounted $TESTPOOL/rename_test/child; then + log_fail "$TESTPOOL/rename_test/child is mounted" +fi +if ismounted $TESTPOOL/rename_test/child/grandchild; then + log_fail "$TESTPOOL/rename_test/child/grandchild is mounted" +fi + +log_must zfs rename $TESTPOOL/rename_test $TESTPOOL/renamed + +log_mustnot zfs list $TESTPOOL/rename_test +log_mustnot zfs list $TESTPOOL/rename_test/ds +log_mustnot zfs list $TESTPOOL/rename_test/child +log_mustnot zfs list $TESTPOOL/rename_test/child/grandchild + +log_must zfs list $TESTPOOL/renamed +log_must zfs list $TESTPOOL/renamed/ds +log_must zfs list $TESTPOOL/renamed/child +log_must zfs list $TESTPOOL/renamed/child/grandchild + +if ! ismounted $TESTPOOL/renamed; then + log_must zfs get all $TESTPOOL/renamed + log_fail "$TESTPOOL/renamed is not mounted" +fi +if ! ismounted $TESTPOOL/renamed/ds; then + log_fail "$TESTPOOL/renamed/ds is not mounted" +fi +if ismounted $TESTPOOL/renamed/child; then + log_fail "$TESTPOOL/renamed/child is mounted" +fi +if ismounted $TESTPOOL/renamed/child/grandchild; then + log_fail "$TESTPOOL/renamed/child/grandchild is mounted" +fi + +log_pass "Verified rename for mountpoint=none children."