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 <[email protected]>
To: [email protected]
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 <[email protected]>
Acked-by: Eric Sandeen <[email protected]>
Signed-off-by: Timothy Shimmin <[email protected]>
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
}
}
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 <[email protected]>
>To: [email protected]
>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 <[email protected]>
>Acked-by: Eric Sandeen <[email protected]>
>Signed-off-by: Timothy Shimmin <[email protected]>
>
>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 <[email protected]>
---
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
}
}
--
1.5.2.5
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 <[email protected]>
>> To: [email protected]
>> 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 <[email protected]>
>> Acked-by: Eric Sandeen <[email protected]>
>> Signed-off-by: Timothy Shimmin <[email protected]>
>>
>> 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 <[email protected]>
> ---
> 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:[email protected]
/"\
\ / ASCII RIBBON CAMPAIGN
X AGAINST HTML MAIL
/ \
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 <[email protected]>
>>To: [email protected]
>>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 <[email protected]>
>>Acked-by: Eric Sandeen <[email protected]>
>>Signed-off-by: Timothy Shimmin <[email protected]>
>>
>>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 <[email protected]>
>---
> 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."
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 <[email protected]>
>>> To: [email protected]
>>> 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 <[email protected]>
>>> Acked-by: Eric Sandeen <[email protected]>
>>> Signed-off-by: Timothy Shimmin <[email protected]>
>>>
>>> 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 <[email protected]>
>> ---
>> 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 <[email protected]>
---
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: [email protected]
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.
Is this patch already in Linus's tree? If so, do you have a git commit
id?
thanks,
greg k-h
On Thu, Oct 16, 2008 at 10:35:29AM -0700, Greg KH wrote:
> Is this patch already in Linus's tree? If so, do you have a git commit
6c5e51dae2c37127e00be392f40842e08077e96a
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
[email protected]
On Thu, Oct 16, 2008 at 02:10:44PM -0400, Christoph Hellwig wrote:
> On Thu, Oct 16, 2008 at 10:35:29AM -0700, Greg KH wrote:
> > Is this patch already in Linus's tree? If so, do you have a git commit
>
> 6c5e51dae2c37127e00be392f40842e08077e96a
Thanks, now queued up.
greg k-h
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