Skip to content

Commit 3e7ec34

Browse files
Darrick J. Wongcmaiolino
authored andcommitted
xfs: loudly complain about defunct mount options
Apparently we can never deprecate mount options in this project, because it will invariably turn out that some foolish userspace depends on some behavior and break. From Oleksandr Natalenko: In v6.18, the attr2 XFS mount option is removed. This may silently break system boot if the attr2 option is still present in /etc/fstab for rootfs. Consider Arch Linux that is being set up from scratch with / being formatted as XFS. The genfstab command that is used to generate /etc/fstab produces something like this by default: /dev/sda2 on / type xfs (rw,relatime,attr2,discard,inode64,logbufs=8,logbsize=32k,noquota) Once the system is set up and rebooted, there's no deprecation warning seen in the kernel log: # cat /proc/cmdline root=UUID=77b42de2-397e-47ee-a1ef-4dfd430e47e9 rootflags=discard rd.luks.options=discard quiet # dmesg | grep -i xfs [ 2.409818] SGI XFS with ACLs, security attributes, realtime, scrub, repair, quota, no debug enabled [ 2.415341] XFS (sda2): Mounting V5 Filesystem 77b42de2-397e-47ee-a1ef-4dfd430e47e9 [ 2.442546] XFS (sda2): Ending clean mount Although as per the deprecation intention, it should be there. Vlastimil (in Cc) suggests this is because xfs_fs_warn_deprecated() doesn't produce any warning by design if the XFS FS is set to be rootfs and gets remounted read-write during boot. This imposes two problems: 1) a user doesn't see the deprecation warning; and 2) with v6.18 kernel, the read-write remount fails because of unknown attr2 option rendering system unusable: systemd[1]: Switching root. systemd-remount-fs[225]: /usr/bin/mount for / exited with exit status 32. # mount -o rw / mount: /: fsconfig() failed: xfs: Unknown parameter 'attr2'. Thorsten (in Cc) suggested reporting this as a user-visible regression. From my PoV, although the deprecation is in place for 5 years already, it may not be visible enough as the warning is not emitted for rootfs. Considering the amount of systems set up with XFS on /, this may impose a mass problem for users. Vlastimil suggested making attr2 option a complete noop instead of removing it. IOWs, the initrd mounts the root fs with (I assume) no mount options, and mount -a remounts with whatever options are in fstab. However, XFS doesn't complain about deprecated mount options during a remount, so technically speaking we were not warning all users in all combinations that they were heading for a cliff. Gotcha!! Now, how did 'attr2' get slurped up on so many systems? The old code would put that in /proc/mounts if the filesystem happened to be in attr2 mode, even if user hadn't mounted with any such option. IOWs, this is because someone thought it would be a good idea to advertise system state via /proc/mounts. The easy way to fix this is to reintroduce the four mount options but map them to a no-op option that ignores them, and hope that nobody's depending on attr2 to appear in /proc/mounts. (Hint: use the fsgeometry ioctl). But we've learned our lesson, so complain as LOUDLY as possible about the deprecation. Lessons learned: 1. Don't expose system state via /proc/mounts; the only strings that ought to be there are options *explicitly* provided by the user. 2. Never tidy, it's not worth the stress and irritation. Reported-by: Vlastimil Babka <[email protected]> Reported-by: Oleksandr Natalenko <[email protected]> Cc: [email protected] # v6.18-rc1 Fixes: b9a176e ("xfs: remove deprecated mount options") Signed-off-by: Darrick J. Wong <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]> Signed-off-by: Carlos Maiolino <[email protected]>
1 parent 630785b commit 3e7ec34

File tree

1 file changed

+18
-2
lines changed

1 file changed

+18
-2
lines changed

fs/xfs/xfs_super.c

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ static const struct constant_table dax_param_enums[] = {
102102
* Table driven mount option parser.
103103
*/
104104
enum {
105-
Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev,
105+
Op_deprecated, Opt_logbufs, Opt_logbsize, Opt_logdev, Opt_rtdev,
106106
Opt_wsync, Opt_noalign, Opt_swalloc, Opt_sunit, Opt_swidth, Opt_nouuid,
107107
Opt_grpid, Opt_nogrpid, Opt_bsdgroups, Opt_sysvgroups,
108108
Opt_allocsize, Opt_norecovery, Opt_inode64, Opt_inode32,
@@ -114,7 +114,21 @@ enum {
114114
Opt_lifetime, Opt_nolifetime, Opt_max_atomic_write,
115115
};
116116

117+
#define fsparam_dead(NAME) \
118+
__fsparam(NULL, (NAME), Op_deprecated, fs_param_deprecated, NULL)
119+
117120
static const struct fs_parameter_spec xfs_fs_parameters[] = {
121+
/*
122+
* These mount options were supposed to be deprecated in September 2025
123+
* but the deprecation warning was buggy, so not all users were
124+
* notified. The deprecation is now obnoxiously loud and postponed to
125+
* September 2030.
126+
*/
127+
fsparam_dead("attr2"),
128+
fsparam_dead("noattr2"),
129+
fsparam_dead("ikeep"),
130+
fsparam_dead("noikeep"),
131+
118132
fsparam_u32("logbufs", Opt_logbufs),
119133
fsparam_string("logbsize", Opt_logbsize),
120134
fsparam_string("logdev", Opt_logdev),
@@ -1423,6 +1437,9 @@ xfs_fs_parse_param(
14231437
return opt;
14241438

14251439
switch (opt) {
1440+
case Op_deprecated:
1441+
xfs_fs_warn_deprecated(fc, param);
1442+
return 0;
14261443
case Opt_logbufs:
14271444
parsing_mp->m_logbufs = result.uint_32;
14281445
return 0;
@@ -1543,7 +1560,6 @@ xfs_fs_parse_param(
15431560
xfs_mount_set_dax_mode(parsing_mp, result.uint_32);
15441561
return 0;
15451562
#endif
1546-
/* Following mount options will be removed in September 2025 */
15471563
case Opt_max_open_zones:
15481564
parsing_mp->m_max_open_zones = result.uint_32;
15491565
return 0;

0 commit comments

Comments
 (0)