2007-08-09 15:31:01

by Miklos Szeredi

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

From: Miklos Szeredi <[email protected]>

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 <[email protected]>
---

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);

--

2007-08-09 15:36:45

by J. Bruce Fields

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

On Thu, Aug 09, 2007 at 05:27:45PM +0200, [email protected] 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.

2007-08-09 15:42:21

by Miklos Szeredi

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

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

2007-08-09 21:53:47

by Bodo Eggert

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

Miklos Szeredi <[email protected]> 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: [email protected] [email protected]

2007-08-10 05:41:21

by Miklos Szeredi

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

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