xfs
[Top] [All Lists]

Re: [PATCH] stop rejecting options in remount

To: Timothy Shimmin <tes@xxxxxxx>
Subject: Re: [PATCH] stop rejecting options in remount
From: Christoph Hellwig <hch@xxxxxx>
Date: Mon, 11 Aug 2008 14:11:18 +0200
Cc: xfs@xxxxxxxxxxx, jasper@xxxxxxxxxxxx
In-reply-to: <489FC4B1.5040402@xxxxxxx>
References: <20080809195159.GA8562@xxxxxx> <489FB537.70001@xxxxxxx> <489FC4B1.5040402@xxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.3.28i
On Mon, Aug 11, 2008 at 02:48:49PM +1000, Timothy Shimmin wrote:
> >> +            /*
> >> +             * Logically we would return an error here to prevent
> >> +             * users from believing they might have changed
> >> +             * mount options using remount which can't be changed.
> >> +             *
> >> +             * But unfortunately mount(8) adds all options from
> >> +             * mtab and fstab to the mount arguments in some
> >> +             * cases so we can't blindly reject options.
> >> +             *
> >> +             * The workaround for that behaviour will be to
> >> +             * check for each specified option whether it actually
> >> +             * is a change compared to the status quo and if yes
> >> +             * silently ignore it or otherwise reject the remount
> >> +             * and tell the user about the reason.
> >> +             */
> >> +#if 0
> >>              printk(KERN_INFO
> >>      "XFS: mount option \"%s\" not supported for remount\n", p);
> >>              return -EINVAL;
> >> +#else
> >> +            return 0;
> >> +#endif

> I don't follow the 3rd paragraph of the comment above.
> The current workaround looks like to always let the options through
> (ignoring the unallowed changed ones),
> and a better fix would be to compare what the requested opts are with
> the current opts and for the changed one's see if they are allowed
> on remount.

Yes, that's what I wanted to say with the third paragraph :)  I'll send
an updated version with a better worded comment once we have concensus
on wether to warn or not.


<Prev in Thread] Current Thread [Next in Thread>