Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zvol: allow rename of in use ZVOL dataset #8371

Merged
merged 1 commit into from
Feb 22, 2019
Merged

zvol: allow rename of in use ZVOL dataset #8371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions module/zfs/zvol.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2252,12 +2252,6 @@ zvol_rename_minors_impl(const char *oldname, const char *newname)

mutex_enter(&zv->zv_state_lock);

/* If in use, leave alone */
if (zv->zv_open_count > 0) {
mutex_exit(&zv->zv_state_lock);
continue;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the comment in #4344 (comment) this logic was added to make sure the zv_name wasn't updated while being accessed by zvol_ioctl(). This was a concern because it was accessed outside the zv_state_lock. Since this is no longer the case, this should be fine. This does mean the udev symlink name may change out from underneath us, which is arguably what we want to happen.

Function zvol_rename_minors() does not check the reference count before
modifying the zv_name, which is not good since zvol_ioctl(), for instance,
accesses zv_name without holding zvol_state_lock (but with zv_open_count
being positive).


if (strcmp(zv->zv_name, oldname) == 0) {
zvol_rename_minor(zv, newname);
} else if (strncmp(zv->zv_name, oldname, oldnamelen) == 0 &&
Expand Down
4 changes: 2 additions & 2 deletions tests/runfiles/linux.run
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,8 +887,8 @@ tags = ['functional', 'zvol', 'zvol_cli']
[tests/functional/zvol/zvol_misc]
tests = ['zvol_misc_001_neg', 'zvol_misc_002_pos', 'zvol_misc_003_neg',
'zvol_misc_004_pos', 'zvol_misc_005_neg', 'zvol_misc_006_pos',
'zvol_misc_hierarchy', 'zvol_misc_snapdev', 'zvol_misc_volmode',
'zvol_misc_zil']
'zvol_misc_hierarchy', 'zvol_misc_rename_inuse', 'zvol_misc_snapdev',
'zvol_misc_volmode', 'zvol_misc_zil']
tags = ['functional', 'zvol', 'zvol_misc']

[tests/functional/zvol/zvol_swap]
Expand Down
1 change: 1 addition & 0 deletions tests/zfs-tests/tests/functional/zvol/zvol_misc/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ dist_pkgdata_SCRIPTS = \
zvol_misc_005_neg.ksh \
zvol_misc_006_pos.ksh \
zvol_misc_hierarchy.ksh \
zvol_misc_rename_inuse.ksh \
zvol_misc_snapdev.ksh \
zvol_misc_volmode.ksh \
zvol_misc_zil.ksh
Expand Down
2 changes: 2 additions & 0 deletions tests/zfs-tests/tests/functional/zvol/zvol_misc/cleanup.ksh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,6 @@ verify_runnable "global"

default_zvol_cleanup

[[ -d $TESTDIR ]] && rm -rf $TESTDIR

log_pass
2 changes: 2 additions & 0 deletions tests/zfs-tests/tests/functional/zvol/zvol_misc/setup.ksh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,6 @@ verify_runnable "global"

default_zvol_setup $DISK $VOLSIZE

log_must mkdir $TESTDIR

log_pass
2 changes: 0 additions & 2 deletions tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_002_pos.ksh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ function cleanup
ismounted $TESTDIR $NEWFS_DEFAULT_FS
(( $? == 0 )) && log_must umount $TESTDIR

[[ -e $TESTDIR ]] && rm -rf $TESTDIR
zfs set volsize=$volsize $TESTPOOL/$TESTVOL
}

Expand All @@ -70,7 +69,6 @@ log_must zfs set volsize=128m $TESTPOOL/$TESTVOL
echo "y" | newfs -v ${ZVOL_RDEVDIR}/$TESTPOOL/$TESTVOL >/dev/null 2>&1
(( $? != 0 )) && log_fail "Unable to newfs(1M) $TESTPOOL/$TESTVOL"

log_must mkdir $TESTDIR
log_must mount ${ZVOL_DEVDIR}/$TESTPOOL/$TESTVOL $TESTDIR

typeset -i fn=0
Expand Down
71 changes: 71 additions & 0 deletions tests/zfs-tests/tests/functional/zvol/zvol_misc/zvol_misc_rename_inuse.ksh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
#!/bin/ksh -p
#
# 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 of the CDDL is also available via the Internet at
# http://www.illumos.org/license/CDDL.
#

#
# Copyright 2019, loli10K <[email protected]>. All rights reserved.
#

. $STF_SUITE/include/libtest.shlib
. $STF_SUITE/tests/functional/zvol/zvol_common.shlib
. $STF_SUITE/tests/functional/zvol/zvol_misc/zvol_misc_common.kshlib

#
# DESCRIPTION:
# Verify 'zfs rename' works on a ZVOL already in use as block device
#
# STRATEGY:
# 1. Create a ZVOL
# 2. Create a filesystem on the ZVOL device and mount it
# 3. Rename the ZVOL dataset
# 4. Receive a send stream with the same name as the old ZVOL dataset and verify
# we don't trigger any issue like the one reported in #6263
#

verify_runnable "global"

function cleanup
{
log_must umount "$MNTPFS"
log_must rmdir "$MNTPFS"
for ds in "$SENDFS" "$ZVOL" "$ZVOL-renamed"; do
destroy_dataset "$ds" '-rf'
done
udev_wait
}

log_assert "Verify 'zfs rename' works on a ZVOL already in use as block device"
log_onexit cleanup

ZVOL="$TESTPOOL/vol.$$"
ZDEV="$ZVOL_DEVDIR/$ZVOL"
MNTPFS="$TESTDIR/zvol_inuse_rename"
SENDFS="$TESTPOOL/sendfs.$$"

# 1. Create a ZVOL
log_must zfs create -V $VOLSIZE "$ZVOL"

# 2. Create a filesystem on the ZVOL device and mount it
udev_wait
log_must eval "echo y | newfs $ZDEV >/dev/null 2>&1"
log_must mkdir "$MNTPFS"
log_must mount "$ZDEV" "$MNTPFS"

# 3. Rename the ZVOL dataset
log_must zfs rename "$ZVOL" "$ZVOL-renamed"

# 4. Receive a send stream with the same name as the old ZVOL dataset and verify
# we don't trigger any issue like the one reported in #6263
log_must zfs create "$SENDFS"
log_must zfs snap "$SENDFS@snap"
log_must eval "zfs send $SENDFS@snap | zfs recv $ZVOL"

log_pass "Renaming in use ZVOL works successfully"