xfs
[Top] [All Lists]

Re: [PATCH] stop rejecting options in remount

To: Donald Douwsma <donaldd@xxxxxxx>
Subject: Re: [PATCH] stop rejecting options in remount
From: Timothy Shimmin <tes@xxxxxxx>
Date: Mon, 11 Aug 2008 14:48:49 +1000
Cc: Christoph Hellwig <hch@xxxxxx>, xfs@xxxxxxxxxxx, jasper@xxxxxxxxxxxx
In-reply-to: <489FB537.70001@xxxxxxx>
References: <20080809195159.GA8562@xxxxxx> <489FB537.70001@xxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.16 (Macintosh/20080707)
Donald Douwsma wrote:
> Christoph Hellwig wrote:
>> Thanks to some not so nice code in mount(8) we can't blindly reject moun
>> options we don't support to be changed in remount.  See the comment in
>> the code for more details.
>>
>>
>> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>>
>> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.c
>> ===================================================================
>> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_super.c    2008-08-09
>> 16:34:33.000000000 -0300
>> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.c    2008-08-09
>> 16:36:55.000000000 -0300
>> @@ -1349,9 +1349,28 @@ xfs_fs_remount(
>>              mp->m_flags &= ~XFS_MOUNT_BARRIER;
>>              break;
>>          default:
>> +            /*
>> +             * 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
>>          }
>>      }
>>  
> 
> We should sill issue a warning that the option was ignored. It's not as
> obvious as refusing the mount but will allow some kind of triage if
> strange behavior results.
> 
Well, this is just a temporary partial fix, right.
If we warn then we will be warning incorrectly in the cases where we
are just carrying through the original mount options (which could
be in fstab etc.) AFAICT
which was the problem that was reported.

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.

--Tim


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