Its possible to change CANMOUNT from ON to NOAUTO on a mounted filesystem, but its not possible to change CANMOUNT from NOAUTO to ON as it REMOUNTS that dataset even if its ALREADY MOUNTED. Fix: Do not remount dataset after setting CANMOUNT to ON from NOAUTO when its already mounted. How-To-Repeat: # mount | head -1 zroot/ROOT/default on / (zfs, local, noatime, nfsv4acls) # zfs get canmount zroot/ROOT/default NAME PROPERTY VALUE SOURCE zroot/ROOT/default canmount noauto local # zfs set canmount=on zroot/ROOT/default cannot unmount '/': Invalid argument
Responsible Changed From-To: freebsd-bugs->freebsd-fs Over to maintainer(s).
Attached is a patch to fix setting 'zfs set canmount=on' to not cause a remount if the dataset is *already mounted*. This fixes the issue reported here, as well as here http://lists.freebsd.org/pipermail/freebsd-fs/2012-May/014241.html $ cd /usr/src/cddl $ patch -p1 < patch-zfs-dataset-canmount-on.txt $ make obj depend all install The change adds to the complex condition as I did not want to refactor it too much given the unclear "contrib" status of the code. Also attached is a test script to see the functionality before and after. I did some research and neither OpenIndiana/Illumos nor ZfsOnLinux have addressed this issue. Not sure the proper way to share or report this "upstream" currently. Regards, Bryan Drewery
Hi, at last I had some time to check Your work, but the patch did not applied to 9-STABLE @ r236934: / # cd /usr/src/cddl /usr/src/cddl # patch -p1 < /root/patch-zfs-dataset-canmount-on Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |--- cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c.orig 2012-06-12 00:10:11.000000000 -0500 |+++ cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c 2012-06-12 00:17:34.000000000 -0500 -------------------------- Patching file contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c using Plan A... Hunk #1 failed at 1467. 1 out of 1 hunks failed--saving rejects to contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c.rej done Regards, vermaden "Bryan Drewery" <bryan@shatow.net> pisze: > Attached is a patch to fix setting 'zfs set canmount=on' to not cause a > remount if the dataset is *already mounted*. This fixes the issue > reported here, as well as here > http://lists.freebsd.org/pipermail/freebsd-fs/2012-May/014241.html > > $ cd /usr/src/cddl > $ patch -p1 < patch-zfs-dataset-canmount-on.txt > $ make obj depend all install > > The change adds to the complex condition as I did not want to refactor > it too much given the unclear "contrib" status of the code. > > Also attached is a test script to see the functionality before and after. > > I did some research and neither OpenIndiana/Illumos nor ZfsOnLinux have > addressed this issue. Not sure the proper way to share or report this > "upstream" currently. > > Regards, > Bryan Drewery > -- ...
On 6/13/2012 1:37 AM, vermaden wrote: > at last I had some time to check Your work, but the patch did not applied to 9-STABLE @ r236934: It applies to HEAD and 9/stable for me. HEAD/cddl# patch -p1 -C < ../patch Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |--- cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c.orig 2012-06-12 00:10:11.000000000 -0500 |+++ cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c 2012-06-12 00:17:34.000000000 -0500 -------------------------- Patching file contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c using Plan A... Hunk #1 succeeded at 1485 (offset 18 lines). done -- Regards, Bryan Drewery bdrewery@freenode/EFNet
Responsible Changed From-To: freebsd-fs->mm I'll take it.
I agree to your change, I have modified it only cosmetically. CDDL requires copyright information, so you have two choices: a) you provide me with your full name and will be added to the Copyright section with your full name b) you pass the copyright to me and allow me to commit without changing the Copyright header (my Copyright for 2012 is already included) Thanks, mm -- Martin Matuska FreeBSD committer http://blog.vx.sk
Hi, Bryan Drewery (CC) is author of that patch, ask him about the copyright line, I am the author of beadm on FreeBSD. Regards, vermaden "Martin Matuska" <mm@FreeBSD.org> pisze: > I agree to your change, I have modified it only cosmetically. >=20 > CDDL requires copyright information, so you have two choices: > a) you provide me with your full name and will be added to the Copyright > section with your full name > b) you pass the copyright to me and allow me to commit without changing > the Copyright header (my Copyright for 2012 is already included) >=20 > Thanks, > mm >=20 > --=20 > Martin Matuska > FreeBSD committer > http://blog.vx.sk >=20 >=20 --=20 ...
I'm fine passing the copyright to you. Thanks, Bryan Drewery
Author: mm Date: Fri Jun 15 07:38:21 2012 New Revision: 237119 URL: http://svn.freebsd.org/changeset/base/237119 Log: Do not remount ZFS dataset if changing canmount property to "on" and dataset is already mounted. PR: 167905 Submitted by: Bryan Drewery <bryan@shatow.net> MFC after: 1 week Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c ============================================================================== --- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Fri Jun 15 07:26:39 2012 (r237118) +++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Fri Jun 15 07:38:21 2012 (r237119) @@ -1485,11 +1485,13 @@ zfs_prop_set(zfs_handle_t *zhp, const ch /* * If the dataset's canmount property is being set to noauto, + * or being set to on and the dataset is already mounted, * then we want to prevent unmounting & remounting it. */ do_prefix = !((prop == ZFS_PROP_CANMOUNT) && (zprop_string_to_index(prop, propval, &idx, - ZFS_TYPE_DATASET) == 0) && (idx == ZFS_CANMOUNT_NOAUTO)); + ZFS_TYPE_DATASET) == 0) && (idx == ZFS_CANMOUNT_NOAUTO || + (idx == ZFS_CANMOUNT_ON && zfs_is_mounted(zhp, NULL)))); if (do_prefix && (ret = changelist_prefix(cl)) != 0) goto error; _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Author: mm Date: Fri Jun 22 20:38:00 2012 New Revision: 237456 URL: http://svn.freebsd.org/changeset/base/237456 Log: MFC r237119: Do not remount ZFS dataset if changing canmount property to "on" and dataset is already mounted. PR: 167905 Submitted by: Bryan Drewery <bryan@shatow.net> Modified: stable/9/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Directory Properties: stable/9/cddl/contrib/opensolaris/ (props changed) Modified: stable/9/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c ============================================================================== --- stable/9/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Fri Jun 22 19:19:58 2012 (r237455) +++ stable/9/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Fri Jun 22 20:38:00 2012 (r237456) @@ -1485,11 +1485,13 @@ zfs_prop_set(zfs_handle_t *zhp, const ch /* * If the dataset's canmount property is being set to noauto, + * or being set to on and the dataset is already mounted, * then we want to prevent unmounting & remounting it. */ do_prefix = !((prop == ZFS_PROP_CANMOUNT) && (zprop_string_to_index(prop, propval, &idx, - ZFS_TYPE_DATASET) == 0) && (idx == ZFS_CANMOUNT_NOAUTO)); + ZFS_TYPE_DATASET) == 0) && (idx == ZFS_CANMOUNT_NOAUTO || + (idx == ZFS_CANMOUNT_ON && zfs_is_mounted(zhp, NULL)))); if (do_prefix && (ret = changelist_prefix(cl)) != 0) goto error; _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Author: mm Date: Fri Jun 22 20:38:08 2012 New Revision: 237457 URL: http://svn.freebsd.org/changeset/base/237457 Log: MFC r237119: Do not remount ZFS dataset if changing canmount property to "on" and dataset is already mounted. PR: 167905 Submitted by: Bryan Drewery <bryan@shatow.net> Modified: stable/8/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Directory Properties: stable/8/cddl/contrib/opensolaris/ (props changed) Modified: stable/8/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c ============================================================================== --- stable/8/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Fri Jun 22 20:38:00 2012 (r237456) +++ stable/8/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Fri Jun 22 20:38:08 2012 (r237457) @@ -1485,11 +1485,13 @@ zfs_prop_set(zfs_handle_t *zhp, const ch /* * If the dataset's canmount property is being set to noauto, + * or being set to on and the dataset is already mounted, * then we want to prevent unmounting & remounting it. */ do_prefix = !((prop == ZFS_PROP_CANMOUNT) && (zprop_string_to_index(prop, propval, &idx, - ZFS_TYPE_DATASET) == 0) && (idx == ZFS_CANMOUNT_NOAUTO)); + ZFS_TYPE_DATASET) == 0) && (idx == ZFS_CANMOUNT_NOAUTO || + (idx == ZFS_CANMOUNT_ON && zfs_is_mounted(zhp, NULL)))); if (do_prefix && (ret = changelist_prefix(cl)) != 0) goto error; _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
State Changed From-To: open->closed Committed. Thanks!
Author: mm Date: Thu Jul 12 06:29:54 2012 New Revision: 238391 URL: http://svn.freebsd.org/changeset/base/238391 Log: Change behavior introduced in r237119 to vendor solution References: https://www.illumos.org/issues/2883 PR: 167905 Obtained from: illumos (issue #2883) MFC after: 2 weeks Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c ============================================================================== --- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Thu Jul 12 04:23:11 2012 (r238390) +++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Thu Jul 12 06:29:54 2012 (r238391) @@ -1484,14 +1484,17 @@ zfs_prop_set(zfs_handle_t *zhp, const ch } /* - * If the dataset's canmount property is being set to noauto, - * or being set to on and the dataset is already mounted, - * then we want to prevent unmounting & remounting it. - */ - do_prefix = !((prop == ZFS_PROP_CANMOUNT) && - (zprop_string_to_index(prop, propval, &idx, - ZFS_TYPE_DATASET) == 0) && (idx == ZFS_CANMOUNT_NOAUTO || - (idx == ZFS_CANMOUNT_ON && zfs_is_mounted(zhp, NULL)))); + * We don't want to unmount & remount the dataset when changing + * its canmount property to 'on' or 'noauto'. We only use + * the changelist logic to unmount when setting canmount=off. + */ + if (prop == ZFS_PROP_CANMOUNT) { + uint64_t idx; + int err = zprop_string_to_index(prop, propval, &idx, + ZFS_TYPE_DATASET); + if (err == 0 && idx != ZFS_CANMOUNT_OFF) + do_prefix = B_FALSE; + } if (do_prefix && (ret = changelist_prefix(cl)) != 0) goto error; _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"