From b068f8622bb4798d90428348e5d862cbd9efab52 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Wed, 11 Oct 2017 16:23:52 -0700 Subject: [PATCH] Remove vn_rename and vn_remove dependency The only place vn_rename and vn_remove are used is when writing out an updated pool configuration file. By truncating the file instead of renaming and removing it we can avoid having to implement these interfaces entirely. Functionally an empty cache file is treated the same as a missing cache file. This is particularly advantageous because the Linux kernel has never provided a way to reliably implement vn_rename and vn_remove. The cachefile_004_pos.ksh test case was updated to understand that an empty cache file is the same as a missing one. And the documentation was updated accordingly. Signed-off-by: Brian Behlendorf Requires-spl: refs/pull/661/head --- man/man8/zpool.8 | 2 +- module/zfs/spa_config.c | 35 ++++++++++++++----- .../cachefile/cachefile_004_pos.ksh | 6 ++-- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/man/man8/zpool.8 b/man/man8/zpool.8 index 85a0cc5f032f..54ba68698bd6 100644 --- a/man/man8/zpool.8 +++ b/man/man8/zpool.8 @@ -654,7 +654,7 @@ Because the kernel destroys and recreates this file when pools are added and removed, care should be taken when attempting to access this file. When the last pool using a .Sy cachefile -is exported or destroyed, the file is removed. +is exported or destroyed, the file will be empty. .It Sy comment Ns = Ns Ar text A text string consisting of printable ASCII characters that will be stored such that it is available even if the pool becomes faulted. diff --git a/module/zfs/spa_config.c b/module/zfs/spa_config.c index 8459e7362813..a49452ca6db2 100644 --- a/module/zfs/spa_config.c +++ b/module/zfs/spa_config.c @@ -147,6 +147,26 @@ spa_config_load(void) kobj_close_file(file); } +static int +spa_config_remove(spa_config_dirent_t *dp) +{ +#if defined(__linux__) && defined(_KERNEL) + int error, flags = FWRITE | FTRUNC; + uio_seg_t seg = UIO_SYSSPACE; + vnode_t *vp; + + error = vn_open(dp->scd_path, seg, flags, 0644, &vp, 0, 0); + if (error == 0) { + (void) VOP_FSYNC(vp, FSYNC, kcred, NULL); + (void) VOP_CLOSE(vp, 0, 1, 0, kcred, NULL); + } + + return (error); +#else + return (vn_remove(dp->scd_path, UIO_SYSSPACE, RMFILE)); +#endif +} + static int spa_config_write(spa_config_dirent_t *dp, nvlist_t *nvl) { @@ -161,12 +181,10 @@ spa_config_write(spa_config_dirent_t *dp, nvlist_t *nvl) * If the nvlist is empty (NULL), then remove the old cachefile. */ if (nvl == NULL) { - err = vn_remove(dp->scd_path, UIO_SYSSPACE, RMFILE); - /* - * Don't report an error when the cache file is already removed - */ + err = spa_config_remove(dp); if (err == ENOENT) err = 0; + return (err); } @@ -179,9 +197,9 @@ spa_config_write(spa_config_dirent_t *dp, nvlist_t *nvl) #if defined(__linux__) && defined(_KERNEL) /* * Write the configuration to disk. Due to the complexity involved - * in performing a rename from within the kernel the file is truncated - * and overwritten in place. In the event of an error the file is - * unlinked to make sure we always have a consistent view of the data. + * in performing a rename and remove from within the kernel the file + * is instead truncated and overwritten in place. This way we always + * always have a consistent view of the data or a zero length file. */ err = vn_open(dp->scd_path, UIO_SYSSPACE, oflags, 0644, &vp, 0, 0); if (err == 0) { @@ -191,9 +209,8 @@ spa_config_write(spa_config_dirent_t *dp, nvlist_t *nvl) err = VOP_FSYNC(vp, FSYNC, kcred, NULL); (void) VOP_CLOSE(vp, oflags, 1, 0, kcred, NULL); - if (err) - (void) vn_remove(dp->scd_path, UIO_SYSSPACE, RMFILE); + (void) spa_config_remove(dp); } #else /* diff --git a/tests/zfs-tests/tests/functional/cachefile/cachefile_004_pos.ksh b/tests/zfs-tests/tests/functional/cachefile/cachefile_004_pos.ksh index ae54a9365f54..e0b81e166279 100755 --- a/tests/zfs-tests/tests/functional/cachefile/cachefile_004_pos.ksh +++ b/tests/zfs-tests/tests/functional/cachefile/cachefile_004_pos.ksh @@ -98,13 +98,13 @@ log_must zpool set cachefile=$CPATH2 $TESTPOOL1 log_must pool_in_cache $TESTPOOL1 $CPATH2 log_must zpool set cachefile=$CPATH2 $TESTPOOL2 log_must pool_in_cache $TESTPOOL2 $CPATH2 -if [[ -f $CPATH1 ]]; then +if [[ -s $CPATH1 ]]; then log_fail "Verify set when cachefile is set on pool." fi log_must zpool export $TESTPOOL1 log_must zpool export $TESTPOOL2 -if [[ -f $CPATH2 ]]; then +if [[ -s $CPATH2 ]]; then log_fail "Verify export when cachefile is set on pool." fi @@ -117,7 +117,7 @@ log_must pool_in_cache $TESTPOOL2 $CPATH2 log_must zpool destroy $TESTPOOL1 log_must zpool destroy $TESTPOOL2 -if [[ -f $CPATH2 ]]; then +if [[ -s $CPATH2 ]]; then log_fail "Verify destroy when cachefile is set on pool." fi