Archive-Team
/
zfs
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 103 Wiki Activity

Update the behavior of mountpoint property 

There are some inconsistencies in the handling of mountpoint
property. This commit updates the behavior and makes it
consistent.

If mountpoint property is set when dataset is unmounted, this
would update the mountpoint property. The mountpoint could be
valid or invalid in this case. Setting the mountpoint property
would result in success in this case. Dataset would still be
unmounted here.

On the other hand, if dataset is mounted and mountpoint
property is updated to something invalid where mount cannot be
successful, for example, setting the mountpoint inside a readonly
directory. This would unmount the dataset, set the mountpoint
property to requested value and tries to mount the dataset. The
mount operation returns error and this error is treated as
overall failure of setting the property while the property is
actually set.

To make the behavior consistent in case dataset is mounted or
unmounted, we should try to mount the dataset whenever mountpoint
property is updated. This would result in mounting the datasets
if canmount property is set to on, regardless if the dataset was
previously unmounted.

The failure in mount operation while setting the mountpoint
property should not be treated as failure, since the property is
actually set now to user requested value.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Ameer Hamza <ahamza@ixsystems.com>
Signed-off-by: Umer Saleem <usaleem@ixsystems.com>
Closes #15240
This commit is contained in:
Umer Saleem 2023-09-05 13:27:53 +05:00 committed by Brian Behlendorf
parent 7228ba1114
commit bbac1d2977
7 changed files with 34 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
cmd/zfs/zfs_main.c
View File

@ -4207,8 +4207,9 @@ static int
set_callback(zfs_handle_t *zhp, void *data) set_callback(zfs_handle_t *zhp, void *data)
{ {
nvlist_t *props = data; nvlist_t *props = data;
int ret = zfs_prop_set_list(zhp, props);
if (zfs_prop_set_list(zhp, props) != 0) { if (ret != 0 || libzfs_errno(g_zfs) != EZFS_SUCCESS) {
switch (libzfs_errno(g_zfs)) { switch (libzfs_errno(g_zfs)) {
case EZFS_MOUNTFAILED: case EZFS_MOUNTFAILED:
(void) fprintf(stderr, gettext("property may be set " (void) fprintf(stderr, gettext("property may be set "
@ -4217,11 +4218,11 @@ set_callback(zfs_handle_t *zhp, void *data)
case EZFS_SHARENFSFAILED: case EZFS_SHARENFSFAILED:
(void) fprintf(stderr, gettext("property may be set " (void) fprintf(stderr, gettext("property may be set "
"but unable to reshare filesystem\n")); "but unable to reshare filesystem\n"));
ret = 1;
break; break;
} }
return (1);
} }
return (0); return (ret);
} }
static int static int

8
lib/libzfs/libzfs_changelist.c
View File

@ -244,13 +244,13 @@ changelist_postfix(prop_changelist_t *clp)
zfs_is_mounted(cn->cn_handle, NULL); zfs_is_mounted(cn->cn_handle, NULL);
if (!mounted && !needs_key && (cn->cn_mounted || if (!mounted && !needs_key && (cn->cn_mounted ||
((sharenfs || sharesmb || clp->cl_waslegacy) && (((clp->cl_prop == ZFS_PROP_MOUNTPOINT &&
clp->cl_prop == clp->cl_realprop) ||
sharenfs || sharesmb || clp->cl_waslegacy) &&
(zfs_prop_get_int(cn->cn_handle, (zfs_prop_get_int(cn->cn_handle,
ZFS_PROP_CANMOUNT) == ZFS_CANMOUNT_ON)))) { ZFS_PROP_CANMOUNT) == ZFS_CANMOUNT_ON)))) {
if (zfs_mount(cn->cn_handle, NULL, 0) != 0) if (zfs_mount(cn->cn_handle, NULL, 0) == 0)
errors++;
else
mounted = TRUE; mounted = TRUE;
} }

2
tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_006_pos.ksh
View File

@ -94,7 +94,7 @@ while (( depth < MAXDEPTH )); do
done done
log_must zfs set mountpoint=$mtpt $TESTPOOL/$TESTFS log_must zfs set mountpoint=$mtpt $TESTPOOL/$TESTFS
log_must zfs $mountcmd $TESTPOOL/$TESTFS log_must ismounted $TESTPOOL/$TESTFS
log_must zfs set overlay=off $TESTPOOL/$TESTFS log_must zfs set overlay=off $TESTPOOL/$TESTFS
if ! is_illumos; then if ! is_illumos; then

4
tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_008_pos.ksh
View File

@ -71,7 +71,7 @@ log_must mkfile 1M $testfile $testfile1
log_must zfs unmount $fs1 log_must zfs unmount $fs1
log_must zfs set mountpoint=$mntpnt $fs1 log_must zfs set mountpoint=$mntpnt $fs1
log_must zfs mount $fs1 log_must ismounted $fs1
log_must zfs unmount $fs1 log_must zfs unmount $fs1
log_must zfs mount -O $fs1 log_must zfs mount -O $fs1
@ -85,7 +85,7 @@ log_must ls $mntpnt/$TESTFILE1 $mntpnt/$TESTFILE2
# Verify $TESTFILE2 was created in $fs1, rather than $fs # Verify $TESTFILE2 was created in $fs1, rather than $fs
log_must zfs unmount $fs1 log_must zfs unmount $fs1
log_must zfs set mountpoint=$mntpnt1 $fs1 log_must zfs set mountpoint=$mntpnt1 $fs1
log_must zfs mount $fs1 log_must ismounted $fs1
log_must ls $testfile1 $mntpnt1/$TESTFILE2 log_must ls $testfile1 $mntpnt1/$TESTFILE2
# Verify $TESTFILE2 was not created in $fs, and $fs is accessible again. # Verify $TESTFILE2 was not created in $fs, and $fs is accessible again.

15
tests/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_012_pos.ksh
View File

@ -25,13 +25,12 @@
# STRATEGY: # STRATEGY:
# 1. Unmount the dataset # 1. Unmount the dataset
# 2. Create a new empty directory # 2. Create a new empty directory
# 3. Set the dataset's mountpoint # 3. Set the dataset's mountpoint, this should mount the dataset
# 4. Attempt to mount the dataset # 4. Verify the mount succeeds
# 5. Verify the mount succeeds # 5. Unmount the dataset
# 6. Unmount the dataset # 6. Create a file in the directory created in step 2
# 7. Create a file in the directory created in step 2 # 7. Attempt to mount the dataset
# 8. Attempt to mount the dataset # 8. Verify the mount succeeds
# 9. Verify the mount succeeds
# #
verify_runnable "both" verify_runnable "both"
@ -43,7 +42,7 @@ fs=$TESTPOOL/$TESTFS
log_must zfs umount $fs log_must zfs umount $fs
log_must mkdir -p $TESTDIR log_must mkdir -p $TESTDIR
log_must zfs set mountpoint=$TESTDIR $fs log_must zfs set mountpoint=$TESTDIR $fs
log_must zfs mount $fs log_must ismounted $fs
log_must zfs umount $fs log_must zfs umount $fs
log_must touch $TESTDIR/testfile.$$ log_must touch $TESTDIR/testfile.$$
log_must zfs mount $fs log_must zfs mount $fs

10
tests/zfs-tests/tests/functional/cli_root/zfs_set/mountpoint_002_pos.ksh
View File

@ -35,7 +35,9 @@
# #
# DESCRIPTION: # DESCRIPTION:
# If ZFS is currently managing the file system but it is currently unmounted, # If ZFS is currently managing the file system but it is currently unmounted,
# and the mountpoint property is changed, the file system remains unmounted. # and the mountpoint property is changed, the file system should be mounted
# if it is a valid mountpoint and canmount allows to mount, otherwise it
# should not be mounted.
# #
# STRATEGY: # STRATEGY:
# 1. Setup a pool and create fs, ctr within it. # 1. Setup a pool and create fs, ctr within it.
@ -62,7 +64,7 @@ function cleanup
} }
log_assert "Setting a valid mountpoint for an unmounted file system, \ log_assert "Setting a valid mountpoint for an unmounted file system, \
it remains unmounted." it gets mounted."
log_onexit cleanup log_onexit cleanup
old_fs_mpt=$(get_prop mountpoint $TESTPOOL/$TESTFS) old_fs_mpt=$(get_prop mountpoint $TESTPOOL/$TESTFS)
@ -83,7 +85,11 @@ while (( i < ${#dataset[@]} )); do
while (( j < ${#values[@]} )); do while (( j < ${#values[@]} )); do
set_n_check_prop "${values[j]}" "mountpoint" \ set_n_check_prop "${values[j]}" "mountpoint" \
"${dataset[i]}" "${dataset[i]}"
if [ "${dataset[i]}" = "$TESTPOOL/$TESTFS" ]; then
log_must ismounted ${dataset[i]}
else
log_mustnot ismounted ${dataset[i]} log_mustnot ismounted ${dataset[i]}
fi
(( j += 1 )) (( j += 1 ))
done done
cleanup cleanup

10
tests/zfs-tests/tests/functional/cli_root/zfs_set/zfs_set_003_neg.ksh
View File

@ -33,7 +33,9 @@
# #
# DESCRIPTION: # DESCRIPTION:
# 'zfs set mountpoint/sharenfs' should fail when the mountpoint is invalid # 'zfs set mountpoint/sharenfs' should set the property when mountpoint
# is invalid. Setting the property should be successful, but dataset
# should not be mounted, as mountpoint is invalid.
# #
# STRATEGY: # STRATEGY:
# 1. Create invalid scenarios # 1. Create invalid scenarios
@ -62,10 +64,12 @@ longpath=$(gen_dataset_name 1030 "abcdefg")
log_must zfs create -o mountpoint=legacy $TESTPOOL/foo log_must zfs create -o mountpoint=legacy $TESTPOOL/foo
# Do the negative testing about "property may be set but unable to remount filesystem" # Do the negative testing about "property may be set but unable to remount filesystem"
log_mustnot eval "zfs set mountpoint=$badpath $TESTPOOL/foo >/dev/null 2>&1" set_n_check_prop "$badpath" "mountpoint" "$TESTPOOL/foo"
log_mustnot ismounted $TESTPOOL/foo
# Do the negative testing about "property may be set but unable to reshare filesystem" # Do the negative testing about "property may be set but unable to reshare filesystem"
log_mustnot eval "zfs set sharenfs=on $TESTPOOL/foo >/dev/null 2>&1" set_n_check_prop "on" "sharenfs" "$TESTPOOL/foo"
log_mustnot ismounted $TESTPOOL/foo
# Do the negative testing about "sharenfs property can not be set to null" # Do the negative testing about "sharenfs property can not be set to null"
log_mustnot eval "zfs set sharenfs= $TESTPOOL/foo >/dev/null 2>&1" log_mustnot eval "zfs set sharenfs= $TESTPOOL/foo >/dev/null 2>&1"