Skip to contentSkip to conversation

- remove mounpoint UUID code

RFC PATCH In review Eric Sandeen 5 participants 14 messages Mar 8, 2008 – Jul 25, 2008 XFS filesystem

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 - remove mounpoint UUID code

It looks like all of the below is unused... and according to Nathan,

"dont think it even got used/implemented anywhere, but i think it was meant to be an auto-mount kinda thing... such that when you look up at that point, it knows to mount the device with that uuid there, if its not already it was never really written anywhere ... just an idea in doug doucettes brain i think."

Think it'll ever go anywhere, or should it get pruned?

The below builds; not at all tested, until I get an idea if it's worth doing. Need to double check that some structures might not need padding out to keep things compatible/consistent...

 dmapi/xfs_dm.c    |    2 --
 xfs_attr_leaf.c   |    6 +-----
 xfs_bmap.c        |    4 ----
 xfs_dinode.h      |    2 --
 xfs_inode.c       |    8 --------
 xfs_inode.h       |    1 -
 xfs_inode_item.c  |   53 +++++++++++------------------------------------------
 xfs_inode_item.h  |   26 ++++++++------------------
 xfs_itable.c      |    2 --
 xfs_log_recover.c |   10 ++--------
 xfsidbg.c         |    2 +-
 11 files changed, 23 insertions(+), 93 deletions(-)
fs/xfs/dmapi/xfs_dm.c +0 -2
Index: linux-2.6-xfs/fs/xfs/dmapi/xfs_dm.c
===================================================================
@@ -363,7 +363,6 @@ xfs_dip_to_stat(
 		buf->dt_blocks = 0;
 		break;
 	case XFS_DINODE_FMT_LOCAL:
-	case XFS_DINODE_FMT_UUID:
 		buf->dt_rdev = 0;
 		buf->dt_blksize = mp->m_sb.sb_blocksize;
 		buf->dt_blocks = 0;
@@ -431,7 +430,6 @@ xfs_ip_to_stat(
 		buf->dt_blocks = 0;
 		break;
 	case XFS_DINODE_FMT_LOCAL:
-	case XFS_DINODE_FMT_UUID:
 		buf->dt_rdev = 0;
 		buf->dt_blksize = mp->m_sb.sb_blocksize;
 		buf->dt_blocks = 0;
fs/xfs/xfs_attr_leaf.c +1 -5
Index: linux-2.6-xfs/fs/xfs/xfs_attr_leaf.c
===================================================================
@@ -155,13 +155,9 @@ xfs_attr_shortform_bytesfit(xfs_inode_t 
 
 	offset = (XFS_LITINO(mp) - bytes) >> 3; /* rounded down */
 
-	switch (dp->i_d.di_format) {
-	case XFS_DINODE_FMT_DEV:
+	if (dp->i_d.di_format == XFS_DINODE_FMT_DEV) {
 		minforkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
 		return (offset >= minforkoff) ? minforkoff : 0;
-	case XFS_DINODE_FMT_UUID:
-		minforkoff = roundup(sizeof(uuid_t), 8) >> 3;
-		return (offset >= minforkoff) ? minforkoff : 0;
 	}
 
 	if (!(mp->m_flags & XFS_MOUNT_ATTR2)) {
fs/xfs/xfs_bmap.c +0 -4
Index: linux-2.6-xfs/fs/xfs/xfs_bmap.c
===================================================================
@@ -3532,7 +3532,6 @@ xfs_bmap_forkoff_reset(
 {
 	if (whichfork == XFS_ATTR_FORK &&
 	    (ip->i_d.di_format != XFS_DINODE_FMT_DEV) &&
-	    (ip->i_d.di_format != XFS_DINODE_FMT_UUID) &&
 	    (ip->i_d.di_format != XFS_DINODE_FMT_BTREE) &&
 	    ((mp->m_attroffset >> 3) > ip->i_d.di_forkoff)) {
 		ip->i_d.di_forkoff = mp->m_attroffset >> 3;
@@ -4000,9 +3999,6 @@ xfs_bmap_add_attrfork(
 	case XFS_DINODE_FMT_DEV:
 		ip->i_d.di_forkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
 		break;
-	case XFS_DINODE_FMT_UUID:
-		ip->i_d.di_forkoff = roundup(sizeof(uuid_t), 8) >> 3;
-		break;
 	case XFS_DINODE_FMT_LOCAL:
 	case XFS_DINODE_FMT_EXTENTS:
 	case XFS_DINODE_FMT_BTREE:
fs/xfs/xfs_dinode.h +0 -2
Index: linux-2.6-xfs/fs/xfs/xfs_dinode.h
===================================================================
@@ -88,7 +88,6 @@ typedef struct xfs_dinode
 		xfs_dir2_sf_t	di_dir2sf;	/* shortform directory v2 */
 		char		di_c[1];	/* local contents */
 		__be32		di_dev;		/* device for S_IFCHR/S_IFBLK */
-		uuid_t		di_muuid;	/* mount point value */
 		char		di_symlink[1];	/* local symbolic link */
 	}		di_u;
 	union {
@@ -150,7 +149,6 @@ typedef enum xfs_dinode_fmt
 					/* LNK: di_symlink */
 	XFS_DINODE_FMT_EXTENTS,		/* DIR, REG, LNK: di_bmx */
 	XFS_DINODE_FMT_BTREE,		/* DIR, REG, LNK: di_bmbt */
-	XFS_DINODE_FMT_UUID		/* MNT: di_uuid */
 } xfs_dinode_fmt_t;
 
 /*
fs/xfs/xfs_inode.c +0 -8
Index: linux-2.6-xfs/fs/xfs/xfs_inode.c
===================================================================
@@ -2970,14 +2970,6 @@ xfs_iflush_fork(
 		}
 		break;
 
-	case XFS_DINODE_FMT_UUID:
-		if (iip->ili_format.ilf_fields & XFS_ILOG_UUID) {
-			ASSERT(whichfork == XFS_DATA_FORK);
-			memcpy(&dip->di_u.di_muuid, &ip->i_df.if_u2.if_uuid,
-				sizeof(uuid_t));
-		}
-		break;
-
 	default:
 		ASSERT(0);
 		break;
fs/xfs/xfs_inode_item.c +11 -42

114 lines not on this page · raw message

fs/xfs/xfs_itable.c +0 -2
Index: linux-2.6-xfs/fs/xfs/xfs_itable.c
===================================================================
@@ -111,7 +111,6 @@ xfs_bulkstat_one_iget(
 		buf->bs_blocks = 0;
 		break;
 	case XFS_DINODE_FMT_LOCAL:
-	case XFS_DINODE_FMT_UUID:
 		buf->bs_rdev = 0;
 		buf->bs_blksize = mp->m_sb.sb_blocksize;
 		buf->bs_blocks = 0;
@@ -186,7 +185,6 @@ xfs_bulkstat_one_dinode(
 		buf->bs_blocks = 0;
 		break;
 	case XFS_DINODE_FMT_LOCAL:
-	case XFS_DINODE_FMT_UUID:
 		buf->bs_rdev = 0;
 		buf->bs_blksize = mp->m_sb.sb_blocksize;
 		buf->bs_blocks = 0;
fs/xfs/xfs_inode_item.h +8 -18
Index: linux-2.6-xfs/fs/xfs/xfs_inode_item.h
===================================================================
@@ -31,10 +31,7 @@ typedef struct xfs_inode_log_format {
 	__uint16_t		ilf_asize;	/* size of attr d/ext/root */
 	__uint16_t		ilf_dsize;	/* size of data/ext/root */
 	__uint64_t		ilf_ino;	/* inode number */
-	union {
-		__uint32_t	ilfu_rdev;	/* rdev value for dev inode*/
-		uuid_t		ilfu_uuid;	/* mount point value */
-	} ilf_u;
+	__uint32_t		ilf_rdev;	/* rdev value for dev inode*/
 	__int64_t		ilf_blkno;	/* blkno of inode buffer */
 	__int32_t		ilf_len;	/* len of inode buffer */
 	__int32_t		ilf_boffset;	/* off of inode in buffer */
@@ -48,10 +45,7 @@ typedef struct xfs_inode_log_format_32 {
 	__uint16_t		ilf_asize;	/* size of attr d/ext/root */
 	__uint16_t		ilf_dsize;	/* size of data/ext/root */
 	__uint64_t		ilf_ino;	/* inode number */
-	union {
-		__uint32_t	ilfu_rdev;	/* rdev value for dev inode*/
-		uuid_t		ilfu_uuid;	/* mount point value */
-	} ilf_u;
+	__uint32_t		ilf_rdev;	/* rdev value for dev inode*/
 	__int64_t		ilf_blkno;	/* blkno of inode buffer */
 	__int32_t		ilf_len;	/* len of inode buffer */
 	__int32_t		ilf_boffset;	/* off of inode in buffer */
@@ -66,10 +60,7 @@ typedef struct xfs_inode_log_format_64 {
 	__uint16_t		ilf_dsize;	/* size of data/ext/root */
 	__uint32_t		ilf_pad;	/* pad for 64 bit boundary */
 	__uint64_t		ilf_ino;	/* inode number */
-	union {
-		__uint32_t	ilfu_rdev;	/* rdev value for dev inode*/
-		uuid_t		ilfu_uuid;	/* mount point value */
-	} ilf_u;
+	__uint32_t		ilf_rdev;	/* rdev value for dev inode*/
 	__int64_t		ilf_blkno;	/* blkno of inode buffer */
 	__int32_t		ilf_len;	/* len of inode buffer */
 	__int32_t		ilf_boffset;	/* off of inode in buffer */
@@ -83,15 +74,15 @@ typedef struct xfs_inode_log_format_64 {
 #define	XFS_ILOG_DEXT	0x004	/* log i_df.if_extents */
 #define	XFS_ILOG_DBROOT	0x008	/* log i_df.i_broot */
 #define	XFS_ILOG_DEV	0x010	/* log the dev field */
-#define	XFS_ILOG_UUID	0x020	/* log the uuid field */
+/*			0x020*/ /* unused */
 #define	XFS_ILOG_ADATA	0x040	/* log i_af.if_data */
 #define	XFS_ILOG_AEXT	0x080	/* log i_af.if_extents */
 #define	XFS_ILOG_ABROOT	0x100	/* log i_af.i_broot */
 
 #define	XFS_ILOG_NONCORE	(XFS_ILOG_DDATA | XFS_ILOG_DEXT | \
 				 XFS_ILOG_DBROOT | XFS_ILOG_DEV | \
-				 XFS_ILOG_UUID | XFS_ILOG_ADATA | \
-				 XFS_ILOG_AEXT | XFS_ILOG_ABROOT)
+				 XFS_ILOG_ADATA | XFS_ILOG_AEXT | \
+				 XFS_ILOG_ABROOT)
 
 #define	XFS_ILOG_DFORK		(XFS_ILOG_DDATA | XFS_ILOG_DEXT | \
 				 XFS_ILOG_DBROOT)
@@ -101,9 +92,8 @@ typedef struct xfs_inode_log_format_64 {
 
 #define	XFS_ILOG_ALL		(XFS_ILOG_CORE | XFS_ILOG_DDATA | \
 				 XFS_ILOG_DEXT | XFS_ILOG_DBROOT | \
-				 XFS_ILOG_DEV | XFS_ILOG_UUID | \
-				 XFS_ILOG_ADATA | XFS_ILOG_AEXT | \
-				 XFS_ILOG_ABROOT)
+				 XFS_ILOG_DEV | XFS_ILOG_ADATA | \
+				 XFS_ILOG_AEXT | XFS_ILOG_ABROOT)
 
 #define	XFS_ILI_HOLD		0x1
 #define	XFS_ILI_IOLOCKED_EXCL	0x2
fs/xfs/xfs_log_recover.c +2 -8
Index: linux-2.6-xfs/fs/xfs/xfs_log_recover.c
===================================================================
@@ -2420,14 +2420,8 @@ xlog_recover_do_inode_trans(
 	}
 
 	fields = in_f->ilf_fields;
-	switch (fields & (XFS_ILOG_DEV | XFS_ILOG_UUID)) {
-	case XFS_ILOG_DEV:
-		dip->di_u.di_dev = cpu_to_be32(in_f->ilf_u.ilfu_rdev);
-		break;
-	case XFS_ILOG_UUID:
-		dip->di_u.di_muuid = in_f->ilf_u.ilfu_uuid;
-		break;
-	}
+	if (fields & XFS_ILOG_DEV)
+		dip->di_u.di_dev = cpu_to_be32(in_f->ilf_rdev);
 
 	if (in_f->ilf_size == 2)
 		goto write_inode_buffer;
fs/xfs/xfsidbg.c +1 -1
Index: linux-2.6-xfs/fs/xfs/xfsidbg.c
===================================================================
@@ -3424,7 +3424,7 @@ xfs_inode_item_print(xfs_inode_log_item_
 	kdb_printf("dsize %d, asize %d, rdev 0x%x\n",
 		ilip->ili_format.ilf_dsize,
 		ilip->ili_format.ilf_asize,
-		ilip->ili_format.ilf_u.ilfu_rdev);
+		ilip->ili_format.ilf_rdev);
 	kdb_printf("blkno 0x%Lx len 0x%x boffset 0x%x\n",
 		ilip->ili_format.ilf_blkno,
 		ilip->ili_format.ilf_len,
fs/xfs/xfs_inode.h +0 -1
Index: linux-2.6-xfs/fs/xfs/xfs_inode.h
===================================================================
@@ -79,7 +79,6 @@ typedef struct xfs_ifork {
 		char		if_inline_data[XFS_INLINE_DATA];
 						/* very small file data */
 		xfs_dev_t	if_rdev;	/* dev number if special */
-		uuid_t		if_uuid;	/* mount point value */
 	} if_u2;
 } xfs_ifork_t;
 
#1 lore raw

CXFS does use this to mount filesystems, i.e. on a client read the sb get the uuid and use the uuid to lookup the MDS to request the mount...

Having said that if it is removed then it will force us to look at another method to mount a cxfs filesystem, and also remove the necessity for cxfs clients to not read the superblock (which is the only metadata cxfs clients read off disk at this point)...

Regards,

Signature · 3 lines
Ian Costello   Phone: +61 3 9963 1952
R&D Engineer   Mobile: +61 417 508 522
CXFS MultiOS
Quoted 394 lines · Eric Sandeen

It looks like all of the below is unused... and according to Nathan,

"dont think it even got used/implemented anywhere, but i think it was meant to be an auto-mount kinda thing... such that when you look up at that point, it knows to mount the device with that uuid there, if its not already it was never really written anywhere ... just an idea in doug doucettes brain i think."

Think it'll ever go anywhere, or should it get pruned?

The below builds; not at all tested, until I get an idea if it's worth doing. Need to double check that some structures might not need padding out to keep things compatible/consistent...

 dmapi/xfs_dm.c    |    2 --
 xfs_attr_leaf.c   |    6 +-----
 xfs_bmap.c        |    4 ----
 xfs_dinode.h      |    2 --
 xfs_inode.c       |    8 --------
 xfs_inode.h       |    1 -
 xfs_inode_item.c  |   53 +++++++++++------------------------------------------
 xfs_inode_item.h  |   26 ++++++++------------------
 xfs_itable.c      |    2 --
 xfs_log_recover.c |   10 ++--------
 xfsidbg.c         |    2 +-
 11 files changed, 23 insertions(+), 93 deletions(-)

Index: linux-2.6-xfs/fs/xfs/dmapi/xfs_dm.c
===================================================================

fs/xfs/dmapi/xfs_dm.c +0 -2
@@ -363,7 +363,6 @@ xfs_dip_to_stat(
 		buf->dt_blocks = 0;
 		break;
 	case XFS_DINODE_FMT_LOCAL:
-	case XFS_DINODE_FMT_UUID:
 		buf->dt_rdev = 0;
 		buf->dt_blksize = mp->m_sb.sb_blocksize;
 		buf->dt_blocks = 0;
@@ -431,7 +430,6 @@ xfs_ip_to_stat(
 		buf->dt_blocks = 0;
 		break;
 	case XFS_DINODE_FMT_LOCAL:
-	case XFS_DINODE_FMT_UUID:
 		buf->dt_rdev = 0;
 		buf->dt_blksize = mp->m_sb.sb_blocksize;
 		buf->dt_blocks = 0;

Index: linux-2.6-xfs/fs/xfs/xfs_attr_leaf.c
===================================================================

fs/xfs/xfs_attr_leaf.c +1 -5
@@ -155,13 +155,9 @@ xfs_attr_shortform_bytesfit(xfs_inode_t 
 
 	offset = (XFS_LITINO(mp) - bytes) >> 3; /* rounded down */
 
-	switch (dp->i_d.di_format) {
-	case XFS_DINODE_FMT_DEV:
+	if (dp->i_d.di_format == XFS_DINODE_FMT_DEV) {
 		minforkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
 		return (offset >= minforkoff) ? minforkoff : 0;
-	case XFS_DINODE_FMT_UUID:
-		minforkoff = roundup(sizeof(uuid_t), 8) >> 3;
-		return (offset >= minforkoff) ? minforkoff : 0;
 	}
 
 	if (!(mp->m_flags & XFS_MOUNT_ATTR2)) {

Index: linux-2.6-xfs/fs/xfs/xfs_bmap.c
===================================================================

fs/xfs/xfs_bmap.c +0 -4
@@ -3532,7 +3532,6 @@ xfs_bmap_forkoff_reset(
 {
 	if (whichfork == XFS_ATTR_FORK &&
 	    (ip->i_d.di_format != XFS_DINODE_FMT_DEV) &&
-	    (ip->i_d.di_format != XFS_DINODE_FMT_UUID) &&
 	    (ip->i_d.di_format != XFS_DINODE_FMT_BTREE) &&
 	    ((mp->m_attroffset >> 3) > ip->i_d.di_forkoff)) {
 		ip->i_d.di_forkoff = mp->m_attroffset >> 3;
@@ -4000,9 +3999,6 @@ xfs_bmap_add_attrfork(
 	case XFS_DINODE_FMT_DEV:
 		ip->i_d.di_forkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
 		break;
-	case XFS_DINODE_FMT_UUID:
-		ip->i_d.di_forkoff = roundup(sizeof(uuid_t), 8) >> 3;
-		break;
 	case XFS_DINODE_FMT_LOCAL:
 	case XFS_DINODE_FMT_EXTENTS:
 	case XFS_DINODE_FMT_BTREE:

Index: linux-2.6-xfs/fs/xfs/xfs_dinode.h
===================================================================

fs/xfs/xfs_dinode.h +0 -2
@@ -88,7 +88,6 @@ typedef struct xfs_dinode
 		xfs_dir2_sf_t	di_dir2sf;	/* shortform directory v2 */
 		char		di_c[1];	/* local contents */
 		__be32		di_dev;		/* device for S_IFCHR/S_IFBLK */
-		uuid_t		di_muuid;	/* mount point value */
 		char		di_symlink[1];	/* local symbolic link */
 	}		di_u;
 	union {
@@ -150,7 +149,6 @@ typedef enum xfs_dinode_fmt
 					/* LNK: di_symlink */
 	XFS_DINODE_FMT_EXTENTS,		/* DIR, REG, LNK: di_bmx */
 	XFS_DINODE_FMT_BTREE,		/* DIR, REG, LNK: di_bmbt */
-	XFS_DINODE_FMT_UUID		/* MNT: di_uuid */
 } xfs_dinode_fmt_t;
 
 /*

Index: linux-2.6-xfs/fs/xfs/xfs_inode.c
===================================================================

fs/xfs/xfs_inode.c +0 -8
@@ -2970,14 +2970,6 @@ xfs_iflush_fork(
 		}
 		break;
 
-	case XFS_DINODE_FMT_UUID:
-		if (iip->ili_format.ilf_fields & XFS_ILOG_UUID) {
-			ASSERT(whichfork == XFS_DATA_FORK);
-			memcpy(&dip->di_u.di_muuid, &ip->i_df.if_u2.if_uuid,
-				sizeof(uuid_t));
-		}
-		break;
-
 	default:
 		ASSERT(0);
 		break;

Index: linux-2.6-xfs/fs/xfs/xfs_inode_item.c
===================================================================

fs/xfs/xfs_inode_item.c +11 -42
@@ -70,8 +70,7 @@ xfs_inode_item_size(
 	switch (ip->i_d.di_format) {
 	case XFS_DINODE_FMT_EXTENTS:
 		iip->ili_format.ilf_fields &=
-			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
-			  XFS_ILOG_DEV | XFS_ILOG_UUID);
+			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT | XFS_ILOG_DEV);
 		if ((iip->ili_format.ilf_fields & XFS_ILOG_DEXT) &&
 		    (ip->i_d.di_nextents > 0) &&
 		    (ip->i_df.if_bytes > 0)) {
@@ -86,8 +85,7 @@ xfs_inode_item_size(
 		ASSERT(ip->i_df.if_ext_max ==
 		       XFS_IFORK_DSIZE(ip) / (uint)sizeof(xfs_bmbt_rec_t));
 		iip->ili_format.ilf_fields &=
-			~(XFS_ILOG_DDATA | XFS_ILOG_DEXT |
-			  XFS_ILOG_DEV | XFS_ILOG_UUID);
+			~(XFS_ILOG_DDATA | XFS_ILOG_DEXT | XFS_ILOG_DEV);
 		if ((iip->ili_format.ilf_fields & XFS_ILOG_DBROOT) &&
 		    (ip->i_df.if_broot_bytes > 0)) {
 			ASSERT(ip->i_df.if_broot != NULL);
@@ -112,8 +110,7 @@ xfs_inode_item_size(
 
 	case XFS_DINODE_FMT_LOCAL:
 		iip->ili_format.ilf_fields &=
-			~(XFS_ILOG_DEXT | XFS_ILOG_DBROOT |
-			  XFS_ILOG_DEV | XFS_ILOG_UUID);
+			~(XFS_ILOG_DEXT | XFS_ILOG_DBROOT | XFS_ILOG_DEV);
 		if ((iip->ili_format.ilf_fields & XFS_ILOG_DDATA) &&
 		    (ip->i_df.if_bytes > 0)) {
 			ASSERT(ip->i_df.if_u1.if_data != NULL);
@@ -126,14 +123,7 @@ xfs_inode_item_size(
 
 	case XFS_DINODE_FMT_DEV:
 		iip->ili_format.ilf_fields &=
-			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
-			  XFS_ILOG_DEXT | XFS_ILOG_UUID);
-		break;
-
-	case XFS_DINODE_FMT_UUID:
-		iip->ili_format.ilf_fields &=
-			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
-			  XFS_ILOG_DEXT | XFS_ILOG_DEV);
+			~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT | XFS_ILOG_DEXT);
 		break;
 
 	default:
@@ -319,8 +309,7 @@ xfs_inode_item_format(
 	switch (ip->i_d.di_format) {
 	case XFS_DINODE_FMT_EXTENTS:
 		ASSERT(!(iip->ili_format.ilf_fields &
-			 (XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
-			  XFS_ILOG_DEV | XFS_ILOG_UUID)));
+			 (XFS_ILOG_DDATA | XFS_ILOG_DBROOT | XFS_ILOG_DEV)));
 		if (iip->ili_format.ilf_fields & XFS_ILOG_DEXT) {
 			ASSERT(ip->i_df.if_bytes > 0);
 			ASSERT(ip->i_df.if_u1.if_extents != NULL);
@@ -369,8 +358,7 @@ xfs_inode_item_format(
 
 	case XFS_DINODE_FMT_BTREE:
 		ASSERT(!(iip->ili_format.ilf_fields &
-			 (XFS_ILOG_DDATA | XFS_ILOG_DEXT |
-			  XFS_ILOG_DEV | XFS_ILOG_UUID)));
+			 (XFS_ILOG_DDATA | XFS_ILOG_DEXT | XFS_ILOG_DEV)));
 		if (iip->ili_format.ilf_fields & XFS_ILOG_DBROOT) {
 			ASSERT(ip->i_df.if_broot_bytes > 0);
 			ASSERT(ip->i_df.if_broot != NULL);
@@ -385,8 +373,7 @@ xfs_inode_item_format(
 
 	case XFS_DINODE_FMT_LOCAL:
 		ASSERT(!(iip->ili_format.ilf_fields &
-			 (XFS_ILOG_DBROOT | XFS_ILOG_DEXT |
-			  XFS_ILOG_DEV | XFS_ILOG_UUID)));
+			 (XFS_ILOG_DBROOT | XFS_ILOG_DEXT | XFS_ILOG_DEV)));
 		if (iip->ili_format.ilf_fields & XFS_ILOG_DDATA) {
 			ASSERT(ip->i_df.if_bytes > 0);
 			ASSERT(ip->i_df.if_u1.if_data != NULL);
@@ -411,21 +398,9 @@ xfs_inode_item_format(
 
 	case XFS_DINODE_FMT_DEV:
 		ASSERT(!(iip->ili_format.ilf_fields &
-			 (XFS_ILOG_DBROOT | XFS_ILOG_DEXT |
-			  XFS_ILOG_DDATA | XFS_ILOG_UUID)));
+			 (XFS_ILOG_DBROOT | XFS_ILOG_DEXT | XFS_ILOG_DDATA)));
 		if (iip->ili_format.ilf_fields & XFS_ILOG_DEV) {
-			iip->ili_format.ilf_u.ilfu_rdev =
-				ip->i_df.if_u2.if_rdev;
-		}
-		break;
-
-	case XFS_DINODE_FMT_UUID:
-		ASSERT(!(iip->ili_format.ilf_fields &
-			 (XFS_ILOG_DBROOT | XFS_ILOG_DEXT |
-			  XFS_ILOG_DDATA | XFS_ILOG_DEV)));
-		if (iip->ili_format.ilf_fields & XFS_ILOG_UUID) {
-			iip->ili_format.ilf_u.ilfu_uuid =
-				ip->i_df.if_u2.if_uuid;
+			iip->ili_format.ilf_rdev = ip->i_df.if_u2.if_rdev;
 		}
 		break;
 
@@ -1088,10 +1063,7 @@ xfs_inode_item_format_convert(
 		in_f->ilf_asize = in_f32->ilf_asize;
 		in_f->ilf_dsize = in_f32->ilf_dsize;
 		in_f->ilf_ino = in_f32->ilf_ino;
-		/* copy biggest field of ilf_u */
-		memcpy(in_f->ilf_u.ilfu_uuid.__u_bits,
-		       in_f32->ilf_u.ilfu_uuid.__u_bits,
-		       sizeof(uuid_t));
+		in_f->ilf_rdev = in_f32->ilf_rdev;
 		in_f->ilf_blkno = in_f32->ilf_blkno;
 		in_f->ilf_len = in_f32->ilf_len;
 		in_f->ilf_boffset = in_f32->ilf_boffset;
@@ -1106,10 +1078,7 @@ xfs_inode_item_format_convert(
 		in_f->ilf_asize = in_f64->ilf_asize;
 		in_f->ilf_dsize = in_f64->ilf_dsize;
 		in_f->ilf_ino = in_f64->ilf_ino;
-		/* copy biggest field of ilf_u */
-		memcpy(in_f->ilf_u.ilfu_uuid.__u_bits,
-		       in_f64->ilf_u.ilfu_uuid.__u_bits,
-		       sizeof(uuid_t));
+		in_f->ilf_rdev = in_f64->ilf_rdev;
 		in_f->ilf_blkno = in_f64->ilf_blkno;
 		in_f->ilf_len = in_f64->ilf_len;
 		in_f->ilf_boffset = in_f64->ilf_boffset;

Index: linux-2.6-xfs/fs/xfs/xfs_itable.c
===================================================================

fs/xfs/xfs_itable.c +0 -2
@@ -111,7 +111,6 @@ xfs_bulkstat_one_iget(
 		buf->bs_blocks = 0;
 		break;
 	case XFS_DINODE_FMT_LOCAL:
-	case XFS_DINODE_FMT_UUID:
 		buf->bs_rdev = 0;
 		buf->bs_blksize = mp->m_sb.sb_blocksize;
 		buf->bs_blocks = 0;
@@ -186,7 +185,6 @@ xfs_bulkstat_one_dinode(
 		buf->bs_blocks = 0;
 		break;
 	case XFS_DINODE_FMT_LOCAL:
-	case XFS_DINODE_FMT_UUID:
 		buf->bs_rdev = 0;
 		buf->bs_blksize = mp->m_sb.sb_blocksize;
 		buf->bs_blocks = 0;

Index: linux-2.6-xfs/fs/xfs/xfs_inode_item.h
===================================================================

fs/xfs/xfs_inode_item.h +8 -18
@@ -31,10 +31,7 @@ typedef struct xfs_inode_log_format {
 	__uint16_t		ilf_asize;	/* size of attr d/ext/root */
 	__uint16_t		ilf_dsize;	/* size of data/ext/root */
 	__uint64_t		ilf_ino;	/* inode number */
-	union {
-		__uint32_t	ilfu_rdev;	/* rdev value for dev inode*/
-		uuid_t		ilfu_uuid;	/* mount point value */
-	} ilf_u;
+	__uint32_t		ilf_rdev;	/* rdev value for dev inode*/
 	__int64_t		ilf_blkno;	/* blkno of inode buffer */
 	__int32_t		ilf_len;	/* len of inode buffer */
 	__int32_t		ilf_boffset;	/* off of inode in buffer */
@@ -48,10 +45,7 @@ typedef struct xfs_inode_log_format_32 {
 	__uint16_t		ilf_asize;	/* size of attr d/ext/root */
 	__uint16_t		ilf_dsize;	/* size of data/ext/root */
 	__uint64_t		ilf_ino;	/* inode number */
-	union {
-		__uint32_t	ilfu_rdev;	/* rdev value for dev inode*/
-		uuid_t		ilfu_uuid;	/* mount point value */
-	} ilf_u;
+	__uint32_t		ilf_rdev;	/* rdev value for dev inode*/
 	__int64_t		ilf_blkno;	/* blkno of inode buffer */
 	__int32_t		ilf_len;	/* len of inode buffer */
 	__int32_t		ilf_boffset;	/* off of inode in buffer */
@@ -66,10 +60,7 @@ typedef struct xfs_inode_log_format_64 {
 	__uint16_t		ilf_dsize;	/* size of data/ext/root */
 	__uint32_t		ilf_pad;	/* pad for 64 bit boundary */
 	__uint64_t		ilf_ino;	/* inode number */
-	union {
-		__uint32_t	ilfu_rdev;	/* rdev value for dev inode*/
-		uuid_t		ilfu_uuid;	/* mount point value */
-	} ilf_u;
+	__uint32_t		ilf_rdev;	/* rdev value for dev inode*/
 	__int64_t		ilf_blkno;	/* blkno of inode buffer */
 	__int32_t		ilf_len;	/* len of inode buffer */
 	__int32_t		ilf_boffset;	/* off of inode in buffer */
@@ -83,15 +74,15 @@ typedef struct xfs_inode_log_format_64 {
 #define	XFS_ILOG_DEXT	0x004	/* log i_df.if_extents */
 #define	XFS_ILOG_DBROOT	0x008	/* log i_df.i_broot */
 #define	XFS_ILOG_DEV	0x010	/* log the dev field */
-#define	XFS_ILOG_UUID	0x020	/* log the uuid field */
+/*			0x020*/ /* unused */
 #define	XFS_ILOG_ADATA	0x040	/* log i_af.if_data */
 #define	XFS_ILOG_AEXT	0x080	/* log i_af.if_extents */
 #define	XFS_ILOG_ABROOT	0x100	/* log i_af.i_broot */
 
 #define	XFS_ILOG_NONCORE	(XFS_ILOG_DDATA | XFS_ILOG_DEXT | \
 				 XFS_ILOG_DBROOT | XFS_ILOG_DEV | \
-				 XFS_ILOG_UUID | XFS_ILOG_ADATA | \
-				 XFS_ILOG_AEXT | XFS_ILOG_ABROOT)
+				 XFS_ILOG_ADATA | XFS_ILOG_AEXT | \
+				 XFS_ILOG_ABROOT)
 
 #define	XFS_ILOG_DFORK		(XFS_ILOG_DDATA | XFS_ILOG_DEXT | \
 				 XFS_ILOG_DBROOT)
@@ -101,9 +92,8 @@ typedef struct xfs_inode_log_format_64 {
 
 #define	XFS_ILOG_ALL		(XFS_ILOG_CORE | XFS_ILOG_DDATA | \
 				 XFS_ILOG_DEXT | XFS_ILOG_DBROOT | \
-				 XFS_ILOG_DEV | XFS_ILOG_UUID | \
-				 XFS_ILOG_ADATA | XFS_ILOG_AEXT | \
-				 XFS_ILOG_ABROOT)
+				 XFS_ILOG_DEV | XFS_ILOG_ADATA | \
+				 XFS_ILOG_AEXT | XFS_ILOG_ABROOT)
 
 #define	XFS_ILI_HOLD		0x1
 #define	XFS_ILI_IOLOCKED_EXCL	0x2

Index: linux-2.6-xfs/fs/xfs/xfs_log_recover.c
===================================================================

fs/xfs/xfs_log_recover.c +2 -8
@@ -2420,14 +2420,8 @@ xlog_recover_do_inode_trans(
 	}
 
 	fields = in_f->ilf_fields;
-	switch (fields & (XFS_ILOG_DEV | XFS_ILOG_UUID)) {
-	case XFS_ILOG_DEV:
-		dip->di_u.di_dev = cpu_to_be32(in_f->ilf_u.ilfu_rdev);
-		break;
-	case XFS_ILOG_UUID:
-		dip->di_u.di_muuid = in_f->ilf_u.ilfu_uuid;
-		break;
-	}
+	if (fields & XFS_ILOG_DEV)
+		dip->di_u.di_dev = cpu_to_be32(in_f->ilf_rdev);
 
 	if (in_f->ilf_size == 2)
 		goto write_inode_buffer;

Index: linux-2.6-xfs/fs/xfs/xfsidbg.c
===================================================================

fs/xfs/xfsidbg.c +1 -1
@@ -3424,7 +3424,7 @@ xfs_inode_item_print(xfs_inode_log_item_
 	kdb_printf("dsize %d, asize %d, rdev 0x%x\n",
 		ilip->ili_format.ilf_dsize,
 		ilip->ili_format.ilf_asize,
-		ilip->ili_format.ilf_u.ilfu_rdev);
+		ilip->ili_format.ilf_rdev);
 	kdb_printf("blkno 0x%Lx len 0x%x boffset 0x%x\n",
 		ilip->ili_format.ilf_blkno,
 		ilip->ili_format.ilf_len,

Index: linux-2.6-xfs/fs/xfs/xfs_inode.h
===================================================================

fs/xfs/xfs_inode.h +0 -1
@@ -79,7 +79,6 @@ typedef struct xfs_ifork {
 		char		if_inline_data[XFS_INLINE_DATA];
 						/* very small file data */
 		xfs_dev_t	if_rdev;	/* dev number if special */
-		uuid_t		if_uuid;	/* mount point value */
 	} if_u2;
 } xfs_ifork_t;
 
#2 lore raw
Quoted 2 lines · Ian Costello

CXFS does use this to mount filesystems, i.e. on a client read the sb get the uuid and use the uuid to lookup the MDS to request the mount...

Well, this isn't removing the sb uuid... it's this special file type XFS_DINODE_FMT_UUID... which is cxfs using?

-Eric

Quoted 7 lines

Having said that if it is removed then it will force us to look at another method to mount a cxfs filesystem, and also remove the necessity for cxfs clients to not read the superblock (which is the only metadata cxfs clients read off disk at this point)...

Regards,

#3 lore raw

Eric,

My bad, cxfs uses the sb_uuid, should have looked a little closer at the patch. I assumed the worst from the heading (must have filtered the "point" out of the heading :)

Quoted 14 lines · Eric Sandeen
Quoted 1 line · Ian Costello

CXFS does use this to mount filesystems, i.e. on a client read the sb get the uuid and use the uuid to lookup the MDS to request the mount...

Well, this isn't removing the sb uuid... it's this special file type XFS_DINODE_FMT_UUID... which is cxfs using?

-Eric

Quoted 4 lines

Having said that if it is removed then it will force us to look at another method to mount a cxfs filesystem, and also remove the necessity for cxfs clients to not read the superblock (which is the only metadata cxfs clients read off disk at this point)...

Regards,

Thanks,

Signature · 3 lines
Ian Costello   Phone: +61 3 9963 1952
R&D Engineer   Mobile: +61 417 508 522
CXFS MultiOS
#4 lore raw
Quoted 5 lines · Ian Costello

Eric,

My bad, cxfs uses the sb_uuid, should have looked a little closer at the patch. I assumed the worst from the heading (must have filtered the "point" out of the heading :)

Nah, I wouldn't do that to you ;)

(and you wouldn't have to accept it if I did try!) :)

-Eric

#5 lore raw
In reply to Eric Sandeen
Quoted 9 lines · Eric Sandeen

It looks like all of the below is unused... and according to Nathan,

"dont think it even got used/implemented anywhere, but i think it was meant to be an auto-mount kinda thing... such that when you look up at that point, it knows to mount the device with that uuid there, if its not already it was never really written anywhere ... just an idea in doug doucettes brain i think."

Quoted 5 lines

Think it'll ever go anywhere, or should it get pruned?

The below builds; not at all tested, until I get an idea if it's worth doing. Need to double check that some structures might not need padding out to keep things compatible/consistent...

This looks fine to me. But please keep the XFS_DINODE_FMT_UUID enum value and add a big comment close to it documenting what it was an maybe a approximate removal date so people who care about it can look it up in the SCM history easily.

#6 lore raw
In reply to Eric Sandeen
Quoted 9 lines · Eric Sandeen

It looks like all of the below is unused... and according to Nathan,

"dont think it even got used/implemented anywhere, but i think it was meant to be an auto-mount kinda thing... such that when you look up at that point, it knows to mount the device with that uuid there, if its not already it was never really written anywhere ... just an idea in doug doucettes brain i think."

Quoted 5 lines

Think it'll ever go anywhere, or should it get pruned?

The below builds; not at all tested, until I get an idea if it's worth doing. Need to double check that some structures might not need padding out to keep things compatible/consistent...

Since effectively all versions of XFS support this feature ondisk, including complete support in recovery, it would be better IMO to leave it in for someone to implement/experiment with the syscall and auto-mounting userspace support. That would then require no new feature bits, mkfs/repair changes, etc. There is effectively zero cost to leaving it there - and non-zero cost in removing it, if our seriously bad regression-via-cleanup history is anything to go by ... :|

It would be really unfortunate to remove this, and then find that it was useful to someone (who didn't know about it at this time). OTOH, if there is definately never ever any chance this can ever be useful, then it should indeed be removed. :)

cheers.

Signature · 1 line
Nathan
#7 lore raw

Nathan Scott wrote:

(hope you didn't too much mind my quoting you in this thread) ;)

Quoted 8 lines

Since effectively all versions of XFS support this feature ondisk, including complete support in recovery, it would be better IMO to leave it in for someone to implement/experiment with the syscall and auto-mounting userspace support. That would then require no new feature bits, mkfs/repair changes, etc. There is effectively zero cost to leaving it there - and non-zero cost in removing it, if our seriously bad regression-via-cleanup history is anything to go by ... :|

the only cost to leaving it is having another instance of "ok now what the heck is THIS?!" ... death by a thousand cuts of xfs complexity. But yeah, removing it has some risk too.

Quoted 4 lines

It would be really unfortunate to remove this, and then find that it was useful to someone (who didn't know about it at this time). OTOH, if there is definately never ever any chance this can ever be useful, then it should indeed be removed. :)

Well I'm not hung up about it. If anyone thinks it'll be useful, I'm not bothered by leaving it as is. So, Nathan, what are your plans for this code? *grin*

-Eric

#8 lore raw
Quoted 4 lines

Nathan Scott wrote:

(hope you didn't too much mind my quoting you in this thread) ;)

(hope you didn't mind too much my dissing your cleanup) :)

Quoted 9 lines
Quoted 8 lines

Since effectively all versions of XFS support this feature ondisk, including complete support in recovery, it would be better IMO to leave it in for someone to implement/experiment with the syscall and auto-mounting userspace support. That would then require no new feature bits, mkfs/repair changes, etc. There is effectively zero cost to leaving it there - and non-zero cost in removing it, if our seriously bad regression-via-cleanup history is anything to go by ... :|

Quoted 3 lines

the only cost to leaving it is having another instance of "ok now what the heck is THIS?!" ... death by a thousand cuts of xfs complexity. But yeah, removing it has some risk too.

So, document it and move on. It would be a fun little project to go and experiment with this code a bit. It amounts to a trivial amount of code at the end of the day, and theres certainly nothing "complex" about it.

Quoted 8 lines
Quoted 4 lines

It would be really unfortunate to remove this, and then find that it was useful to someone (who didn't know about it at this time). OTOH, if there is definately never ever any chance this can ever be useful, then it should indeed be removed. :)

Well I'm not hung up about it. If anyone thinks it'll be useful, I'm not bothered by leaving it as is. So, Nathan, what are your plans for this code? *grin*

I don't have any immediate plans. I can imagine it could be used to stitch parts of the namespace together in a filesystem that supports multiple devices (in a chunkfs kinda way) ... or maybe more simply just an in-filesystem auto-mounter. *shrug*. But its there, the tools support it (once again, I didn't see a userspace patch - hohum), so I would vote for leaving it in its current form so some enterprising, constructive young coder can try to make something useful from it at some point. :)

cheers.

#9 lore raw
Quoted 8 lines · On Wed, Mar 12, 2008 at 07:09:56AM +1100, [email protected] wrote:

I don't have any immediate plans. I can imagine it could be used to stitch parts of the namespace together in a filesystem that supports multiple devices (in a chunkfs kinda way) ... or maybe more simply just an in-filesystem auto-mounter. *shrug*. But its there, the tools support it (once again, I didn't see a userspace patch - hohum), so I would vote for leaving it in its current form so some enterprising, constructive young coder can try to make something useful from it at some point. :)

That kind of automounter really doesn't belong into the low-level filesystem. If we really wanted it it would go into the VFS, storing the uuid or other identifier for the mountpoint in an xattr. This is really just dead junk that should go away.

#10 lore raw

Re: [PATCH, RFC] - remove mountpoint UUID code

Quoted 10 lines · Christoph Hellwig
Quoted 8 lines · On Wed, Mar 12, 2008 at 07:09:56AM +1100, [email protected] wrote:

I don't have any immediate plans. I can imagine it could be used to stitch parts of the namespace together in a filesystem that supports multiple devices (in a chunkfs kinda way) ... or maybe more simply just an in-filesystem auto-mounter. *shrug*. But its there, the tools support it (once again, I didn't see a userspace patch - hohum), so I would vote for leaving it in its current form so some enterprising, constructive young coder can try to make something useful from it at some point. :)

Quoted 4 lines

That kind of automounter really doesn't belong into the low-level filesystem. If we really wanted it it would go into the VFS, storing the uuid or other identifier for the mountpoint in an xattr. This is really just dead junk that should go away.

Can I get a yea or nay on this from sgi, then?

Thanks,
-Eric

#12 lore raw
Quoted 1 line · Niv Sardi

ACK, removing it, it should be trivial to put back in if we ever need it.

Ok, hch had some comments and this needs a careful eye towards any affected disk structures so probably should not go in immediately as-is.

-Eric

#13 lore raw
Quoted 5 lines · Eric Sandeen
Quoted 1 line · Niv Sardi

ACK, removing it, it should be trivial to put back in if we ever need it.

Ok, hch had some comments and this needs a careful eye towards any affected disk structures so probably should not go in immediately as-is.

Yeah, just saw that, well it's only in -dev now, so we'll audit it carefully, and see where it goes.

I only read hch's last comment which ended in «This is really just dead junk that should go away.».

Anyway, we now can use the awesome xfsqa infrastructure to have this tested =)

Cheers,

Signature · 1 line
Niv Sardi