Bug 163668 - [patch] fstab[5] 'failok' option has no effect on missing hard drive
Summary: [patch] fstab[5] 'failok' option has no effect on missing hard drive
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: 8.2-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Jaakko Heinonen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-28 13:40 UTC by Oleg Baranov
Modified: 2021-11-28 23:37 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Oleg Baranov 2011-12-28 13:40:10 UTC
System drops to Single mode on boot in case special device is missing even corresponding filesystem is marked as 'failok' in /etc/fstab.

Thus server cannot boot becomes remotely unaccessible even if some unimportant drive is missing (or got corrupted and undetectable).

fstab(5) documentation mentions:
"If the option ``failok'' is specified, the system will ignore any error
     which happens during the mount of that filesystem, which would otherwise
     cause the system to drop into single user mode.
"
I fill this behavior as a bug suggesting a server must withstand minor failures as long as it can and definitely should not get rendered totally useles in case of some secondary (non-root in general) drive/filesystem fault. It should be up to server admin to define those 'secondary' filesystems. 'failok' option has been designed for this particular purpose as far as I understand.


Part of boot log from a faulty system:
-----------------------------
Starting file system checks:
/dev/ada0p2: FILE SYSTEM CLEAN; SKIPPING CHECKS
/dev/ada0p2: clean, 50085826 free (6626 frags, 6259900 blocks, 0.0% fragmentatio
n)
Mounting local file systems:mount: /dev/ufs/botva : No such file or directory
Comment 1 Oleg Baranov 2011-12-28 14:02:59 UTC
Don't know why the 'How-To-Repeat' section has disappeared during bug 
submission.

How-To-Repeat:
100% repeatable. Have a machine (possibly virtual) with two disk 
devices. Mark second one (non-root) filesystem as 'failok' and shutddown.

# cat /etc/fstab
# Device    Mountpoint    FStype    Options    Dump    Pass#
/dev/ada0p2    /        ufs    rw    1    1
/dev/ufs/botva    /mnt        ufs    rw,failok    0    0


Unplug the second drive and try to boot.
Comment 2 Jaakko Heinonen freebsd_committer 2012-01-02 15:45:41 UTC
Hi,

On 2011-12-28, Oleg Baranov wrote:
> System drops to Single mode on boot in case special device is missing
> even corresponding filesystem is marked as 'failok' in /etc/fstab.

This is a bug in mount(8). The mountfs() function calls several
functions which may cause mount(8) to exit with error status. This
breaks the "failok" functionality.

I tried to remove most exit points in the following patch. Changing
checkpath() isn't trivial because also file system specific mount
commands use the function.

	http://people.freebsd.org/~jh/patches/mount-failok.diff

Please report back if you are able to test the patch.

-- 
Jaakko
Comment 3 Oleg Baranov 2012-01-02 17:06:50 UTC
Hi,


On 01/02/2012 07:45 PM, Jaakko Heinonen wrote:
>
> 	http://people.freebsd.org/~jh/patches/mount-failok.diff
>
> Please report back if you are able to test the patch.
>



The patch works fine for me. Tested it on the same virtual instance that 
I filled this bug-report with.

Thank you!
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2012-01-02 19:16:55 UTC
State Changed
From-To: open->analyzed

Submitted patch seems to fix the problem.
Comment 5 dfilter service freebsd_committer 2012-01-16 19:34:35 UTC
Author: jh
Date: Mon Jan 16 19:34:21 2012
New Revision: 230226
URL: http://svn.freebsd.org/changeset/base/230226

Log:
  Change checkpath() to not exit on error. This is a prerequisite for
  fixing the mount(8) "failok" option.
  
  PR:		163668
  Reviewed by:	Garrett Cooper, delphij (previous version)

Modified:
  head/sbin/mount/getmntopts.c
  head/sbin/mount/mntopts.h
  head/sbin/mount/mount.c
  head/sbin/mount/mount_fs.c
  head/sbin/mount_cd9660/mount_cd9660.c
  head/sbin/mount_ext2fs/mount_ext2fs.c
  head/sbin/mount_msdosfs/mount_msdosfs.c
  head/sbin/mount_nfs/mount_nfs.c
  head/sbin/mount_ntfs/mount_ntfs.c
  head/sbin/mount_nullfs/mount_nullfs.c
  head/sbin/mount_reiserfs/mount_reiserfs.c
  head/sbin/mount_std/mount_std.c
  head/sbin/mount_udf/mount_udf.c
  head/sbin/mount_unionfs/mount_unionfs.c
  head/usr.sbin/mount_portalfs/mount_portalfs.c

Modified: head/sbin/mount/getmntopts.c
==============================================================================
--- head/sbin/mount/getmntopts.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount/getmntopts.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -124,16 +124,20 @@ rmslashes(char *rrpin, char *rrpout)
 		*rrpout = '\0';
 }
 
-void
+int
 checkpath(const char *path, char *resolved)
 {
 	struct stat sb;
 
 	if (realpath(path, resolved) != NULL && stat(resolved, &sb) == 0) {
-		if (!S_ISDIR(sb.st_mode))
-			errx(EX_USAGE, "%s: not a directory", resolved);
+		if (!S_ISDIR(sb.st_mode)) {
+			errno = ENOTDIR;
+			return (1);
+		}
 	} else
-		errx(EX_USAGE, "%s: %s", resolved, strerror(errno));
+		return (1);
+
+	return (0);
 }
 
 void

Modified: head/sbin/mount/mntopts.h
==============================================================================
--- head/sbin/mount/mntopts.h	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount/mntopts.h	Mon Jan 16 19:34:21 2012	(r230226)
@@ -93,7 +93,7 @@ struct mntopt {
 
 void getmntopts(const char *, const struct mntopt *, int *, int *);
 void rmslashes(char *, char *);
-void checkpath(const char *, char resolved_path[]);
+int checkpath(const char *, char resolved_path[]);
 extern int getmnt_silent;
 void build_iovec(struct iovec **iov, int *iovlen, const char *name, void *val, size_t len);
 void build_iovec_argf(struct iovec **iov, int *iovlen, const char *name, const char *fmt, ...);

Modified: head/sbin/mount/mount.c
==============================================================================
--- head/sbin/mount/mount.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount/mount.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -539,7 +539,10 @@ mountfs(const char *vfstype, const char 
 	static struct cpa mnt_argv;
 
 	/* resolve the mountpoint with realpath(3) */
-	(void)checkpath(name, mntpath);
+	if (checkpath(name, mntpath) != 0) {
+		warn("%s", mntpath);
+		return (1);
+	}
 	name = mntpath;
 
 	if (mntopts == NULL)

Modified: head/sbin/mount/mount_fs.c
==============================================================================
--- head/sbin/mount/mount_fs.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount/mount_fs.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -118,7 +118,10 @@ mount_fs(const char *vfstype, int argc, 
 	dev = argv[0];
 	dir = argv[1];
 
-	(void)checkpath(dir, mntpath);
+	if (checkpath(dir, mntpath) != 0) {
+		warn("%s", mntpath);
+		return (1);
+	}
 	(void)rmslashes(dev, dev);
 
 	build_iovec(&iov, &iovlen, "fstype", fstype, (size_t)-1);

Modified: head/sbin/mount_cd9660/mount_cd9660.c
==============================================================================
--- head/sbin/mount_cd9660/mount_cd9660.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount_cd9660/mount_cd9660.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -149,7 +149,8 @@ main(int argc, char **argv)
 	 * Resolve the mountpoint with realpath(3) and remove unnecessary
 	 * slashes from the devicename if there are any.
 	 */
-	(void)checkpath(dir, mntpath);
+	if (checkpath(dir, mntpath) != 0)
+		err(1, "%s", mntpath);
 	(void)rmslashes(dev, dev);
 
 	if (ssector == -1) {

Modified: head/sbin/mount_ext2fs/mount_ext2fs.c
==============================================================================
--- head/sbin/mount_ext2fs/mount_ext2fs.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount_ext2fs/mount_ext2fs.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -103,7 +103,8 @@ main(int argc, char *argv[])
 	 * Resolve the mountpoint with realpath(3) and remove unnecessary
 	 * slashes from the devicename if there are any.
 	 */
-	(void)checkpath(fs_name, mntpath);
+	if (checkpath(fs_name, mntpath) != 0)
+		err(EX_USAGE, "%s", mntpath);
 	(void)rmslashes(fspec, fspec);
 
 	build_iovec(&iov, &iovlen, "fstype", fstype, strlen(fstype) + 1);

Modified: head/sbin/mount_msdosfs/mount_msdosfs.c
==============================================================================
--- head/sbin/mount_msdosfs/mount_msdosfs.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount_msdosfs/mount_msdosfs.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -193,7 +193,8 @@ main(int argc, char **argv)
 	 * Resolve the mountpoint with realpath(3) and remove unnecessary
 	 * slashes from the devicename if there are any.
 	 */
-	(void)checkpath(dir, mntpath);
+	if (checkpath(dir, mntpath) != 0)
+		err(EX_USAGE, "%s", mntpath);
 	(void)rmslashes(dev, dev);
 
 	if (!set_gid || !set_uid || !set_mask) {

Modified: head/sbin/mount_nfs/mount_nfs.c
==============================================================================
--- head/sbin/mount_nfs/mount_nfs.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount_nfs/mount_nfs.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -411,7 +411,8 @@ main(int argc, char *argv[])
 		exit(1);
 
 	/* resolve the mountpoint with realpath(3) */
-	(void)checkpath(name, mntpath);
+	if (checkpath(name, mntpath) != 0)
+		err(1, "%s", mntpath);
 
 	build_iovec(&iov, &iovlen, "fstype", fstype, (size_t)-1);
 	build_iovec(&iov, &iovlen, "fspath", mntpath, (size_t)-1);

Modified: head/sbin/mount_ntfs/mount_ntfs.c
==============================================================================
--- head/sbin/mount_ntfs/mount_ntfs.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount_ntfs/mount_ntfs.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -163,7 +163,8 @@ main(int argc, char *argv[])
 	 * Resolve the mountpoint with realpath(3) and remove unnecessary 
 	 * slashes from the devicename if there are any.
 	 */
-	(void)checkpath(dir, mntpath);
+	if (checkpath(dir, mntpath) != 0)
+		err(EX_USAGE, "%s", mntpath);
 	(void)rmslashes(dev, dev);
 
 	args.fspec = dev;

Modified: head/sbin/mount_nullfs/mount_nullfs.c
==============================================================================
--- head/sbin/mount_nullfs/mount_nullfs.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount_nullfs/mount_nullfs.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -90,8 +90,10 @@ main(int argc, char *argv[])
 		usage();
 
 	/* resolve target and source with realpath(3) */
-	(void)checkpath(argv[0], target);
-	(void)checkpath(argv[1], source);
+	if (checkpath(argv[0], target) != 0)
+		err(EX_USAGE, "%s", target);
+	if (checkpath(argv[1], source) != 0)
+		err(EX_USAGE, "%s", source);
 
 	if (subdir(target, source) || subdir(source, target))
 		errx(EX_USAGE, "%s (%s) and %s are not distinct paths",

Modified: head/sbin/mount_reiserfs/mount_reiserfs.c
==============================================================================
--- head/sbin/mount_reiserfs/mount_reiserfs.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount_reiserfs/mount_reiserfs.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -78,7 +78,8 @@ main(int argc, char *argv[])
 	 * Resolve the mountpoint with realpath(3) and remove unnecessary
 	 * slashes from the devicename if there are any.
 	 */
-	(void)checkpath(dir, mntpath);
+	if (checkpath(dir, mntpath) != 0)
+		err(EX_USAGE, "%s", mntpath);
 	(void)rmslashes(dev, dev);
 
 	/* Read-only support for now */

Modified: head/sbin/mount_std/mount_std.c
==============================================================================
--- head/sbin/mount_std/mount_std.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount_std/mount_std.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -112,7 +112,8 @@ main(int argc, char *argv[])
 		usage();
 
 	/* resolve the mountpoint with realpath(3) */
-	(void)checkpath(argv[1], mntpath);
+	if (checkpath(argv[1], mntpath) != 0)
+		err(EX_USAGE, "%s", mntpath);
 
 	iov[0].iov_base = "fstype";
 	iov[0].iov_len = sizeof("fstype");

Modified: head/sbin/mount_udf/mount_udf.c
==============================================================================
--- head/sbin/mount_udf/mount_udf.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount_udf/mount_udf.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -111,7 +111,8 @@ main(int argc, char **argv)
 	 * Resolve the mountpoint with realpath(3) and remove unnecessary
 	 * slashes from the devicename if there are any.
 	 */
-	(void)checkpath(dir, mntpath);
+	if (checkpath(dir, mntpath) != 0)
+		err(EX_USAGE, "%s", mntpath);
 	(void)rmslashes(dev, dev);
 
 	/*

Modified: head/sbin/mount_unionfs/mount_unionfs.c
==============================================================================
--- head/sbin/mount_unionfs/mount_unionfs.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/sbin/mount_unionfs/mount_unionfs.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -176,8 +176,10 @@ main(int argc, char *argv[])
 		usage();
 
 	/* resolve both target and source with realpath(3) */
-	(void)checkpath(argv[0], target);
-	(void)checkpath(argv[1], source);
+	if (checkpath(argv[0], target) != 0)
+		err(EX_USAGE, "%s", target);
+	if (checkpath(argv[1], source) != 0)
+		err(EX_USAGE, "%s", source);
 
 	if (subdir(target, source) || subdir(source, target))
 		errx(EX_USAGE, "%s (%s) and %s (%s) are not distinct paths",

Modified: head/usr.sbin/mount_portalfs/mount_portalfs.c
==============================================================================
--- head/usr.sbin/mount_portalfs/mount_portalfs.c	Mon Jan 16 18:19:53 2012	(r230225)
+++ head/usr.sbin/mount_portalfs/mount_portalfs.c	Mon Jan 16 19:34:21 2012	(r230226)
@@ -140,7 +140,8 @@ main(int argc, char *argv[])
 	}
 
 	/* resolve the mountpoint with realpath(3) */
-	(void)checkpath(argv[optind+1], mountpt);
+	if (checkpath(argv[optind+1], mountpt) != 0)
+		err(EX_USAGE, "%s", mountpt);
 
 	/*
 	 * Construct the listening socket
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 6 Jaakko Heinonen freebsd_committer 2012-01-19 17:24:19 UTC
Responsible Changed
From-To: freebsd-bugs->jh

Take.
Comment 7 dfilter service freebsd_committer 2012-01-20 10:06:42 UTC
Author: jh
Date: Fri Jan 20 10:06:28 2012
New Revision: 230373
URL: http://svn.freebsd.org/changeset/base/230373

Log:
  Change mount_fs() to not exit on error. The "failok" mount option
  requires that errors are passed to the caller.
  
  PR:		163668
  Reviewed by:	Garrett Cooper

Modified:
  head/sbin/mount/mount_fs.c

Modified: head/sbin/mount/mount_fs.c
==============================================================================
--- head/sbin/mount/mount_fs.c	Fri Jan 20 07:29:29 2012	(r230372)
+++ head/sbin/mount/mount_fs.c	Fri Jan 20 10:06:28 2012	(r230373)
@@ -82,7 +82,6 @@ mount_fs(const char *vfstype, int argc, 
 	char fstype[32];
 	char errmsg[255];
 	char *p, *val;
-	int ret;
 
 	strlcpy(fstype, vfstype, sizeof(fstype));
 	memset(errmsg, 0, sizeof(errmsg));
@@ -128,10 +127,10 @@ mount_fs(const char *vfstype, int argc, 
 	build_iovec(&iov, &iovlen, "fspath", mntpath, (size_t)-1);
 	build_iovec(&iov, &iovlen, "from", dev, (size_t)-1);
 	build_iovec(&iov, &iovlen, "errmsg", errmsg, sizeof(errmsg));
-	
-	ret = nmount(iov, iovlen, mntflags);
-	if (ret < 0)
-		err(1, "%s %s", dev, errmsg);
 
-	return (ret);
+	if (nmount(iov, iovlen, mntflags) == -1) {
+		warn("%s: %s", dev, errmsg);
+		return (1);
+	}
+	return (0);
 }
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 8 Jaakko Heinonen freebsd_committer 2012-01-20 10:15:47 UTC
State Changed
From-To: analyzed->patched

Fixed in head (r230226 and r230373).
Comment 9 dfilter service freebsd_committer 2012-02-13 18:27:12 UTC
Author: jh
Date: Mon Feb 13 18:26:58 2012
New Revision: 231591
URL: http://svn.freebsd.org/changeset/base/231591

Log:
  MFC r230373:
  
  Change mount_fs() to not exit on error. The "failok" mount option
  requires that errors are passed to the caller.
  
  PR:		163668

Modified:
  stable/9/sbin/mount/mount_fs.c
Directory Properties:
  stable/9/sbin/mount/   (props changed)

Modified: stable/9/sbin/mount/mount_fs.c
==============================================================================
--- stable/9/sbin/mount/mount_fs.c	Mon Feb 13 18:10:13 2012	(r231590)
+++ stable/9/sbin/mount/mount_fs.c	Mon Feb 13 18:26:58 2012	(r231591)
@@ -82,7 +82,6 @@ mount_fs(const char *vfstype, int argc, 
 	char fstype[32];
 	char errmsg[255];
 	char *p, *val;
-	int ret;
 
 	strlcpy(fstype, vfstype, sizeof(fstype));
 	memset(errmsg, 0, sizeof(errmsg));
@@ -125,10 +124,10 @@ mount_fs(const char *vfstype, int argc, 
 	build_iovec(&iov, &iovlen, "fspath", mntpath, (size_t)-1);
 	build_iovec(&iov, &iovlen, "from", dev, (size_t)-1);
 	build_iovec(&iov, &iovlen, "errmsg", errmsg, sizeof(errmsg));
-	
-	ret = nmount(iov, iovlen, mntflags);
-	if (ret < 0)
-		err(1, "%s %s", dev, errmsg);
 
-	return (ret);
+	if (nmount(iov, iovlen, mntflags) == -1) {
+		warn("%s: %s", dev, errmsg);
+		return (1);
+	}
+	return (0);
 }
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 10 dfilter service freebsd_committer 2012-06-11 19:02:39 UTC
Author: jh
Date: Mon Jun 11 18:02:30 2012
New Revision: 236901
URL: http://svn.freebsd.org/changeset/base/236901

Log:
  MFC r230373:
  
  Change mount_fs() to not exit on error. The "failok" mount option
  requires that errors are passed to the caller.
  
  PR:		163668

Modified:
  stable/8/sbin/mount/mount_fs.c
Directory Properties:
  stable/8/sbin/mount/   (props changed)

Modified: stable/8/sbin/mount/mount_fs.c
==============================================================================
--- stable/8/sbin/mount/mount_fs.c	Mon Jun 11 17:54:40 2012	(r236900)
+++ stable/8/sbin/mount/mount_fs.c	Mon Jun 11 18:02:30 2012	(r236901)
@@ -86,7 +86,6 @@ mount_fs(const char *vfstype, int argc, 
 	char fstype[32];
 	char errmsg[255];
 	char *p, *val;
-	int ret;
 
 	strlcpy(fstype, vfstype, sizeof(fstype));
 	memset(errmsg, 0, sizeof(errmsg));
@@ -129,10 +128,10 @@ mount_fs(const char *vfstype, int argc, 
 	build_iovec(&iov, &iovlen, "fspath", mntpath, (size_t)-1);
 	build_iovec(&iov, &iovlen, "from", dev, (size_t)-1);
 	build_iovec(&iov, &iovlen, "errmsg", errmsg, sizeof(errmsg));
-	
-	ret = nmount(iov, iovlen, mntflags);
-	if (ret < 0)
-		err(1, "%s %s", dev, errmsg);
 
-	return (ret);
+	if (nmount(iov, iovlen, mntflags) == -1) {
+		warn("%s: %s", dev, errmsg);
+		return (1);
+	}
+	return (0);
 }
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 11 Jaakko Heinonen freebsd_committer 2012-06-11 19:25:57 UTC
State Changed
From-To: patched->closed

Fixed in head, stable/9 and stable/8.