public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [patch 0/5] Patch queue for 2.6.29
@ 2008-12-09  9:47 Christoph Hellwig
  2008-12-09  9:47 ` [patch 1/5] replace b_fspriv with b_mount Christoph Hellwig
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Christoph Hellwig @ 2008-12-09  9:47 UTC (permalink / raw)
  To: xfs

These are my outstanding patches for 2.6.29.  The first two have already
been rewied by Dave, the others still need review.

-- 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [patch 1/5] replace b_fspriv with b_mount
  2008-12-09  9:47 [patch 0/5] Patch queue for 2.6.29 Christoph Hellwig
@ 2008-12-09  9:47 ` Christoph Hellwig
  2008-12-09  9:56   ` Martin Steigerwald
  2008-12-09 20:04   ` Russell Cattelan
  2008-12-09  9:47 ` [patch 2/5] simplify projid check in xfs_rename Christoph Hellwig
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 15+ messages in thread
From: Christoph Hellwig @ 2008-12-09  9:47 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-add-bp_mount-field --]
[-- Type: text/plain, Size: 6484 bytes --]

Replace the b_fspriv pointer and it's ugly accessors with a properly types
xfs_mount pointer.  Also switch log reocvery over to it instead of using
b_fspriv for the mount pointer.


Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <david@fromorbit.com>

Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_buf.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_buf.c	2008-11-15 15:27:32.000000000 +0100
+++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_buf.c	2008-11-15 15:28:01.000000000 +0100
@@ -1085,7 +1085,7 @@ xfs_bawrite(
 	bp->b_flags &= ~(XBF_READ | XBF_DELWRI | XBF_READ_AHEAD);
 	bp->b_flags |= (XBF_WRITE | XBF_ASYNC | _XBF_RUN_QUEUES);
 
-	bp->b_fspriv3 = mp;
+	bp->b_mount = mp;
 	bp->b_strat = xfs_bdstrat_cb;
 	return xfs_bdstrat_cb(bp);
 }
@@ -1098,7 +1098,7 @@ xfs_bdwrite(
 	XB_TRACE(bp, "bdwrite", 0);
 
 	bp->b_strat = xfs_bdstrat_cb;
-	bp->b_fspriv3 = mp;
+	bp->b_mount = mp;
 
 	bp->b_flags &= ~XBF_READ;
 	bp->b_flags |= (XBF_DELWRI | XBF_ASYNC);
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_buf.h
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_buf.h	2008-11-15 15:28:08.000000000 +0100
+++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_buf.h	2008-11-15 15:29:12.000000000 +0100
@@ -168,7 +168,7 @@ typedef struct xfs_buf {
 	struct completion	b_iowait;	/* queue for I/O waiters */
 	void			*b_fspriv;
 	void			*b_fspriv2;
-	void			*b_fspriv3;
+	struct xfs_mount	*b_mount;
 	unsigned short		b_error;	/* error code on I/O */
 	unsigned int		b_page_count;	/* size of page array */
 	unsigned int		b_offset;	/* page offset in first page */
@@ -335,8 +335,6 @@ extern void xfs_buf_trace(xfs_buf_t *, c
 #define XFS_BUF_SET_FSPRIVATE(bp, val)		((bp)->b_fspriv = (void*)(val))
 #define XFS_BUF_FSPRIVATE2(bp, type)		((type)(bp)->b_fspriv2)
 #define XFS_BUF_SET_FSPRIVATE2(bp, val)		((bp)->b_fspriv2 = (void*)(val))
-#define XFS_BUF_FSPRIVATE3(bp, type)		((type)(bp)->b_fspriv3)
-#define XFS_BUF_SET_FSPRIVATE3(bp, val)		((bp)->b_fspriv3 = (void*)(val))
 #define XFS_BUF_SET_START(bp)			do { } while (0)
 #define XFS_BUF_SET_BRELSE_FUNC(bp, func)	((bp)->b_relse = (func))
 
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_lrw.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_lrw.c	2008-11-15 15:25:29.000000000 +0100
+++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_lrw.c	2008-11-15 15:26:27.000000000 +0100
@@ -847,13 +847,7 @@ retry:
 int
 xfs_bdstrat_cb(struct xfs_buf *bp)
 {
-	xfs_mount_t	*mp;
-
-	mp = XFS_BUF_FSPRIVATE3(bp, xfs_mount_t *);
-	if (!XFS_FORCED_SHUTDOWN(mp)) {
-		xfs_buf_iorequest(bp);
-		return 0;
-	} else {
+	if (XFS_FORCED_SHUTDOWN(bp->b_mount)) {
 		xfs_buftrace("XFS__BDSTRAT IOERROR", bp);
 		/*
 		 * Metadata write that didn't get logged but
@@ -866,6 +860,9 @@ xfs_bdstrat_cb(struct xfs_buf *bp)
 		else
 			return (xfs_bioerror(bp));
 	}
+
+	xfs_buf_iorequest(bp);
+	return 0;
 }
 
 /*
Index: linux-2.6-xfs/fs/xfs/xfs_buf_item.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/xfs_buf_item.c	2008-11-15 15:24:38.000000000 +0100
+++ linux-2.6-xfs/fs/xfs/xfs_buf_item.c	2008-11-15 15:25:05.000000000 +0100
@@ -707,8 +707,8 @@ xfs_buf_item_init(
 	 * the first.  If we do already have one, there is
 	 * nothing to do here so return.
 	 */
-	if (XFS_BUF_FSPRIVATE3(bp, xfs_mount_t *) != mp)
-		XFS_BUF_SET_FSPRIVATE3(bp, mp);
+	if (bp->b_mount != mp)
+		bp->b_mount = mp;
 	XFS_BUF_SET_BDSTRAT_FUNC(bp, xfs_bdstrat_cb);
 	if (XFS_BUF_FSPRIVATE(bp, void *) != NULL) {
 		lip = XFS_BUF_FSPRIVATE(bp, xfs_log_item_t *);
Index: linux-2.6-xfs/fs/xfs/xfs_rw.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/xfs_rw.c	2008-11-15 15:26:37.000000000 +0100
+++ linux-2.6-xfs/fs/xfs/xfs_rw.c	2008-11-15 15:27:17.000000000 +0100
@@ -406,7 +406,7 @@ xfs_bwrite(
 	 * XXXsup how does this work for quotas.
 	 */
 	XFS_BUF_SET_BDSTRAT_FUNC(bp, xfs_bdstrat_cb);
-	XFS_BUF_SET_FSPRIVATE3(bp, mp);
+	bp->b_mount = mp;
 	XFS_BUF_WRITE(bp);
 
 	if ((error = XFS_bwrite(bp))) {
Index: linux-2.6-xfs/fs/xfs/xfs_log_recover.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/xfs_log_recover.c	2008-11-15 15:31:41.000000000 +0100
+++ linux-2.6-xfs/fs/xfs/xfs_log_recover.c	2008-11-15 15:35:12.000000000 +0100
@@ -267,21 +267,16 @@ STATIC void
 xlog_recover_iodone(
 	struct xfs_buf	*bp)
 {
-	xfs_mount_t	*mp;
-
-	ASSERT(XFS_BUF_FSPRIVATE(bp, void *));
-
 	if (XFS_BUF_GETERROR(bp)) {
 		/*
 		 * We're not going to bother about retrying
 		 * this during recovery. One strike!
 		 */
-		mp = XFS_BUF_FSPRIVATE(bp, xfs_mount_t *);
 		xfs_ioerror_alert("xlog_recover_iodone",
-				  mp, bp, XFS_BUF_ADDR(bp));
-		xfs_force_shutdown(mp, SHUTDOWN_META_IO_ERROR);
+				  bp->b_mount, bp, XFS_BUF_ADDR(bp));
+		xfs_force_shutdown(bp->b_mount, SHUTDOWN_META_IO_ERROR);
 	}
-	XFS_BUF_SET_FSPRIVATE(bp, NULL);
+	bp->b_mount = NULL;
 	XFS_BUF_CLR_IODONE_FUNC(bp);
 	xfs_biodone(bp);
 }
@@ -2225,9 +2220,8 @@ xlog_recover_do_buffer_trans(
 		XFS_BUF_STALE(bp);
 		error = xfs_bwrite(mp, bp);
 	} else {
-		ASSERT(XFS_BUF_FSPRIVATE(bp, void *) == NULL ||
-		       XFS_BUF_FSPRIVATE(bp, xfs_mount_t *) == mp);
-		XFS_BUF_SET_FSPRIVATE(bp, mp);
+		ASSERT(bp->b_mount == NULL || bp->b_mount == mp);
+		bp->b_mount = mp;
 		XFS_BUF_SET_IODONE_FUNC(bp, xlog_recover_iodone);
 		xfs_bdwrite(mp, bp);
 	}
@@ -2490,9 +2484,8 @@ xlog_recover_do_inode_trans(
 
 write_inode_buffer:
 	if (ITEM_TYPE(item) == XFS_LI_INODE) {
-		ASSERT(XFS_BUF_FSPRIVATE(bp, void *) == NULL ||
-		       XFS_BUF_FSPRIVATE(bp, xfs_mount_t *) == mp);
-		XFS_BUF_SET_FSPRIVATE(bp, mp);
+		ASSERT(bp->b_mount == NULL || bp->b_mount == mp);
+		bp->b_mount = mp;
 		XFS_BUF_SET_IODONE_FUNC(bp, xlog_recover_iodone);
 		xfs_bdwrite(mp, bp);
 	} else {
@@ -2623,9 +2616,8 @@ xlog_recover_do_dquot_trans(
 	memcpy(ddq, recddq, item->ri_buf[1].i_len);
 
 	ASSERT(dq_f->qlf_size == 2);
-	ASSERT(XFS_BUF_FSPRIVATE(bp, void *) == NULL ||
-	       XFS_BUF_FSPRIVATE(bp, xfs_mount_t *) == mp);
-	XFS_BUF_SET_FSPRIVATE(bp, mp);
+	ASSERT(bp->b_mount == NULL || bp->b_mount == mp);
+	bp->b_mount = mp;
 	XFS_BUF_SET_IODONE_FUNC(bp, xlog_recover_iodone);
 	xfs_bdwrite(mp, bp);
 

-- 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [patch 2/5] simplify projid check in xfs_rename
  2008-12-09  9:47 [patch 0/5] Patch queue for 2.6.29 Christoph Hellwig
  2008-12-09  9:47 ` [patch 1/5] replace b_fspriv with b_mount Christoph Hellwig
@ 2008-12-09  9:47 ` Christoph Hellwig
  2008-12-09  9:47 ` [patch 3/5] resync headers with libxfs Christoph Hellwig
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2008-12-09  9:47 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-rename-projid-check --]
[-- Type: text/plain, Size: 2817 bytes --]


Check for the project ID after attaching all inodes to the transaction.
That way the unlock in the error case is done by the transaction subsystem,
which guaratees that is uses the right flags (which was wrong from day one
of this check), and avoids having special code unlocking an array of inodes
with potential duplicates.  Attaching the inode first is the method used
by xfs_rename and the other namespace methods all other error that require
multiple locked inodes.


Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <david@fromorbit.com>

Index: xfs/fs/xfs/xfs_rename.c
===================================================================
--- xfs.orig/fs/xfs/xfs_rename.c	2008-12-03 23:26:34.000000000 +0100
+++ xfs/fs/xfs/xfs_rename.c	2008-12-03 23:29:09.000000000 +0100
@@ -42,31 +42,6 @@
 
 
 /*
- * Given an array of up to 4 inode pointers, unlock the pointed to inodes.
- * If there are fewer than 4 entries in the array, the empty entries will
- * be at the end and will have NULL pointers in them.
- */
-STATIC void
-xfs_rename_unlock4(
-	xfs_inode_t	**i_tab,
-	uint		lock_mode)
-{
-	int	i;
-
-	xfs_iunlock(i_tab[0], lock_mode);
-	for (i = 1; i < 4; i++) {
-		if (i_tab[i] == NULL)
-			break;
-
-		/*
-		 * Watch out for duplicate entries in the table.
-		 */
-		if (i_tab[i] != i_tab[i-1])
-			xfs_iunlock(i_tab[i], lock_mode);
-	}
-}
-
-/*
  * Enter all inodes for a rename transaction into a sorted array.
  */
 STATIC void
@@ -205,19 +180,6 @@ xfs_rename(
 	xfs_lock_inodes(inodes, num_inodes, XFS_ILOCK_EXCL);
 
 	/*
-	 * If we are using project inheritance, we only allow renames
-	 * into our tree when the project IDs are the same; else the
-	 * tree quota mechanism would be circumvented.
-	 */
-	if (unlikely((target_dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) &&
-		     (target_dp->i_d.di_projid != src_ip->i_d.di_projid))) {
-		error = XFS_ERROR(EXDEV);
-		xfs_rename_unlock4(inodes, XFS_ILOCK_EXCL);
-		xfs_trans_cancel(tp, cancel_flags);
-		goto std_return;
-	}
-
-	/*
 	 * Join all the inodes to the transaction. From this point on,
 	 * we can rely on either trans_commit or trans_cancel to unlock
 	 * them.  Note that we need to add a vnode reference to the
@@ -242,6 +204,17 @@ xfs_rename(
 	}
 
 	/*
+	 * If we are using project inheritance, we only allow renames
+	 * into our tree when the project IDs are the same; else the
+	 * tree quota mechanism would be circumvented.
+	 */
+	if (unlikely((target_dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) &&
+		     (target_dp->i_d.di_projid != src_ip->i_d.di_projid))) {
+		error = XFS_ERROR(EXDEV);
+		goto error_return;
+	}
+
+	/*
 	 * Set up the target.
 	 */
 	if (target_ip == NULL) {

-- 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [patch 3/5] resync headers with libxfs
  2008-12-09  9:47 [patch 0/5] Patch queue for 2.6.29 Christoph Hellwig
  2008-12-09  9:47 ` [patch 1/5] replace b_fspriv with b_mount Christoph Hellwig
  2008-12-09  9:47 ` [patch 2/5] simplify projid check in xfs_rename Christoph Hellwig
@ 2008-12-09  9:47 ` Christoph Hellwig
  2008-12-10  3:28   ` Lachlan McIlroy
  2008-12-09  9:47 ` [patch 4/5] add a FMODE flag to make XFS invisible I/O less hacky Christoph Hellwig
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2008-12-09  9:47 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-resync-with-xfs-cmds --]
[-- Type: text/plain, Size: 3814 bytes --]

 - xfs_sb.h add the XFS_SB_VERSION2_PARENTBIT features2 that has been
   around in userspace for some time
 - xfs_inode.h: move a few things out of __KERNEL__ that are needed by
   userspace
 - xfs_mount.h: only include xfs_sync.h under __KERNEL__
 - xfs_inode.c: minor whitespace fixup.  I accidentaly changes this when
   importing this file for use by userspace.


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

Index: xfs/fs/xfs/xfs_sb.h
===================================================================
--- xfs.orig/fs/xfs/xfs_sb.h	2008-12-04 08:03:24.000000000 +0100
+++ xfs/fs/xfs/xfs_sb.h	2008-12-06 13:22:03.000000000 +0100
@@ -79,6 +79,7 @@ struct xfs_mount;
 #define XFS_SB_VERSION2_LAZYSBCOUNTBIT	0x00000002	/* Superblk counters */
 #define XFS_SB_VERSION2_RESERVED4BIT	0x00000004
 #define XFS_SB_VERSION2_ATTR2BIT	0x00000008	/* Inline attr rework */
+#define XFS_SB_VERSION2_PARENTBIT	0x00000010	/* parent pointers */
 
 #define	XFS_SB_VERSION2_OKREALFBITS	\
 	(XFS_SB_VERSION2_LAZYSBCOUNTBIT	| \
Index: xfs/fs/xfs/xfs_inode.h
===================================================================
--- xfs.orig/fs/xfs/xfs_inode.h	2008-12-05 17:19:55.000000000 +0100
+++ xfs/fs/xfs/xfs_inode.h	2008-12-06 13:22:35.000000000 +0100
@@ -483,12 +483,6 @@ static inline void xfs_ifunlock(xfs_inod
 	 ((pip)->i_d.di_mode & S_ISGID))
 
 /*
- * Flags for xfs_iget()
- */
-#define XFS_IGET_CREATE		0x1
-#define XFS_IGET_BULKSTAT	0x2
-
-/*
  * xfs_iget.c prototypes.
  */
 xfs_inode_t	*xfs_inode_incore(struct xfs_mount *, xfs_ino_t,
@@ -509,8 +503,6 @@ void		xfs_ireclaim(xfs_inode_t *);
 /*
  * xfs_inode.c prototypes.
  */
-int		xfs_iread(struct xfs_mount *, struct xfs_trans *,
-			  struct xfs_inode *, xfs_daddr_t, uint);
 int		xfs_ialloc(struct xfs_trans *, xfs_inode_t *, mode_t,
 			   xfs_nlink_t, xfs_dev_t, struct cred *, xfs_prid_t,
 			   int, struct xfs_buf **, boolean_t *, xfs_inode_t **);
@@ -583,12 +575,20 @@ do { \
 
 #endif /* __KERNEL__ */
 
+/*
+ * Flags for xfs_iget()
+ */
+#define XFS_IGET_CREATE		0x1
+#define XFS_IGET_BULKSTAT	0x2
+
 int		xfs_inotobp(struct xfs_mount *, struct xfs_trans *,
 			    xfs_ino_t, struct xfs_dinode **,
 			    struct xfs_buf **, int *, uint);
 int		xfs_itobp(struct xfs_mount *, struct xfs_trans *,
 			  struct xfs_inode *, struct xfs_dinode **,
 			  struct xfs_buf **, uint);
+int		xfs_iread(struct xfs_mount *, struct xfs_trans *,
+			  struct xfs_inode *, xfs_daddr_t, uint);
 void		xfs_dinode_from_disk(struct xfs_icdinode *,
 				     struct xfs_dinode *);
 void		xfs_dinode_to_disk(struct xfs_dinode *,
Index: xfs/fs/xfs/xfs_inode.c
===================================================================
--- xfs.orig/fs/xfs/xfs_inode.c	2008-12-04 14:50:44.000000000 +0100
+++ xfs/fs/xfs/xfs_inode.c	2008-12-06 13:24:23.000000000 +0100
@@ -870,7 +870,7 @@ xfs_iread(
 	 * around for a while.  This helps to keep recently accessed
 	 * meta-data in-core longer.
 	 */
-	 XFS_BUF_SET_REF(bp, XFS_INO_REF);
+	XFS_BUF_SET_REF(bp, XFS_INO_REF);
 
 	/*
 	 * Use xfs_trans_brelse() to release the buffer containing the
Index: xfs/fs/xfs/xfs_mount.h
===================================================================
--- xfs.orig/fs/xfs/xfs_mount.h	2008-12-05 17:19:56.000000000 +0100
+++ xfs/fs/xfs/xfs_mount.h	2008-12-06 13:31:27.000000000 +0100
@@ -18,8 +18,6 @@
 #ifndef __XFS_MOUNT_H__
 #define	__XFS_MOUNT_H__
 
-#include "xfs_sync.h"
-
 typedef struct xfs_trans_reservations {
 	uint	tr_write;	/* extent alloc trans */
 	uint	tr_itruncate;	/* truncate trans */
@@ -53,6 +51,8 @@ typedef struct xfs_trans_reservations {
 
 #else /* __KERNEL__ */
 
+#include "xfs_sync.h"
+
 struct cred;
 struct log;
 struct xfs_mount_args;

-- 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [patch 4/5] add a FMODE flag to make XFS invisible I/O less hacky
  2008-12-09  9:47 [patch 0/5] Patch queue for 2.6.29 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2008-12-09  9:47 ` [patch 3/5] resync headers with libxfs Christoph Hellwig
@ 2008-12-09  9:47 ` Christoph Hellwig
  2008-12-10  3:27   ` Lachlan McIlroy
  2008-12-09  9:47 ` [patch 5/5] use inode_change_ok for setattr permission checking Christoph Hellwig
  2008-12-09  9:57 ` [patch 0/5] Patch queue for 2.6.29 Martin Steigerwald
  5 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2008-12-09  9:47 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-add-FMODE_INVISIBLE --]
[-- Type: text/plain, Size: 12980 bytes --]

XFS has a mode called invisble I/O that doesn't update any of the               
timestamps.  It's used for HSM-style applications and exposed through           
the nasty open by handle ioctl.

Instead of doing directly assignment of file operations that set an             
internal flag for it add a new FMODE_NOCMTIME flag that we can check           
in the normal file operations.                                                  

(addition of the generic VFS flag has been ACKed by Al as an interims
 solution)
                                                                                

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

Index: xfs/fs/xfs/linux-2.6/xfs_ioctl32.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_ioctl32.c	2008-12-04 14:15:20.000000000 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_ioctl32.c	2008-12-04 14:30:21.000000000 +0100
@@ -599,19 +599,24 @@ out:
 	return error;
 }
 
-STATIC long
-xfs_compat_ioctl(
-	xfs_inode_t	*ip,
-	struct file	*filp,
-	int		ioflags,
-	unsigned	cmd,
-	void		__user *arg)
+long
+xfs_file_compat_ioctl(
+	struct file		*filp,
+	unsigned		cmd,
+	unsigned long		p)
 {
-	struct inode	*inode = filp->f_path.dentry->d_inode;
-	xfs_mount_t	*mp = ip->i_mount;
-	int		error;
+	struct inode		*inode = filp->f_path.dentry->d_inode;
+	struct xfs_inode	*ip = XFS_I(inode);
+	struct xfs_mount	*mp = ip->i_mount;
+	void			__user *arg = (void __user *)p;
+	int			ioflags = 0;
+	int			error;
+
+	if (filp->f_mode & FMODE_NOCMTIME)
+		ioflags |= IO_INVIS;
+
+	xfs_itrace_entry(ip);
 
-	xfs_itrace_entry(XFS_I(inode));
 	switch (cmd) {
 	/* No size or alignment issues on any arch */
 	case XFS_IOC_DIOINFO:
@@ -632,7 +637,7 @@ xfs_compat_ioctl(
 	case XFS_IOC_GOINGDOWN:
 	case XFS_IOC_ERROR_INJECTION:
 	case XFS_IOC_ERROR_CLEARALL:
-		return xfs_ioctl(ip, filp, ioflags, cmd, arg);
+		return xfs_file_ioctl(filp, cmd, p);
 #ifndef BROKEN_X86_ALIGNMENT
 	/* These are handled fine if no alignment issues */
 	case XFS_IOC_ALLOCSP:
@@ -646,7 +651,7 @@ xfs_compat_ioctl(
 	case XFS_IOC_FSGEOMETRY_V1:
 	case XFS_IOC_FSGROWFSDATA:
 	case XFS_IOC_FSGROWFSRT:
-		return xfs_ioctl(ip, filp, ioflags, cmd, arg);
+		return xfs_file_ioctl(filp, cmd, p);
 #else
 	case XFS_IOC_ALLOCSP_32:
 	case XFS_IOC_FREESP_32:
@@ -687,7 +692,7 @@ xfs_compat_ioctl(
 	case XFS_IOC_SETXFLAGS_32:
 	case XFS_IOC_GETVERSION_32:
 		cmd = _NATIVE_IOC(cmd, long);
-		return xfs_ioctl(ip, filp, ioflags, cmd, arg);
+		return xfs_file_ioctl(filp, cmd, p);
 	case XFS_IOC_SWAPEXT: {
 		struct xfs_swapext	  sxp;
 		struct compat_xfs_swapext __user *sxu = arg;
@@ -738,26 +743,3 @@ xfs_compat_ioctl(
 		return -XFS_ERROR(ENOIOCTLCMD);
 	}
 }
-
-long
-xfs_file_compat_ioctl(
-	struct file		*filp,
-	unsigned int		cmd,
-	unsigned long		p)
-{
-	struct inode	*inode = filp->f_path.dentry->d_inode;
-
-	return xfs_compat_ioctl(XFS_I(inode), filp, 0, cmd, (void __user *)p);
-}
-
-long
-xfs_file_compat_invis_ioctl(
-	struct file		*filp,
-	unsigned int		cmd,
-	unsigned long		p)
-{
-	struct inode	*inode = filp->f_path.dentry->d_inode;
-
-	return xfs_compat_ioctl(XFS_I(inode), filp, IO_INVIS, cmd,
-				(void __user *)p);
-}
Index: xfs/fs/xfs/linux-2.6/xfs_file.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_file.c	2008-12-04 14:15:20.000000000 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_file.c	2008-12-04 14:30:21.000000000 +0100
@@ -45,81 +45,45 @@
 
 static struct vm_operations_struct xfs_file_vm_ops;
 
-STATIC_INLINE ssize_t
-__xfs_file_read(
+STATIC ssize_t
+xfs_file_aio_read(
 	struct kiocb		*iocb,
 	const struct iovec	*iov,
 	unsigned long		nr_segs,
-	int			ioflags,
 	loff_t			pos)
 {
 	struct file		*file = iocb->ki_filp;
+	int			ioflags = IO_ISAIO;
 
 	BUG_ON(iocb->ki_pos != pos);
 	if (unlikely(file->f_flags & O_DIRECT))
 		ioflags |= IO_ISDIRECT;
+	if (file->f_mode & FMODE_NOCMTIME)
+		ioflags |= IO_INVIS;
 	return xfs_read(XFS_I(file->f_path.dentry->d_inode), iocb, iov,
 				nr_segs, &iocb->ki_pos, ioflags);
 }
 
 STATIC ssize_t
-xfs_file_aio_read(
-	struct kiocb		*iocb,
-	const struct iovec	*iov,
-	unsigned long		nr_segs,
-	loff_t			pos)
-{
-	return __xfs_file_read(iocb, iov, nr_segs, IO_ISAIO, pos);
-}
-
-STATIC ssize_t
-xfs_file_aio_read_invis(
-	struct kiocb		*iocb,
-	const struct iovec	*iov,
-	unsigned long		nr_segs,
-	loff_t			pos)
-{
-	return __xfs_file_read(iocb, iov, nr_segs, IO_ISAIO|IO_INVIS, pos);
-}
-
-STATIC_INLINE ssize_t
-__xfs_file_write(
+xfs_file_aio_write(
 	struct kiocb		*iocb,
 	const struct iovec	*iov,
 	unsigned long		nr_segs,
-	int			ioflags,
 	loff_t			pos)
 {
-	struct file	*file = iocb->ki_filp;
+	struct file		*file = iocb->ki_filp;
+	int			ioflags = IO_ISAIO;
 
 	BUG_ON(iocb->ki_pos != pos);
 	if (unlikely(file->f_flags & O_DIRECT))
 		ioflags |= IO_ISDIRECT;
+	if (file->f_mode & FMODE_NOCMTIME)
+		ioflags |= IO_INVIS;
 	return xfs_write(XFS_I(file->f_mapping->host), iocb, iov, nr_segs,
 				&iocb->ki_pos, ioflags);
 }
 
 STATIC ssize_t
-xfs_file_aio_write(
-	struct kiocb		*iocb,
-	const struct iovec	*iov,
-	unsigned long		nr_segs,
-	loff_t			pos)
-{
-	return __xfs_file_write(iocb, iov, nr_segs, IO_ISAIO, pos);
-}
-
-STATIC ssize_t
-xfs_file_aio_write_invis(
-	struct kiocb		*iocb,
-	const struct iovec	*iov,
-	unsigned long		nr_segs,
-	loff_t			pos)
-{
-	return __xfs_file_write(iocb, iov, nr_segs, IO_ISAIO|IO_INVIS, pos);
-}
-
-STATIC ssize_t
 xfs_file_splice_read(
 	struct file		*infilp,
 	loff_t			*ppos,
@@ -127,20 +91,13 @@ xfs_file_splice_read(
 	size_t			len,
 	unsigned int		flags)
 {
-	return xfs_splice_read(XFS_I(infilp->f_path.dentry->d_inode),
-				   infilp, ppos, pipe, len, flags, 0);
-}
+	int			ioflags = 0;
+
+	if (infilp->f_mode & FMODE_NOCMTIME)
+		ioflags |= IO_INVIS;
 
-STATIC ssize_t
-xfs_file_splice_read_invis(
-	struct file		*infilp,
-	loff_t			*ppos,
-	struct pipe_inode_info	*pipe,
-	size_t			len,
-	unsigned int		flags)
-{
 	return xfs_splice_read(XFS_I(infilp->f_path.dentry->d_inode),
-				   infilp, ppos, pipe, len, flags, IO_INVIS);
+				   infilp, ppos, pipe, len, flags, ioflags);
 }
 
 STATIC ssize_t
@@ -151,20 +108,13 @@ xfs_file_splice_write(
 	size_t			len,
 	unsigned int		flags)
 {
-	return xfs_splice_write(XFS_I(outfilp->f_path.dentry->d_inode),
-				    pipe, outfilp, ppos, len, flags, 0);
-}
+	int			ioflags = 0;
+
+	if (outfilp->f_mode & FMODE_NOCMTIME)
+		ioflags |= IO_INVIS;
 
-STATIC ssize_t
-xfs_file_splice_write_invis(
-	struct pipe_inode_info	*pipe,
-	struct file		*outfilp,
-	loff_t			*ppos,
-	size_t			len,
-	unsigned int		flags)
-{
 	return xfs_splice_write(XFS_I(outfilp->f_path.dentry->d_inode),
-				    pipe, outfilp, ppos, len, flags, IO_INVIS);
+				    pipe, outfilp, ppos, len, flags, ioflags);
 }
 
 STATIC int
@@ -275,42 +225,6 @@ xfs_file_mmap(
 	return 0;
 }
 
-STATIC long
-xfs_file_ioctl(
-	struct file	*filp,
-	unsigned int	cmd,
-	unsigned long	p)
-{
-	struct inode	*inode = filp->f_path.dentry->d_inode;
-
-
-	/* NOTE:  some of the ioctl's return positive #'s as a
-	 *	  byte count indicating success, such as
-	 *	  readlink_by_handle.  So we don't "sign flip"
-	 *	  like most other routines.  This means true
-	 *	  errors need to be returned as a negative value.
-	 */
-	return xfs_ioctl(XFS_I(inode), filp, 0, cmd, (void __user *)p);
-}
-
-STATIC long
-xfs_file_ioctl_invis(
-	struct file	*filp,
-	unsigned int	cmd,
-	unsigned long	p)
-{
-	struct inode	*inode = filp->f_path.dentry->d_inode;
-
-
-	/* NOTE:  some of the ioctl's return positive #'s as a
-	 *	  byte count indicating success, such as
-	 *	  readlink_by_handle.  So we don't "sign flip"
-	 *	  like most other routines.  This means true
-	 *	  errors need to be returned as a negative value.
-	 */
-	return xfs_ioctl(XFS_I(inode), filp, IO_INVIS, cmd, (void __user *)p);
-}
-
 /*
  * mmap()d file has taken write protection fault and is being made
  * writable. We can set the page state up correctly for a writable
@@ -346,25 +260,6 @@ const struct file_operations xfs_file_op
 #endif
 };
 
-const struct file_operations xfs_invis_file_operations = {
-	.llseek		= generic_file_llseek,
-	.read		= do_sync_read,
-	.write		= do_sync_write,
-	.aio_read	= xfs_file_aio_read_invis,
-	.aio_write	= xfs_file_aio_write_invis,
-	.splice_read	= xfs_file_splice_read_invis,
-	.splice_write	= xfs_file_splice_write_invis,
-	.unlocked_ioctl	= xfs_file_ioctl_invis,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl	= xfs_file_compat_invis_ioctl,
-#endif
-	.mmap		= xfs_file_mmap,
-	.open		= xfs_file_open,
-	.release	= xfs_file_release,
-	.fsync		= xfs_file_fsync,
-};
-
-
 const struct file_operations xfs_dir_file_operations = {
 	.open		= xfs_dir_open,
 	.read		= generic_read_dir,
Index: xfs/fs/xfs/linux-2.6/xfs_ioctl.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_ioctl.c	2008-12-04 14:15:20.000000000 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_ioctl.c	2008-12-04 14:30:21.000000000 +0100
@@ -319,10 +319,11 @@ xfs_open_by_handle(
 		put_unused_fd(new_fd);
 		return -XFS_ERROR(-PTR_ERR(filp));
 	}
+
 	if (inode->i_mode & S_IFREG) {
 		/* invisible operation should not change atime */
 		filp->f_flags |= O_NOATIME;
-		filp->f_op = &xfs_invis_file_operations;
+		filp->f_mode |= FMODE_NOCMTIME;
 	}
 
 	fd_install(new_fd, filp);
@@ -1328,21 +1329,31 @@ xfs_ioc_getbmapx(
 	return 0;
 }
 
-int
-xfs_ioctl(
-	xfs_inode_t		*ip,
+/*
+ * Note: some of the ioctl's return positive numbers as a
+ * byte count indicating success, such as readlink_by_handle.
+ * So we don't "sign flip" like most other routines.  This means
+ * true errors need to be returned as a negative value.
+ */
+long
+xfs_file_ioctl(
 	struct file		*filp,
-	int			ioflags,
 	unsigned int		cmd,
-	void			__user *arg)
+	unsigned long		p)
 {
 	struct inode		*inode = filp->f_path.dentry->d_inode;
-	xfs_mount_t		*mp = ip->i_mount;
+	struct xfs_inode	*ip = XFS_I(inode);
+	struct xfs_mount	*mp = ip->i_mount;
+	void			__user *arg = (void __user *)p;
+	int			ioflags = 0;
 	int			error;
 
-	xfs_itrace_entry(XFS_I(inode));
-	switch (cmd) {
+	if (filp->f_mode & FMODE_NOCMTIME)
+		ioflags |= IO_INVIS;
 
+	xfs_itrace_entry(ip);
+
+	switch (cmd) {
 	case XFS_IOC_ALLOCSP:
 	case XFS_IOC_FREESP:
 	case XFS_IOC_RESVSP:
Index: xfs/fs/xfs/linux-2.6/xfs_iops.h
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_iops.h	2008-12-04 14:15:20.000000000 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_iops.h	2008-12-04 14:30:21.000000000 +0100
@@ -22,7 +22,6 @@ struct xfs_inode;
 
 extern const struct file_operations xfs_file_operations;
 extern const struct file_operations xfs_dir_file_operations;
-extern const struct file_operations xfs_invis_file_operations;
 
 extern ssize_t xfs_vn_listxattr(struct dentry *, char *data, size_t size);
 
Index: xfs/include/linux/fs.h
===================================================================
--- xfs.orig/include/linux/fs.h	2008-12-04 14:15:20.000000000 +0100
+++ xfs/include/linux/fs.h	2008-12-04 14:30:21.000000000 +0100
@@ -81,6 +81,14 @@ extern int dir_notify_enable;
 #define FMODE_WRITE_IOCTL	((__force fmode_t)128)
 #define FMODE_NDELAY_NOW	((__force fmode_t)256)
 
+/*
+ * Don't update ctime and mtime.
+ *
+ * Currently a special hack for the XFS open_by_handle ioctl, but we'll
+ * hopefully graduate it to a proper O_CMTIME flag supported by open(2) soon.
+ */
+#define FMODE_NOCMTIME		((__force fmode_t)2048)
+
 #define RW_MASK		1
 #define RWA_MASK	2
 #define READ 0
Index: xfs/fs/xfs/linux-2.6/xfs_ioctl.h
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_ioctl.h	2008-12-04 14:19:38.000000000 +0100
+++ xfs/fs/xfs/linux-2.6/xfs_ioctl.h	2008-12-04 14:30:51.000000000 +0100
@@ -68,13 +68,13 @@ xfs_attrmulti_attr_remove(
 	__uint32_t		flags);
 
 extern long
-xfs_file_compat_ioctl(
-	struct file		*file,
+xfs_file_ioctl(
+	struct file		*filp,
 	unsigned int		cmd,
-	unsigned long		arg);
+	unsigned long		p);
 
 extern long
-xfs_file_compat_invis_ioctl(
+xfs_file_compat_ioctl(
 	struct file		*file,
 	unsigned int		cmd,
 	unsigned long		arg);
Index: xfs/fs/xfs/xfs_vnodeops.h
===================================================================
--- xfs.orig/fs/xfs/xfs_vnodeops.h	2008-12-04 14:15:20.000000000 +0100
+++ xfs/fs/xfs/xfs_vnodeops.h	2008-12-04 14:30:21.000000000 +0100
@@ -53,8 +53,6 @@ int xfs_attr_set(struct xfs_inode *dp, c
 int xfs_attr_remove(struct xfs_inode *dp, const char *name, int flags);
 int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
 		int flags, struct attrlist_cursor_kern *cursor);
-int xfs_ioctl(struct xfs_inode *ip, struct file *filp,
-		int ioflags, unsigned int cmd, void __user *arg);
 ssize_t xfs_read(struct xfs_inode *ip, struct kiocb *iocb,
 		const struct iovec *iovp, unsigned int segs,
 		loff_t *offset, int ioflags);

-- 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [patch 5/5] use inode_change_ok for setattr permission checking
  2008-12-09  9:47 [patch 0/5] Patch queue for 2.6.29 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2008-12-09  9:47 ` [patch 4/5] add a FMODE flag to make XFS invisible I/O less hacky Christoph Hellwig
@ 2008-12-09  9:47 ` Christoph Hellwig
  2008-12-10  3:40   ` Lachlan McIlroy
  2008-12-10  5:09   ` Timothy Shimmin
  2008-12-09  9:57 ` [patch 0/5] Patch queue for 2.6.29 Martin Steigerwald
  5 siblings, 2 replies; 15+ messages in thread
From: Christoph Hellwig @ 2008-12-09  9:47 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-setattr-use-inode_change_ok --]
[-- Type: text/plain, Size: 6531 bytes --]

Instead of implementing our own checks use inode_change_ok to check for
necessary permission in setattr.  There is a slight change in behaviour
as inode_change_ok doesn't allow i_mode updates to add the suid or sgid
without superuser privilegues while the old XFS code just stripped away
those bits from the file mode.

(First sent on Semptember 29th)


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

Index: xfs-master/fs/xfs/xfs_vnodeops.c
===================================================================
--- xfs-master.orig/fs/xfs/xfs_vnodeops.c	2008-12-02 14:00:52.000000000 +0100
+++ xfs-master/fs/xfs/xfs_vnodeops.c	2008-12-02 14:00:52.000000000 +0100
@@ -70,7 +70,6 @@ xfs_setattr(
 	gid_t			gid=0, igid=0;
 	int			timeflags = 0;
 	struct xfs_dquot	*udqp, *gdqp, *olddquot1, *olddquot2;
-	int			file_owner;
 	int			need_iolock = 1;
 
 	xfs_itrace_entry(ip);
@@ -81,6 +80,10 @@ xfs_setattr(
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return XFS_ERROR(EIO);
 
+	code = -inode_change_ok(inode, iattr);
+	if (code)
+		return code;
+
 	olddquot1 = olddquot2 = NULL;
 	udqp = gdqp = NULL;
 
@@ -158,56 +161,6 @@ xfs_setattr(
 
 	xfs_ilock(ip, lock_flags);
 
-	/* boolean: are we the file owner? */
-	file_owner = (current_fsuid() == ip->i_d.di_uid);
-
-	/*
-	 * Change various properties of a file.
-	 * Only the owner or users with CAP_FOWNER
-	 * capability may do these things.
-	 */
-	if (mask & (ATTR_MODE|ATTR_UID|ATTR_GID)) {
-		/*
-		 * CAP_FOWNER overrides the following restrictions:
-		 *
-		 * The user ID of the calling process must be equal
-		 * to the file owner ID, except in cases where the
-		 * CAP_FSETID capability is applicable.
-		 */
-		if (!file_owner && !capable(CAP_FOWNER)) {
-			code = XFS_ERROR(EPERM);
-			goto error_return;
-		}
-
-		/*
-		 * CAP_FSETID overrides the following restrictions:
-		 *
-		 * The effective user ID of the calling process shall match
-		 * the file owner when setting the set-user-ID and
-		 * set-group-ID bits on that file.
-		 *
-		 * The effective group ID or one of the supplementary group
-		 * IDs of the calling process shall match the group owner of
-		 * the file when setting the set-group-ID bit on that file
-		 */
-		if (mask & ATTR_MODE) {
-			mode_t m = 0;
-
-			if ((iattr->ia_mode & S_ISUID) && !file_owner)
-				m |= S_ISUID;
-			if ((iattr->ia_mode & S_ISGID) &&
-			    !in_group_p((gid_t)ip->i_d.di_gid))
-				m |= S_ISGID;
-#if 0
-			/* Linux allows this, Irix doesn't. */
-			if ((iattr->ia_mode & S_ISVTX) && !S_ISDIR(ip->i_d.di_mode))
-				m |= S_ISVTX;
-#endif
-			if (m && !capable(CAP_FSETID))
-				iattr->ia_mode &= ~m;
-		}
-	}
-
 	/*
 	 * Change file ownership.  Must be the owner or privileged.
 	 */
@@ -224,22 +177,6 @@ xfs_setattr(
 		uid = (mask & ATTR_UID) ? iattr->ia_uid : iuid;
 
 		/*
-		 * CAP_CHOWN overrides the following restrictions:
-		 *
-		 * If _POSIX_CHOWN_RESTRICTED is defined, this capability
-		 * shall override the restriction that a process cannot
-		 * change the user ID of a file it owns and the restriction
-		 * that the group ID supplied to the chown() function
-		 * shall be equal to either the group ID or one of the
-		 * supplementary group IDs of the calling process.
-		 */
-		if ((iuid != uid ||
-		     (igid != gid && !in_group_p((gid_t)gid))) &&
-		    !capable(CAP_CHOWN)) {
-			code = XFS_ERROR(EPERM);
-			goto error_return;
-		}
-		/*
 		 * Do a quota reservation only if uid/gid is actually
 		 * going to change.
 		 */
@@ -276,36 +213,22 @@ xfs_setattr(
 			code = XFS_ERROR(EINVAL);
 			goto error_return;
 		}
+
 		/*
 		 * Make sure that the dquots are attached to the inode.
 		 */
-		if ((code = XFS_QM_DQATTACH(mp, ip, XFS_QMOPT_ILOCKED)))
+		code = XFS_QM_DQATTACH(mp, ip, XFS_QMOPT_ILOCKED);
+		if (code)
 			goto error_return;
-	}
-
-	/*
-	 * Change file access or modified times.
-	 */
-	if (mask & (ATTR_ATIME|ATTR_MTIME)) {
-		if (!file_owner) {
-			if ((mask & (ATTR_MTIME_SET|ATTR_ATIME_SET)) &&
-			    !capable(CAP_FOWNER)) {
-				code = XFS_ERROR(EPERM);
-				goto error_return;
-			}
-		}
-	}
 
-	/*
-	 * Now we can make the changes.  Before we join the inode
-	 * to the transaction, if ATTR_SIZE is set then take care of
-	 * the part of the truncation that must be done without the
-	 * inode lock.  This needs to be done before joining the inode
-	 * to the transaction, because the inode cannot be unlocked
-	 * once it is a part of the transaction.
-	 */
-	if (mask & ATTR_SIZE) {
-		code = 0;
+		/*
+		 * Now we can make the changes.  Before we join the inode
+		 * to the transaction, if ATTR_SIZE is set then take care of
+		 * the part of the truncation that must be done without the
+		 * inode lock.  This needs to be done before joining the inode
+		 * to the transaction, because the inode cannot be unlocked
+		 * once it is a part of the transaction.
+		 */
 		if (iattr->ia_size > ip->i_size) {
 			/*
 			 * Do the first part of growing a file: zero any data
@@ -360,17 +283,10 @@ xfs_setattr(
 		}
 		commit_flags = XFS_TRANS_RELEASE_LOG_RES;
 		xfs_ilock(ip, XFS_ILOCK_EXCL);
-	}
 
-	if (tp) {
 		xfs_trans_ijoin(tp, ip, lock_flags);
 		xfs_trans_ihold(tp, ip);
-	}
 
-	/*
-	 * Truncate file.  Must have write permission and not be a directory.
-	 */
-	if (mask & ATTR_SIZE) {
 		/*
 		 * Only change the c/mtime if we are changing the size
 		 * or we are explicitly asked to change it. This handles
@@ -410,20 +326,9 @@ xfs_setattr(
 			 */
 			xfs_iflags_set(ip, XFS_ITRUNCATED);
 		}
-	}
-
-	/*
-	 * Change file access modes.
-	 */
-	if (mask & ATTR_MODE) {
-		ip->i_d.di_mode &= S_IFMT;
-		ip->i_d.di_mode |= iattr->ia_mode & ~S_IFMT;
-
-		inode->i_mode &= S_IFMT;
-		inode->i_mode |= iattr->ia_mode & ~S_IFMT;
-
-		xfs_trans_log_inode (tp, ip, XFS_ILOG_CORE);
-		timeflags |= XFS_ICHGTIME_CHG;
+	} else if (tp) {
+		xfs_trans_ijoin(tp, ip, lock_flags);
+		xfs_trans_ihold(tp, ip);
 	}
 
 	/*
@@ -471,6 +376,24 @@ xfs_setattr(
 		timeflags |= XFS_ICHGTIME_CHG;
 	}
 
+	/*
+	 * Change file access modes.
+	 */
+	if (mask & ATTR_MODE) {
+		umode_t mode = iattr->ia_mode;
+
+		if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID))
+			mode &= ~S_ISGID;
+
+		ip->i_d.di_mode &= S_IFMT;
+		ip->i_d.di_mode |= mode & ~S_IFMT;
+
+		inode->i_mode &= S_IFMT;
+		inode->i_mode |= mode & ~S_IFMT;
+
+		xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
+		timeflags |= XFS_ICHGTIME_CHG;
+	}
 
 	/*
 	 * Change file access or modified times.

-- 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 1/5] replace b_fspriv with b_mount
  2008-12-09  9:47 ` [patch 1/5] replace b_fspriv with b_mount Christoph Hellwig
@ 2008-12-09  9:56   ` Martin Steigerwald
  2008-12-09 20:04   ` Russell Cattelan
  1 sibling, 0 replies; 15+ messages in thread
From: Martin Steigerwald @ 2008-12-09  9:56 UTC (permalink / raw)
  To: linux-xfs

Am Dienstag 09 Dezember 2008 schrieb Christoph Hellwig:
> Replace the b_fspriv pointer and it's ugly accessors with a properly
> types xfs_mount pointer.  Also switch log reocvery over to it instead
> of using b_fspriv for the mount pointer.

s/reocvery/recovery

Can't comment on code ;)

-- 
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 0/5] Patch queue for 2.6.29
  2008-12-09  9:47 [patch 0/5] Patch queue for 2.6.29 Christoph Hellwig
                   ` (4 preceding siblings ...)
  2008-12-09  9:47 ` [patch 5/5] use inode_change_ok for setattr permission checking Christoph Hellwig
@ 2008-12-09  9:57 ` Martin Steigerwald
  2008-12-09 10:07   ` Christoph Hellwig
  5 siblings, 1 reply; 15+ messages in thread
From: Martin Steigerwald @ 2008-12-09  9:57 UTC (permalink / raw)
  To: linux-xfs

Am Dienstag 09 Dezember 2008 schrieb Christoph Hellwig:
> These are my outstanding patches for 2.6.29.  The first two have
> already been rewied by Dave, the others still need review.

Patch 4/5 appears to be missing.

-- 
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 0/5] Patch queue for 2.6.29
  2008-12-09  9:57 ` [patch 0/5] Patch queue for 2.6.29 Martin Steigerwald
@ 2008-12-09 10:07   ` Christoph Hellwig
  2008-12-09 11:02     ` Martin Steigerwald
  0 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2008-12-09 10:07 UTC (permalink / raw)
  To: Martin Steigerwald; +Cc: linux-xfs

On Tue, Dec 09, 2008 at 10:57:48AM +0100, Martin Steigerwald wrote:
> Am Dienstag 09 Dezember 2008 schrieb Christoph Hellwig:
> > These are my outstanding patches for 2.6.29.  The first two have
> > already been rewied by Dave, the others still need review.
> 
> Patch 4/5 appears to be missing.

It's now there for me, unfortunately the oss has some rather
non-deterministic delays..

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 0/5] Patch queue for 2.6.29
  2008-12-09 10:07   ` Christoph Hellwig
@ 2008-12-09 11:02     ` Martin Steigerwald
  0 siblings, 0 replies; 15+ messages in thread
From: Martin Steigerwald @ 2008-12-09 11:02 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

Am Dienstag 09 Dezember 2008 schrieb Christoph Hellwig:
> On Tue, Dec 09, 2008 at 10:57:48AM +0100, Martin Steigerwald wrote:
> > Am Dienstag 09 Dezember 2008 schrieb Christoph Hellwig:
> > > These are my outstanding patches for 2.6.29.  The first two have
> > > already been rewied by Dave, the others still need review.
> >
> > Patch 4/5 appears to be missing.
>
> It's now there for me, unfortunately the oss has some rather
> non-deterministic delays..

It appeared. I didn't wait long enough for it to appear. Sorry.

-- 
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 1/5] replace b_fspriv with b_mount
  2008-12-09  9:47 ` [patch 1/5] replace b_fspriv with b_mount Christoph Hellwig
  2008-12-09  9:56   ` Martin Steigerwald
@ 2008-12-09 20:04   ` Russell Cattelan
  1 sibling, 0 replies; 15+ messages in thread
From: Russell Cattelan @ 2008-12-09 20:04 UTC (permalink / raw)
  To: xfs-oss

Christoph Hellwig wrote:

 >  Replace the b_fspriv pointer and it's ugly accessors with a properly 
 > types
 > xfs_mount pointer.  Also switch log reocvery over to it instead of using
 > b_fspriv for the mount pointer.


Not that I object to cleaning this stuff up, (it's messy and hard to follow)

But currently the code maps well on freebsd since it has a
b_fsprivate1-3 buf field and
the macros handle the setting reading currently.

Granted I could simply un-apply this change for freebsd  but I would
prefer we
keep un-needed differences to a minimum.

I would prefer if we came up with a structure that could be attached to
an opaque  b_data field
that would be able to hold all current private info.

Currently it looks like b_strat is used for wrapper functions that
either check the mp for shutdown or
iclog->ic_state & XLOG_STATE_IOERROR.
Maybe we can collapse b_strat,  fspriv2/3 into one structure that would
have something like a
can_io function that would either allow or disallow io.

We could clean up more of the buf macros then.

-Russell



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 4/5] add a FMODE flag to make XFS invisible I/O less hacky
  2008-12-09  9:47 ` [patch 4/5] add a FMODE flag to make XFS invisible I/O less hacky Christoph Hellwig
@ 2008-12-10  3:27   ` Lachlan McIlroy
  0 siblings, 0 replies; 15+ messages in thread
From: Lachlan McIlroy @ 2008-12-10  3:27 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

Looks good.

Christoph Hellwig wrote:

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 3/5] resync headers with libxfs
  2008-12-09  9:47 ` [patch 3/5] resync headers with libxfs Christoph Hellwig
@ 2008-12-10  3:28   ` Lachlan McIlroy
  0 siblings, 0 replies; 15+ messages in thread
From: Lachlan McIlroy @ 2008-12-10  3:28 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

Looks fine to me.

Christoph Hellwig wrote:

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 5/5] use inode_change_ok for setattr permission checking
  2008-12-09  9:47 ` [patch 5/5] use inode_change_ok for setattr permission checking Christoph Hellwig
@ 2008-12-10  3:40   ` Lachlan McIlroy
  2008-12-10  5:09   ` Timothy Shimmin
  1 sibling, 0 replies; 15+ messages in thread
From: Lachlan McIlroy @ 2008-12-10  3:40 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

Looks fine to me.

Christoph Hellwig wrote:

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch 5/5] use inode_change_ok for setattr permission checking
  2008-12-09  9:47 ` [patch 5/5] use inode_change_ok for setattr permission checking Christoph Hellwig
  2008-12-10  3:40   ` Lachlan McIlroy
@ 2008-12-10  5:09   ` Timothy Shimmin
  1 sibling, 0 replies; 15+ messages in thread
From: Timothy Shimmin @ 2008-12-10  5:09 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

Christoph Hellwig wrote:
> Instead of implementing our own checks use inode_change_ok to check for
> necessary permission in setattr.  There is a slight change in behaviour
> as inode_change_ok doesn't allow i_mode updates to add the suid or sgid
> without superuser privilegues while the old XFS code just stripped away
> those bits from the file mode.
> 
> (First sent on Semptember 29th)
(No reply from Question on Nov 12th:)

I just wanted to run this thru an amended xfstests/193 first.

Thanks,
--Tim

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2008-12-10  5:09 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-12-09  9:47 [patch 0/5] Patch queue for 2.6.29 Christoph Hellwig
2008-12-09  9:47 ` [patch 1/5] replace b_fspriv with b_mount Christoph Hellwig
2008-12-09  9:56   ` Martin Steigerwald
2008-12-09 20:04   ` Russell Cattelan
2008-12-09  9:47 ` [patch 2/5] simplify projid check in xfs_rename Christoph Hellwig
2008-12-09  9:47 ` [patch 3/5] resync headers with libxfs Christoph Hellwig
2008-12-10  3:28   ` Lachlan McIlroy
2008-12-09  9:47 ` [patch 4/5] add a FMODE flag to make XFS invisible I/O less hacky Christoph Hellwig
2008-12-10  3:27   ` Lachlan McIlroy
2008-12-09  9:47 ` [patch 5/5] use inode_change_ok for setattr permission checking Christoph Hellwig
2008-12-10  3:40   ` Lachlan McIlroy
2008-12-10  5:09   ` Timothy Shimmin
2008-12-09  9:57 ` [patch 0/5] Patch queue for 2.6.29 Martin Steigerwald
2008-12-09 10:07   ` Christoph Hellwig
2008-12-09 11:02     ` Martin Steigerwald

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox