Re: [RFC PATCH 1/4] pass open file to ->setattr()

Re: [RFC PATCH 1/4] pass open file to ->setattr()

[Bodo Eggert]

Miklos Szeredi <miklos@szeredi.hu> wrote:

>> > This is needed to be able to correctly implement open-unlink-fsetattr
>> > semantics in some filesystem such as sshfs, without having to resort
>> > to "silly-renaming".
>> 
>> How do you plan to do that?
> 
> Easy: the SFTP protocol has stateful opens and defines an FSTAT call.

Is it possible to reconnect without umounting? If yes, the unlinked files
would be lost in spite of being opened, wouldn't they?
-- 
Top 100 things you don't want the sysadmin to say:
11. Can you get VMS for this Sparc thingy?

Friß, Spammer: ssbuv@whUG.7eggert.dyndns.org w1tM9GC@a.7eggert.dyndns.org

Re: [RFC PATCH 1/4] pass open file to ->setattr()

[Miklos Szeredi]

> >> > This is needed to be able to correctly implement open-unlink-fsetattr
> >> > semantics in some filesystem such as sshfs, without having to resort
> >> > to "silly-renaming".
> >> 
> >> How do you plan to do that?
> > 
> > Easy: the SFTP protocol has stateful opens and defines an FSTAT call.
> 
> Is it possible to reconnect without umounting?

Yes, but open files and in-progress requests are lost at reconnect.

> If yes, the unlinked files would be lost in spite of being opened,
> wouldn't they?

Sure.  Obviously one of the drawbacks of a stateful protocol is that
the server state can't survive a reconnect.

But that sort of reliability has never been the goal of sshfs.  And
even if that was needed, it could probably be much better handled in a
lower layer.

Miklos

[RFC PATCH 0/4] VFS updates

[miklos]

VFS tweaks needed for some FUSE features, but possibly useful to other
filesystems as well.

Comments are welcome.
-- 

[RFC PATCH 1/4] pass open file to ->setattr()

[miklos]

[-- Attachment #1: vfs_setattr_file.patch --]
[-- Type: text/plain, Size: 3321 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Pass the open file into the filesystem's ->setattr() method for
fchmod, fchown and some of the utimes variants.

This is needed to be able to correctly implement open-unlink-fsetattr
semantics in some filesystem such as sshfs, without having to resort
to "silly-renaming".

The infrastructure is already there, so just need to fill in
attrs.ia_file and set ATTR_FILE in attrs.ia_valid.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/fs/open.c
===================================================================
--- linux.orig/fs/open.c	2007-08-09 16:47:30.000000000 +0200
+++ linux/fs/open.c	2007-08-09 16:48:43.000000000 +0200
@@ -581,7 +581,8 @@ asmlinkage long sys_fchmod(unsigned int 
 	if (mode == (mode_t) -1)
 		mode = inode->i_mode;
 	newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
-	newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
+	newattrs.ia_valid = ATTR_MODE | ATTR_CTIME | ATTR_FILE;
+	newattrs.ia_file = file;
 	err = notify_change(dentry, &newattrs);
 	mutex_unlock(&inode->i_mutex);
 
@@ -631,7 +632,8 @@ asmlinkage long sys_chmod(const char __u
 	return sys_fchmodat(AT_FDCWD, filename, mode);
 }
 
-static int chown_common(struct dentry * dentry, uid_t user, gid_t group)
+static int chown_common(struct dentry * dentry, uid_t user, gid_t group,
+			struct file *file)
 {
 	struct inode * inode;
 	int error;
@@ -659,6 +661,10 @@ static int chown_common(struct dentry * 
 	}
 	if (!S_ISDIR(inode->i_mode))
 		newattrs.ia_valid |= ATTR_KILL_SUID|ATTR_KILL_SGID;
+	if (file) {
+		newattrs.ia_file = file;
+		newattrs.ia_valid |= ATTR_FILE;
+	}
 	mutex_lock(&inode->i_mutex);
 	error = notify_change(dentry, &newattrs);
 	mutex_unlock(&inode->i_mutex);
@@ -674,7 +680,7 @@ asmlinkage long sys_chown(const char __u
 	error = user_path_walk(filename, &nd);
 	if (error)
 		goto out;
-	error = chown_common(nd.dentry, user, group);
+	error = chown_common(nd.dentry, user, group, NULL);
 	path_release(&nd);
 out:
 	return error;
@@ -694,7 +700,7 @@ asmlinkage long sys_fchownat(int dfd, co
 	error = __user_walk_fd(dfd, filename, follow, &nd);
 	if (error)
 		goto out;
-	error = chown_common(nd.dentry, user, group);
+	error = chown_common(nd.dentry, user, group, NULL);
 	path_release(&nd);
 out:
 	return error;
@@ -708,7 +714,7 @@ asmlinkage long sys_lchown(const char __
 	error = user_path_walk_link(filename, &nd);
 	if (error)
 		goto out;
-	error = chown_common(nd.dentry, user, group);
+	error = chown_common(nd.dentry, user, group, NULL);
 	path_release(&nd);
 out:
 	return error;
@@ -727,7 +733,7 @@ asmlinkage long sys_fchown(unsigned int 
 
 	dentry = file->f_path.dentry;
 	audit_inode(NULL, dentry);
-	error = chown_common(dentry, user, group);
+	error = chown_common(dentry, user, group, file);
 	fput(file);
 out:
 	return error;
Index: linux/fs/utimes.c
===================================================================
--- linux.orig/fs/utimes.c	2007-08-09 16:47:30.000000000 +0200
+++ linux/fs/utimes.c	2007-08-09 16:48:43.000000000 +0200
@@ -130,6 +130,10 @@ long do_utimes(int dfd, char __user *fil
 			}
 		}
 	}
+	if (f) {
+		newattrs.ia_file = f;
+		newattrs.ia_valid |= ATTR_FILE;
+	}
 	mutex_lock(&inode->i_mutex);
 	error = notify_change(dentry, &newattrs);
 	mutex_unlock(&inode->i_mutex);

--

Re: [RFC PATCH 1/4] pass open file to ->setattr()

[J. Bruce Fields]

On Thu, Aug 09, 2007 at 05:27:45PM +0200, miklos@szeredi.hu wrote:
> This is needed to be able to correctly implement open-unlink-fsetattr
> semantics in some filesystem such as sshfs, without having to resort
> to "silly-renaming".

How do you plan to do that?

--b.

Re: [RFC PATCH 1/4] pass open file to ->setattr()

[Miklos Szeredi]

> > This is needed to be able to correctly implement open-unlink-fsetattr
> > semantics in some filesystem such as sshfs, without having to resort
> > to "silly-renaming".
> 
> How do you plan to do that?

Easy: the SFTP protocol has stateful opens and defines an FSTAT call.

Miklos