Fix for Bug 246201 - fsck: Does not honour /etc/fstab "failok" option

[mckusick]

Summary

When the fstab(5) option "failok" is configured for a filesystem which may, at times, be missing its device, boot still fails (drops into single-user mode).

This is because fsck also runs against fstab(5), and considers the missing device to be a failure condition.

This update to fsck(8) ignores failures from the check programs for fstab(5) entries with the "failok" attribute.

Test Plan

Make an entry in fstab(5) with a non-existent device and the "failok" attribute. Then verify that the system still manages to start up to multi-user mode.

[kib]

hasopt is too generic name to be added to libc, IMO. It should be renamed, and still I would prefer not to add it to libc. libutil perhaps, but libc namespace should not grow randomly.

sbin/fsck/preen.c

65

Why not int or just bool? It would require including stdbool.h into userspace, but we generally prefer bool over random integer types for booleans.

[mckusick]

I changed the name from hasopt() to getfsopt() in keeping with the other names in getfsent(3) so that it can be added more easily in the future if desired. At the moment there are variations of getfsopt() used in seven places in the base system lib/libutil/quotafile.c, libexec/rpc.rquotad/rquotad.c, sbin/fsck/fsck.c, sbin/mount/mount.c, sbin/swapon/swapon.c, usr.sbin/bsdinstall/partedit/gpart_ops.c, and usr.sbin/bsdinstall/partedit/partedit.c). Updated diff to follow shortly.

sbin/fsck/preen.c

65

Agreed that it should be int.

[mckusick]

Incorporate feedback from kib.

[kib]

sbin/fsck/fsutil.c

81

What is the exact semantic of this function?

I do not quite understand two things in the implementation:

• why does the code need to peel off the 'no' prefixes, if it seemingly cannot ever match no vs. without no
• why do you continue scanning after the first match is found. I.e., the last match for duplicate option wins?

Could you please add a comment explaining the interface.

[mckusick]

Add comment describing getfsopt(). Historical footnote: this code originated in 4.4BSD.

[mckusick]

Comment added. The function has always scanned the entire string and used the last instance of the option. Changing it now may break existing and working installations so I left it as is. Note that if it gets changed here it should be changed in the other six places to be consistent.

[kib]

In D33424#756790, @mckusick wrote:

Comment added. The function has always scanned the entire string and used the last instance of the option. Changing it now may break existing and working installations so I left it as is. Note that if it gets changed here it should be changed in the other six places to be consistent.

I still do not quite understand why do you need to handle 'no' prefix specially, both in the options string and in the name of the option itself. It seems that just tokenizing the string by splitting at commans, and then strcmp() each component with the desired option name is enough.

But I accepted the review on the basis of 'same code'.