Make hostfs_setattr() support operations on closed files.

This patch allows hostfs_setattr() to work on closed files by calling set_attr() (the userspace part) with the inode's fd.

Without this, applications that depend on doing attribute changes to closed files will fail.

It works by using the fd versions instead of the path ones (for example fchmod() instead of chmod(), fchown() instead of chown()) when an fd is available.

Signed-off-by Alberto Bertogli <[email protected]>

---

I was also worried about the semantics, for instance, the outcome of:

	fd = open("f1.tmp");
	write(fd, ...);
	link("f1.tmp", "f1");
	unlink("f1.tmp");
	fd2 = open("f1.tmp");
	fchmod(fd, 0644);

In the host fchmod() changes f1 and not f1.tmp, but maybe the open() in the middle got hostfs confused. Luckily this is not the case and it works fine; but as I'm not familiar with hostfs there may be other issues.

fs/hostfs/hostfs_kern.c +0 -0

index fd301a9..c704d9c 100644
@@ -817,8 +817,14 @@ int hostfs_permission(struct inode *ino, int desired,

struct nameidata *nd) int hostfs_setattr(struct dentry *dentry, struct iattr *attr)
{

fs/hostfs/hostfs_kern.c +49 -6

 	ma = HOSTFS_ATTR_ATIME_SET | HOSTFS_ATTR_MTIME_SET;
 	if((attrs->ia_valid & ma) == ma){
+		struct timeval times[2];
+
 		buf.actime = attrs->ia_atime.tv_sec;
 		buf.modtime = attrs->ia_mtime.tv_sec;
-		if(utime(file, &buf) != 0) return(-errno);
+
+		/* atime */
+		times[0].tv_sec = attrs->ia_atime.tv_sec;
+		times[0].tv_usec = attrs->ia_atime.tv_nsec * 1000;
+
+		/* mtime */
+		times[1].tv_sec = attrs->ia_mtime.tv_sec;
+		times[1].tv_usec = attrs->ia_mtime.tv_nsec * 1000;
+
+		if(fd >= 0){
+			if(futimes(fd, times) != 0)
+				return(-errno);
+		} else if(utime(file, &buf) != 0){
+			return(-errno);
+		}
 	}
 	else {
 		struct timespec ts;
+		struct timeval times[2];
 
 		if(attrs->ia_valid & HOSTFS_ATTR_ATIME_SET){
 			err = stat_file(file, NULL, NULL, NULL, NULL, NULL,
-					NULL, NULL, &ts, NULL, NULL, NULL);
+					NULL, NULL, &ts, NULL, NULL, NULL, fd);
 			if(err != 0)
 				return(err);
 			buf.actime = attrs->ia_atime.tv_sec;
 			buf.modtime = ts.tv_sec;
-			if(utime(file, &buf) != 0)
+
+			/* atime */
+			times[0].tv_sec = attrs->ia_atime.tv_sec;
+			times[0].tv_usec = attrs->ia_atime.tv_nsec * 1000;
+
+			/* mtime */
+			times[1].tv_sec = ts.tv_sec;
+			times[1].tv_usec = ts.tv_nsec * 1000;
+
+			if(fd >= 0){
+				if(futimes(fd, times) != 0)
+					return(-errno);
+			} else if(utime(file, &buf) != 0){
 				return(-errno);
+			}
 		}
 		if(attrs->ia_valid & HOSTFS_ATTR_MTIME_SET){
 			err = stat_file(file, NULL, NULL, NULL, NULL, NULL,
-					NULL, &ts, NULL, NULL, NULL, NULL);
+					NULL, &ts, NULL, NULL, NULL, NULL, fd);
 			if(err != 0)
 				return(err);
 			buf.actime = ts.tv_sec;
 			buf.modtime = attrs->ia_mtime.tv_sec;
-			if(utime(file, &buf) != 0)
+
+			/* atime */
+			times[0].tv_sec = ts.tv_sec;
+			times[0].tv_usec = ts.tv_nsec * 1000;
+
+			/* mtime */
+			times[1].tv_sec = attrs->ia_mtime.tv_sec;
+			times[1].tv_usec = attrs->ia_mtime.tv_nsec * 1000;
+
+			if(fd >= 0){
+				if(futimes(fd, times) != 0)
+					return(-errno);
+			} else if(utime(file, &buf) != 0){
 				return(-errno);
+			}
 		}
 	}
 	if(attrs->ia_valid & HOSTFS_ATTR_CTIME) ;
 	if(attrs->ia_valid & (HOSTFS_ATTR_ATIME | HOSTFS_ATTR_MTIME)){
 		err = stat_file(file, NULL, NULL, NULL, NULL, NULL, NULL,
 				&attrs->ia_atime, &attrs->ia_mtime, NULL,
-				NULL, NULL);
+				NULL, NULL, fd);
 		if(err != 0) return(err);
 	}
 	return(0);

Quoted 11 lines · Alberto Bertogli

This patch allows hostfs_setattr() to work on closed files by calling set_attr() (the userspace part) with the inode's fd.

Without this, applications that depend on doing attribute changes to closed files will fail.

It works by using the fd versions instead of the path ones (for example fchmod() instead of chmod(), fchown() instead of chown()) when an fd is available.

Quoted 2 lines

Signed-off-by Alberto Bertogli <[email protected]>

---

This makes sense and is useful - but:
1) when I read the examples, I understood you meant 'unlinked open files', not 'closed files'. Right?

The patch is of good quality; I've not seen any single bug, which is rare in such submission, but before merging, some little things should be fixed up (either by you or by Jeff).

2) if (...) return ...; on a single line is horrible style, should be corrected (I see sometimes the original code uses it, well it shouldn't).

Quoted 3 lines

In the host fchmod() changes f1 and not f1.tmp, but maybe the open() in the middle got hostfs confused. Luckily this is not the case and it works fine; but as I'm not familiar with hostfs there may be other issues.

I guess the patch would make this work, right?

Quoted 12 lines

fs/hostfs/hostfs_kern.c +0 -0

index fd301a9..c704d9c 100644
@@ -817,8 +817,14 @@ int hostfs_permission(struct inode *ino, int desired,

struct nameidata *nd) int hostfs_setattr(struct dentry *dentry, struct iattr *attr)
{

fs/hostfs/hostfs_kern.c +2 -0

 	struct hostfs_iattr attrs;
+	struct hostfs_inode_info *iinfo;
 	char *name;
 	int err;
+	int fd = -1;

fs/hostfs/hostfs_kern.c int hostfs_permission(struct inode *ino, int desired, struct nameidata *nd) · line 827

Quoted 4 lines

fs/hostfs/hostfs_kern.c +4 -0

+
+	iinfo = HOSTFS_I(dentry->d_inode);
+	if (iinfo != NULL)
+		fd = iinfo->fd;

iinfo cannot be NULL, so please remove that if. HOSTFS_I does not access a field of d_inode; rather, d_inode points to a field of iinfo, i.e. the vfs_inode field of struct hostfs_inode_info.

I do not like the below code. The different code is for setting buf; what follows that is common to all cases, and the conversion probably should be factored out into a conversion function for better clarity: verifying what the code does is difficult here.

This patch allows hostfs_setattr() to work on closed files by calling set_attr() (the userspace part) with the inode's fd.

 fs/hostfs/hostfs.h      |    4 +-
 fs/hostfs/hostfs_kern.c |   10 ++++-
 fs/hostfs/hostfs_user.c |   89 ++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 86 insertions(+), 17 deletions(-)