kern/167905: [zfs] zfs set canmount=on UNMOUNTS dataset

[Bryan Drewery]

The following reply was made to PR kern/167905; it has been noted by GNATS.

From: Bryan Drewery <bryan at shatow.net>
To: bug-followup at FreeBSD.org, vermaden at interia.pl
Cc: Pawel Jakub Dawidek <pjd at FreeBSD.org>, mm at freebsd.org
Subject: Re: kern/167905: [zfs] zfs set canmount=on UNMOUNTS dataset
Date: Tue, 12 Jun 2012 00:35:58 -0500

 This is a multi-part message in MIME format.
 --------------040108060500020307070404
 Content-Type: text/plain; charset=ISO-8859-1
 Content-Transfer-Encoding: 7bit
 
 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
 
 --------------040108060500020307070404
 Content-Type: text/plain; charset=windows-1252;
  name="patch-zfs-dataset-canmount-on.txt"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
  filename="patch-zfs-dataset-canmount-on.txt"
 
 --- 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
 @@ -1467,11 +1467,12 @@
  
  	/*
  	 * If the dataset's canmount property is being set to noauto,
 +	 * or to on and 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;
 
 --------------040108060500020307070404
 Content-Type: text/plain; charset=windows-1252;
  name="test-zsh.sh"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
  filename="test-zsh.sh"
 
 #! /bin/sh
 set -e
 
 cleanup() {
 	test -z "${PASS}" && echo "FAILED"
 	cd /
 	zpool destroy zpool
 	mdconfig -d -u 0
 	mdconfig -d -u 1
 }
 
 run() {
 	echo "$@"
 	"$@"
 	return $?
 }
 
 trap cleanup SIGINT SIGTERM SIGKILL EXIT
 
 
 rm -rf /tmp/test
 mdconfig -a -t swap -s 128M -u 0
 mdconfig -a -t swap -s 128M -u 1
 zpool create zpool mirror md0 md1
 zfs create -o mountpoint=/tmp/test -o canmount=on zpool/test 
 cd /tmp/test
 
 # This fails even though the property is not changing and dataset is already mounted
 run zfs set canmount=on zpool/test
 # Currently works
 run zfs set canmount=noauto zpool/test
 # Expect this to try to unmount and fail, may warrant more discussion
 #run zfs set canmount=off zpool/test
 # This fails even though the property is not changing and dataset is already mounted
 run zfs set canmount=on zpool/test
 
 echo "PASS"
 PASS=1
 
 --------------040108060500020307070404--