On Wed, Feb 10, 2016 at 05:45:33PM -0600, Eric Sandeen wrote:
> This patch attempts to check for a valid set of remount
> options. As far as I can tell, it's tricky; as the old
> comment says, on remount we may get a long string of
> options from /proc/mounts and/or /etc/mtab, as well
> as options specified on the commandline. Later options
> may negate previous options, etc.
>
> At the most basic level, we may be handed a mount option
> which we do not handle on remount, but which may not actually
> be a change from the current mount option set.
>
> Unfortunately our mount option state is somewhat far flung;
> a combinations of m_flags, and values in various other
> mount structure members; see the showargs function for
> a taste of that.
>
> So this extends xfs_test_remount_options() to do a full set
> of mount processing of the options remount sees, to arrive
> at a final state, then compares that state to the current
> state, and determines if we can proceed.
>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
>
> This is lightly tested; mostly just a sanity check to see
> if this approach is a "wtf?" or a "yeah, seems ok."
>
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 986290c..3d4187c 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -455,7 +455,7 @@ xfs_set_maxicount(xfs_mount_t *mp)
> * We use smaller I/O sizes when the file system
> * is being used for NFS service (wsync mount option).
> */
> -STATIC void
> +void
> xfs_set_rw_sizes(xfs_mount_t *mp)
> {
> xfs_sb_t *sbp = &(mp->m_sb);
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index a4e03ab..bee9284 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -335,6 +335,7 @@ extern int xfs_sb_validate_fsb_count(struct xfs_sb *, __uint64_t);
>
> extern int xfs_dev_is_read_only(struct xfs_mount *, char *);
>
> +extern void xfs_set_rw_sizes(xfs_mount_t *);
> extern void xfs_set_low_space_thresholds(struct xfs_mount *);
>
> int xfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index d1cd4fa..50e15d8 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -65,6 +65,8 @@ static struct kset *xfs_kset; /* top-level xfs sysfs dir */
> static struct xfs_kobj xfs_dbg_kobj; /* global debug sysfs attrs */
> #endif
>
> +STATIC int xfs_finish_flags(struct xfs_mount *mp);
> +
> /*
> * Table driven mount option parser.
> */
> @@ -1167,24 +1169,87 @@ xfs_quiesce_attr(
> xfs_log_quiesce(mp);
> }
>
> +#define XFS_BAD_REMOUNT_GOTO(mp, option, l) \
> + { \
> + xfs_warn(mp, \
> + option " options may not be changed via remount"); \
> + goto l; \
> + }
I think hiding a goto like this is wrong - it forces you to go read
the macro, making the code harder to read and follow. Really, what's
wrong with the simple and obvious:
if (bad option) {
bad_option = "bad option string";
goto out_warn;
}
.....
out_warn:
xfs_warn(mp, "%s options may not be changed via remount",
bad_option);
// free stuff
return -EINVAL;
}
Yes, I know that this sort of logic flow hiding was done with the
XFS_WANT_CORRUPTED macros, but they were written back in 90s on Irix
when using macros to implement everything were all the rage.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
