* Re: [PATCH] XFS fix remount rw with unrecognized options
2008-10-16 13:41 ` Jianjun Kong
@ 2008-10-16 13:55 ` Klaus Strebel
2008-10-16 14:07 ` Jianjun Kong
2008-10-16 13:59 ` Américo Wang
2008-10-16 22:36 ` Dave Chinner
2 siblings, 1 reply; 10+ messages in thread
From: Klaus Strebel @ 2008-10-16 13:55 UTC (permalink / raw)
To: Jianjun Kong; +Cc: Tim Shimmin, stable, linux-kernel, xfs
Jianjun Kong schrieb:
> On Thu, Oct 16, 2008 at 11:29:26AM +1100, Tim Shimmin wrote:
>> Resending as I mangled sending the mail from home last time. Sorry.
>>
>> Please include the following patch for 2.6.27.1 stable release as
>> suggested by Christoph Hellwig and Eric Sandeen.
>> It fixes a regression in the recent remount recoding
>> where remounting say from ro to rw allows the xfs flags to
>> be out of sync with the vfs flags, resulting
>> in failures for some programs such as touch (which end up calling xfs_setattr).
>> The fix is a very minor and clear.
>>
>> Thanks,
>> Tim.
>>
>> Date: Sun, 12 Oct 2008 14:30:44 +0200
>> From: Christoph Hellwig <hch@lst.de>
>> To: xfs@oss.sgi.com
>> Subject: [PATCH] fix remount rw with unrecognized options
>>
>> When we skip unrecognized options in xfs_fs_remount we should just break
>> out of the switch and not return because otherwise we may skip clearing
>> the xfs-internal read-only flag. This will only show up on some
>> operations like touch because most read-only checks are done by the VFS
>> which thinks this filesystem is r/w. Eventually we should replace the
>> XFS read-only flag with a helper that always checks the VFS flag to make
>> sure they can never get out of sync.
>>
>> Bug reported and fix verified by Marcel Beister on #xfs.
>> Bug fix verified by updated xfstests/189.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> Acked-by: Eric Sandeen <sandeen@sandeen.net>
>> Signed-off-by: Timothy Shimmin <tes@sgi.com>
>>
>> Index: mainline/fs/xfs/linux-2.6/xfs_super.c
>> ===================================================================
>> --- mainline.orig/fs/xfs/linux-2.6/xfs_super.c 2008-10-15 17:59:26.542652847 +1100
>> +++ mainline/fs/xfs/linux-2.6/xfs_super.c 2008-10-15 17:59:45.376217172 +1100
>> @@ -1323,7 +1323,7 @@ xfs_fs_remount(
>> "XFS: mount option \"%s\" not supported for remount\n", p);
>> return -EINVAL;
>> #else
>> - return 0;
>> + break;
>> #endif
>> }
>> }
>
>
> And the code above "return 0" can not be executed, so delete them.
> __________________________________________
> #if 0
> printk(KERN_INFO
> "XFS: mount option \"%s\" not supported for remount\n", p);
> return -EINVAL;
> #else
> return 0;
> -----------------------------------------
>
>
> Signed-off-by: Jianjun Kong <kongjianjun@gmail.com>
> ---
> fs/xfs/linux-2.6/xfs_super.c | 7 +------
> 1 files changed, 1 insertions(+), 6 deletions(-)
>
> diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
> index e390136..bd5ec81 100644
> --- a/fs/xfs/linux-2.6/xfs_super.c
> +++ b/fs/xfs/linux-2.6/xfs_super.c
> @@ -1318,12 +1318,7 @@ xfs_fs_remount(
> * every remount request, and silently ignore all
> * options that we can't actually change.
> */
> -#if 0
> - printk(KERN_INFO
> - "XFS: mount option \"%s\" not supported for remount\n", p);
> - return -EINVAL;
> -#else
> - break;
> + break 0;
> #endif
> }
> }
I see my compilers error message '"#endif" without corresponding "#if"'
and i think he'll also dislike the 'break 0;' ... 'missing ";" before
"0" in line xxx' ;-).
--
Mit freundlichen Grüssen / best regards
Klaus Strebel, Dipl.-Inform. (FH), mailto:klaus.strebel@gmx.net
/"\
\ / ASCII RIBBON CAMPAIGN
X AGAINST HTML MAIL
/ \
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] XFS fix remount rw with unrecognized options
2008-10-16 13:55 ` Klaus Strebel
@ 2008-10-16 14:07 ` Jianjun Kong
0 siblings, 0 replies; 10+ messages in thread
From: Jianjun Kong @ 2008-10-16 14:07 UTC (permalink / raw)
To: Klaus Strebel; +Cc: Tim Shimmin, stable, linux-kernel, xfs
On Thu, Oct 16, 2008 at 03:55:49PM +0200, Klaus Strebel wrote:
>Jianjun Kong schrieb:
>> On Thu, Oct 16, 2008 at 11:29:26AM +1100, Tim Shimmin wrote:
>>> Resending as I mangled sending the mail from home last time. Sorry.
>>>
>>> Please include the following patch for 2.6.27.1 stable release as
>>> suggested by Christoph Hellwig and Eric Sandeen.
>>> It fixes a regression in the recent remount recoding
>>> where remounting say from ro to rw allows the xfs flags to
>>> be out of sync with the vfs flags, resulting
>>> in failures for some programs such as touch (which end up calling xfs_setattr).
>>> The fix is a very minor and clear.
>>>
>>> Thanks,
>>> Tim.
>>>
>>> Date: Sun, 12 Oct 2008 14:30:44 +0200
>>> From: Christoph Hellwig <hch@lst.de>
>>> To: xfs@oss.sgi.com
>>> Subject: [PATCH] fix remount rw with unrecognized options
>>>
>>> When we skip unrecognized options in xfs_fs_remount we should just break
>>> out of the switch and not return because otherwise we may skip clearing
>>> the xfs-internal read-only flag. This will only show up on some
>>> operations like touch because most read-only checks are done by the VFS
>>> which thinks this filesystem is r/w. Eventually we should replace the
>>> XFS read-only flag with a helper that always checks the VFS flag to make
>>> sure they can never get out of sync.
>>>
>>> Bug reported and fix verified by Marcel Beister on #xfs.
>>> Bug fix verified by updated xfstests/189.
>>>
>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>> Acked-by: Eric Sandeen <sandeen@sandeen.net>
>>> Signed-off-by: Timothy Shimmin <tes@sgi.com>
>>>
>>> Index: mainline/fs/xfs/linux-2.6/xfs_super.c
>>> ===================================================================
>>> --- mainline.orig/fs/xfs/linux-2.6/xfs_super.c 2008-10-15 17:59:26.542652847 +1100
>>> +++ mainline/fs/xfs/linux-2.6/xfs_super.c 2008-10-15 17:59:45.376217172 +1100
>>> @@ -1323,7 +1323,7 @@ xfs_fs_remount(
>>> "XFS: mount option \"%s\" not supported for remount\n", p);
>>> return -EINVAL;
>>> #else
>>> - return 0;
>>> + break;
>>> #endif
<....>
>> Signed-off-by: Jianjun Kong <kongjianjun@gmail.com>
>> ---
>> fs/xfs/linux-2.6/xfs_super.c | 7 +------
>> 1 files changed, 1 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
>> index e390136..bd5ec81 100644
>> --- a/fs/xfs/linux-2.6/xfs_super.c
>> +++ b/fs/xfs/linux-2.6/xfs_super.c
>> @@ -1318,12 +1318,7 @@ xfs_fs_remount(
>> * every remount request, and silently ignore all
>> * options that we can't actually change.
>> */
>> -#if 0
>> - printk(KERN_INFO
>> - "XFS: mount option \"%s\" not supported for remount\n", p);
>> - return -EINVAL;
>> -#else
>> - break;
>> + break 0;
>> #endif
>> }
>> }
>I see my compilers error message '"#endif" without corresponding "#if"'
>and i think he'll also dislike the 'break 0;' ... 'missing ";" before
>"0" in line xxx' ;-).
Sorry. another one :)
Signed-off-by: Jianjun Kong <kongjianjun@gmail.com>
---
fs/xfs/linux-2.6/xfs_super.c | 8 +-------
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
index e390136..c019f78 100644
--- a/fs/xfs/linux-2.6/xfs_super.c
+++ b/fs/xfs/linux-2.6/xfs_super.c
@@ -1318,13 +1318,7 @@ xfs_fs_remount(
* every remount request, and silently ignore all
* options that we can't actually change.
*/
-#if 0
- printk(KERN_INFO
- "XFS: mount option \"%s\" not supported for remount\n", p);
- return -EINVAL;
-#else
- break;
-#endif
+ break ;
}
}
--
1.5.2.5
--
Jianjun Kong | Happy Hacking
HomePage: http://kongove.cn
Gtalk: kongjianjun@gmail.com
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] XFS fix remount rw with unrecognized options
2008-10-16 13:41 ` Jianjun Kong
2008-10-16 13:55 ` Klaus Strebel
@ 2008-10-16 13:59 ` Américo Wang
2008-10-16 22:36 ` Dave Chinner
2 siblings, 0 replies; 10+ messages in thread
From: Américo Wang @ 2008-10-16 13:59 UTC (permalink / raw)
To: Jianjun Kong; +Cc: Tim Shimmin, stable, linux-kernel, xfs
On Thu, Oct 16, 2008 at 09:41:12PM +0800, Jianjun Kong wrote:
>On Thu, Oct 16, 2008 at 11:29:26AM +1100, Tim Shimmin wrote:
>>Resending as I mangled sending the mail from home last time. Sorry.
>>
>>Please include the following patch for 2.6.27.1 stable release as
>>suggested by Christoph Hellwig and Eric Sandeen.
>>It fixes a regression in the recent remount recoding
>>where remounting say from ro to rw allows the xfs flags to
>>be out of sync with the vfs flags, resulting
>>in failures for some programs such as touch (which end up calling xfs_setattr).
>>The fix is a very minor and clear.
>>
>>Thanks,
>>Tim.
>>
>>Date: Sun, 12 Oct 2008 14:30:44 +0200
>>From: Christoph Hellwig <hch@lst.de>
>>To: xfs@oss.sgi.com
>>Subject: [PATCH] fix remount rw with unrecognized options
>>
>>When we skip unrecognized options in xfs_fs_remount we should just break
>>out of the switch and not return because otherwise we may skip clearing
>>the xfs-internal read-only flag. This will only show up on some
>>operations like touch because most read-only checks are done by the VFS
>>which thinks this filesystem is r/w. Eventually we should replace the
>>XFS read-only flag with a helper that always checks the VFS flag to make
>>sure they can never get out of sync.
>>
>>Bug reported and fix verified by Marcel Beister on #xfs.
>>Bug fix verified by updated xfstests/189.
>>
>>Signed-off-by: Christoph Hellwig <hch@lst.de>
>>Acked-by: Eric Sandeen <sandeen@sandeen.net>
>>Signed-off-by: Timothy Shimmin <tes@sgi.com>
>>
>>Index: mainline/fs/xfs/linux-2.6/xfs_super.c
>>===================================================================
>>--- mainline.orig/fs/xfs/linux-2.6/xfs_super.c 2008-10-15 17:59:26.542652847 +1100
>>+++ mainline/fs/xfs/linux-2.6/xfs_super.c 2008-10-15 17:59:45.376217172 +1100
>>@@ -1323,7 +1323,7 @@ xfs_fs_remount(
>> "XFS: mount option \"%s\" not supported for remount\n", p);
>> return -EINVAL;
>> #else
>>- return 0;
>>+ break;
>> #endif
>> }
>> }
>
>
>And the code above "return 0" can not be executed, so delete them.
return 0; is in the removed part... so it's already removed.
>__________________________________________
>#if 0
> printk(KERN_INFO
> "XFS: mount option \"%s\" not supported for remount\n", p);
> return -EINVAL;
>#else
> return 0;
>-----------------------------------------
>
>
>Signed-off-by: Jianjun Kong <kongjianjun@gmail.com>
>---
> fs/xfs/linux-2.6/xfs_super.c | 7 +------
> 1 files changed, 1 insertions(+), 6 deletions(-)
>
>diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
>index e390136..bd5ec81 100644
>--- a/fs/xfs/linux-2.6/xfs_super.c
>+++ b/fs/xfs/linux-2.6/xfs_super.c
>@@ -1318,12 +1318,7 @@ xfs_fs_remount(
> * every remount request, and silently ignore all
> * options that we can't actually change.
> */
>-#if 0
>- printk(KERN_INFO
>- "XFS: mount option \"%s\" not supported for remount\n", p);
>- return -EINVAL;
>-#else
>- break;
>+ break 0;
break 0; doesn't make any sense in C...
--
"Sometimes the only way to stay sane is to go a little crazy."
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] XFS fix remount rw with unrecognized options
2008-10-16 13:41 ` Jianjun Kong
2008-10-16 13:55 ` Klaus Strebel
2008-10-16 13:59 ` Américo Wang
@ 2008-10-16 22:36 ` Dave Chinner
2008-10-16 22:48 ` Timothy Shimmin
2 siblings, 1 reply; 10+ messages in thread
From: Dave Chinner @ 2008-10-16 22:36 UTC (permalink / raw)
To: Jianjun Kong; +Cc: Tim Shimmin, stable, linux-kernel, xfs
On Thu, Oct 16, 2008 at 09:41:12PM +0800, Jianjun Kong wrote:
>
> And the code above "return 0" can not be executed, so delete them.
> __________________________________________
> #if 0
> printk(KERN_INFO
> "XFS: mount option \"%s\" not supported for remount\n", p);
> return -EINVAL;
> #else
> return 0;
> -----------------------------------------
That ifdef was added to stub out the behaviour we *actually want*
remount to have when it encounters an invalid remount option.
The reason for leaving it is in the next release we should have all
the other problems we discovered by doing this fixed, so that
code will be re-enabled. Hence the temporary #if 0 around it.
So at this point, don't remove it.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] XFS fix remount rw with unrecognized options
2008-10-16 22:36 ` Dave Chinner
@ 2008-10-16 22:48 ` Timothy Shimmin
0 siblings, 0 replies; 10+ messages in thread
From: Timothy Shimmin @ 2008-10-16 22:48 UTC (permalink / raw)
To: Jianjun Kong, Dave Chinner, stable, linux-kernel, xfs
Dave Chinner wrote:
> On Thu, Oct 16, 2008 at 09:41:12PM +0800, Jianjun Kong wrote:
>> And the code above "return 0" can not be executed, so delete them.
>> __________________________________________
>> #if 0
>> printk(KERN_INFO
>> "XFS: mount option \"%s\" not supported for remount\n", p);
>> return -EINVAL;
>> #else
>> return 0;
>> -----------------------------------------
>
> That ifdef was added to stub out the behaviour we *actually want*
> remount to have when it encounters an invalid remount option.
> The reason for leaving it is in the next release we should have all
> the other problems we discovered by doing this fixed, so that
> code will be re-enabled. Hence the temporary #if 0 around it.
>
> So at this point, don't remove it.
>
> Cheers,
>
> Dave.
Exactly.
I don't want it removed.
Extra code for comparing existing mount state with new mount state
needs to be done and this code can be reactivated when we sort
this stuff out later.
This patch should be just the fix for the current regression.
Thanks,
--Tim
^ permalink raw reply [flat|nested] 10+ messages in thread