Re: [PATCH 4/7] fs: make remaining filesystems use .rename2

From: Boaz Harrosh <ooo@electrozaur.com>
To: Miklos Szeredi <mszeredi@redhat.com>, linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Al Viro <viro@zeniv.linux.org.uk>,
	Eric Van Hensbergen <ericvh@gmail.com>,
	David Howells <dhowells@redhat.com>,
	Ilya Dryomov <idryomov@gmail.com>,
	Jan Harkes <jaharkes@cs.cmu.edu>,
	Tyler Hicks <tyhicks@canonical.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Oleg Drokin <oleg.drokin@intel.com>,
	Trond Myklebust <trond.myklebust@primarydata.com>,
	Mark Fasheh <mfasheh@suse.com>,
	Mike Marshall <hubcap@omnibond.com>
Subject: Re: [PATCH 4/7] fs: make remaining filesystems use .rename2
Date: Tue, 23 Aug 2016 19:24:39 +0300	[thread overview]
Message-ID: <57BC78C7.9040709@electrozaur.com> (raw)
In-Reply-To: <1471961132-1675-5-git-send-email-mszeredi@redhat.com>

On 08/23/2016 05:05 PM, Miklos Szeredi wrote:
> This is trivial to do:
> 
>  - add flags argument to foo_rename()
>  - check if flags is zero
>  - assign foo_rename() to .rename2 instead of .rename
> 
> This doesn't mean it's impossible to support RENAME_NOREPLACE for these
> filesystems, but it is not trivial, like for local filesystems.
> RENAME_NOREPLACE must guarantee atomicity (i.e. it shouldn't be possible
> for a file to be created on one host while it is overwritten by rename on
> another host).
> 
> Filesystems converted:
> 
> 9p, afs, ceph, coda, ecryptfs, exofs, kernfs, lustre, ncpfs, nfs, ocfs2,
> orangefs.
> 
> After this, we can get rid of the duplicate interfaces for rename.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> Cc: Eric Van Hensbergen <ericvh@gmail.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Ilya Dryomov <idryomov@gmail.com>
> Cc: Jan Harkes <jaharkes@cs.cmu.edu>
> Cc: Tyler Hicks <tyhicks@canonical.com>
> Cc: Boaz Harrosh <ooo@electrozaur.com>

Hi exofs is not a distributed file system in the nfs-client 
sense. All meta-data operations happen on the single exofs mount.
The distribution of an exofs cluster is done by an NFSD-like daemon
that supports pNFS, and an std pNFS-client.
So the code you see below is just the same as an ext4 FS with
a raid of iscsi devices below it. (Even if then later this FS is
exported by an NFSD server)

That said it is fine as is don't sweat over this unused FS so:
ACK-by: Boaz Harrosh <ooo@electrozaur.com>

<>

> diff --git a/fs/exofs/namei.c b/fs/exofs/namei.c
> index 622a686bb08b..897280163f3c 100644
> --- a/fs/exofs/namei.c
> +++ b/fs/exofs/namei.c
> @@ -227,7 +227,8 @@ static int exofs_rmdir(struct inode *dir, struct dentry *dentry)
>  }
>  
>  static int exofs_rename(struct inode *old_dir, struct dentry *old_dentry,
> -		struct inode *new_dir, struct dentry *new_dentry)
> +			struct inode *new_dir, struct dentry *new_dentry,
> +			unsigned int flags)
>  {
>  	struct inode *old_inode = d_inode(old_dentry);
>  	struct inode *new_inode = d_inode(new_dentry);
> @@ -237,6 +238,9 @@ static int exofs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  	struct exofs_dir_entry *old_de;
>  	int err = -ENOENT;
>  
> +	if (flags)
> +		return -EINVAL;
> +
>  	old_de = exofs_find_entry(old_dir, old_dentry, &old_page);
>  	if (!old_de)
>  		goto out;
> @@ -310,7 +314,7 @@ const struct inode_operations exofs_dir_inode_operations = {
>  	.mkdir  	= exofs_mkdir,
>  	.rmdir  	= exofs_rmdir,
>  	.mknod  	= exofs_mknod,
> -	.rename 	= exofs_rename,
> +	.rename2	= exofs_rename,
>  	.setattr	= exofs_setattr,
>  };
>  
<>