[PATCH]

[PATCH]

[Christoph Hellwig]

As reported by Michael-John Turner XFS updates the mtime on the source
inode of a rename call in case it's a directory and changes the parent.

This doesn't make any sense, is not mentioned in the standards and not
performed by any other Linux filesystems so remove it.


(resending this as it might have been lost in the previous thread)


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: linux-2.6-xfs/fs/xfs/xfs_rename.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/xfs_rename.c	2008-06-18 18:24:38.000000000 +0200
+++ linux-2.6-xfs/fs/xfs/xfs_rename.c	2008-06-18 18:30:17.000000000 +0200
@@ -336,22 +336,18 @@ xfs_rename(
 		ASSERT(error != EEXIST);
 		if (error)
 			goto abort_return;
-		xfs_ichgtime(src_ip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
-
-	} else {
-		/*
-		 * We always want to hit the ctime on the source inode.
-		 * We do it in the if clause above for the 'new_parent &&
-		 * src_is_directory' case, and here we get all the other
-		 * cases.  This isn't strictly required by the standards
-		 * since the source inode isn't really being changed,
-		 * but old unix file systems did it and some incremental
-		 * backup programs won't work without it.
-		 */
-		xfs_ichgtime(src_ip, XFS_ICHGTIME_CHG);
 	}
 
 	/*
+	 * We always want to hit the ctime on the source inode.
+	 *
+	 * This isn't strictly required by the standards since the source
+	 * inode isn't really being changed, but old unix file systems did
+	 * it and some incremental backup programs won't work without it.
+	 */
+	xfs_ichgtime(src_ip, XFS_ICHGTIME_CHG);
+
+	/*
 	 * Adjust the link count on src_dp.  This is necessary when
 	 * renaming a directory, either within one parent when
 	 * the target existed, or across two parent directories.

Re: [PATCH] don't update mtime on rename source

[Christoph Hellwig]

Sorry for the missing subject, should be the one in this mail

On Tue, Jun 24, 2008 at 10:13:48AM +0200, Christoph Hellwig wrote:
> As reported by Michael-John Turner XFS updates the mtime on the source
> inode of a rename call in case it's a directory and changes the parent.
> 
> This doesn't make any sense, is not mentioned in the standards and not
> performed by any other Linux filesystems so remove it.
> 
> 
> (resending this as it might have been lost in the previous thread)
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: linux-2.6-xfs/fs/xfs/xfs_rename.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/xfs_rename.c	2008-06-18 18:24:38.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/xfs_rename.c	2008-06-18 18:30:17.000000000 +0200
> @@ -336,22 +336,18 @@ xfs_rename(
>  		ASSERT(error != EEXIST);
>  		if (error)
>  			goto abort_return;
> -		xfs_ichgtime(src_ip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
> -
> -	} else {
> -		/*
> -		 * We always want to hit the ctime on the source inode.
> -		 * We do it in the if clause above for the 'new_parent &&
> -		 * src_is_directory' case, and here we get all the other
> -		 * cases.  This isn't strictly required by the standards
> -		 * since the source inode isn't really being changed,
> -		 * but old unix file systems did it and some incremental
> -		 * backup programs won't work without it.
> -		 */
> -		xfs_ichgtime(src_ip, XFS_ICHGTIME_CHG);
>  	}
>  
>  	/*
> +	 * We always want to hit the ctime on the source inode.
> +	 *
> +	 * This isn't strictly required by the standards since the source
> +	 * inode isn't really being changed, but old unix file systems did
> +	 * it and some incremental backup programs won't work without it.
> +	 */
> +	xfs_ichgtime(src_ip, XFS_ICHGTIME_CHG);
> +
> +	/*
>  	 * Adjust the link count on src_dp.  This is necessary when
>  	 * renaming a directory, either within one parent when
>  	 * the target existed, or across two parent directories.
---end quoted text---

[PATCH]

[Tim Shimmin]

Hi Linus,

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
 		}
 	}

Re: [PATCH]

[Eric Sandeen]

> Date: Wed, 15 Oct 2008 18:03:13 +1100
> To: options@melbourne.sgi.com, unrecognized@melbourne.sgi.com,
>         with@melbourne.sgi.com, rw@melbourne.sgi.com,
>         remount@melbourne.sgi.com, fix@melbourne.sgi.com,
>         XFS@melbourne.sgi.com


I don't think that made it to the right place ...  :)

(stable@vger.kernel.org)

-Eric

Tim Shimmin wrote:
> Hi Linus,
> 
> 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
>  		}
>  	}
> 
>