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.

The zfs-import-* systemd service files were not updated to use
ConditionFileNotEmpty in place of ConditionPathExists.  This
means that after exporting all pools and rebooting new pools
will not the scanned for on the next boot.  This small change
should not impact normal usage since pools are not exported
as part of a normal shutdown.

Documentation was updated accordingly.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Arkadiusz Bubała <arkadiusz.bubala@open-e.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes zfsonlinux/spl#648
Closes #6753

diff --git a/man/man8/zpool.8 b/man/man8/zpool.8
index 328ba3dce6a9cc52352b88875f94a8f737c1f73e..22579101a54322d71cb10d8e74e7f56a00c42f43 100644 (file)
--- a/man/man8/zpool.8
+++ b/man/man8/zpool.8
@@ -655,7 +655,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 5b792b868455b7eda50d99b912c57b4a7f5e7307..fea239014db44a3f53e146e56f8f50840deb3345 100644 (file)
--- a/module/zfs/spa_config.c
+++ b/module/zfs/spa_config.c
@@ -147,6 +147,26 @@ out:
        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,7 +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);
+               err = spa_config_remove(dp);
+               if (err == ENOENT)
+                       err = 0;
+
                return (err);
        }
 
@@ -174,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
+        * 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) {
@@ -186,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 ae54a9365f54668ba98841c91b42cebe3ded1d58..e0b81e166279fdb1f661a698e476dc8b1304a331 100755 (executable)
--- 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