[Cluster-devel] [PATCH 0/2] Fix xfstest generic/424

[Cluster-devel] [PATCH 0/2] Fix xfstest generic/424

[Andreas Gruenbacher]

These two patches fix xfstest generic/424.

Andreas Gruenbacher (2):
  gfs: Fix and clean up FS_IOC_{GET,SET}FLAGS
  gfs2: Add support for statx inode flags

 fs/gfs2/file.c  | 102 ++++++++++++++++++++++++++------------------------------
 fs/gfs2/inode.c |  14 ++++++++
 2 files changed, 61 insertions(+), 55 deletions(-)

-- 
2.13.5

[Cluster-devel] [PATCH 1/2] gfs: Fix and clean up FS_IOC_{GET, SET}FLAGS

[Andreas Gruenbacher]

Switch to a simple array for mapping between the FS_*_FL and GFS_DIF_*
flags.  Clarify how the mapping between FS_JOURNAL_DATA_FL and the
filesystem flags works.  The GFS2_DIF_SYSTEM flag cannot be set from
user space, so remove it from GFS2_FLAGS_USER_SET.  Fail with -EINVAL
when trying to set flags that are not supported instead of silently
ignoring those flags.

Partially fixes xfstest generic/424.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/file.c | 102 ++++++++++++++++++++++++++-------------------------------
 1 file changed, 47 insertions(+), 55 deletions(-)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 33a0cb5701a3..1a33e55de1bc 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -108,45 +108,22 @@ static int gfs2_readdir(struct file *file, struct dir_context *ctx)
 }
 
 /**
- * fsflags_cvt
- * @table: A table of 32 u32 flags
- * @val: a 32 bit value to convert
+ * fsflag_gfs2flag
  *
- * This function can be used to convert between fsflags values and
- * GFS2's own flags values.
- *
- * Returns: the converted flags
+ * The FS_JOURNAL_DATA_FL flag maps to GFS2_DIF_INHERIT_JDATA for directories,
+ * and to GFS2_DIF_JDATA for non-directories.
  */
-static u32 fsflags_cvt(const u32 *table, u32 val)
-{
-	u32 res = 0;
-	while(val) {
-		if (val & 1)
-			res |= *table;
-		table++;
-		val >>= 1;
-	}
-	return res;
-}
-
-static const u32 fsflags_to_gfs2[32] = {
-	[3] = GFS2_DIF_SYNC,
-	[4] = GFS2_DIF_IMMUTABLE,
-	[5] = GFS2_DIF_APPENDONLY,
-	[7] = GFS2_DIF_NOATIME,
-	[12] = GFS2_DIF_EXHASH,
-	[14] = GFS2_DIF_INHERIT_JDATA,
-	[17] = GFS2_DIF_TOPDIR,
-};
-
-static const u32 gfs2_to_fsflags[32] = {
-	[gfs2fl_Sync] = FS_SYNC_FL,
-	[gfs2fl_Immutable] = FS_IMMUTABLE_FL,
-	[gfs2fl_AppendOnly] = FS_APPEND_FL,
-	[gfs2fl_NoAtime] = FS_NOATIME_FL,
-	[gfs2fl_ExHash] = FS_INDEX_FL,
-	[gfs2fl_TopLevel] = FS_TOPDIR_FL,
-	[gfs2fl_InheritJdata] = FS_JOURNAL_DATA_FL,
+static struct {
+	u32 fsflag;
+	u32 gfsflag;
+} fsflag_gfs2flag[] = {
+	{FS_SYNC_FL, GFS2_DIF_SYNC},
+	{FS_IMMUTABLE_FL, GFS2_DIF_IMMUTABLE},
+	{FS_APPEND_FL, GFS2_DIF_APPENDONLY},
+	{FS_NOATIME_FL, GFS2_DIF_NOATIME},
+	{FS_INDEX_FL, GFS2_DIF_EXHASH},
+	{FS_TOPDIR_FL, GFS2_DIF_TOPDIR},
+	{FS_JOURNAL_DATA_FL, GFS2_DIF_JDATA | GFS2_DIF_INHERIT_JDATA},
 };
 
 static int gfs2_get_flags(struct file *filp, u32 __user *ptr)
@@ -154,17 +131,23 @@ static int gfs2_get_flags(struct file *filp, u32 __user *ptr)
 	struct inode *inode = file_inode(filp);
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_holder gh;
-	int error;
-	u32 fsflags;
+	int i, error;
+	u32 gfsflags, fsflags = 0;
 
 	gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
 	error = gfs2_glock_nq(&gh);
 	if (error)
 		goto out_uninit;
 
-	fsflags = fsflags_cvt(gfs2_to_fsflags, ip->i_diskflags);
-	if (!S_ISDIR(inode->i_mode) && ip->i_diskflags & GFS2_DIF_JDATA)
-		fsflags |= FS_JOURNAL_DATA_FL;
+	gfsflags = ip->i_diskflags;
+	if (S_ISDIR(inode->i_mode))
+		gfsflags &= ~GFS2_DIF_JDATA;
+	else
+		gfsflags &= ~GFS2_DIF_INHERIT_JDATA;
+	for (i = 0; i < ARRAY_SIZE(fsflag_gfs2flag); i++)
+		if (gfsflags & fsflag_gfs2flag[i].gfsflag)
+			fsflags |= fsflag_gfs2flag[i].fsflag;
+
 	if (put_user(fsflags, ptr))
 		error = -EFAULT;
 
@@ -199,7 +182,6 @@ void gfs2_set_inode_flags(struct inode *inode)
 			     GFS2_DIF_APPENDONLY|		\
 			     GFS2_DIF_NOATIME|			\
 			     GFS2_DIF_SYNC|			\
-			     GFS2_DIF_SYSTEM|			\
 			     GFS2_DIF_TOPDIR|			\
 			     GFS2_DIF_INHERIT_JDATA)
 
@@ -238,10 +220,6 @@ static int do_gfs2_set_flags(struct file *filp, u32 reqflags, u32 mask)
 	if ((new_flags ^ flags) == 0)
 		goto out;
 
-	error = -EINVAL;
-	if ((new_flags ^ flags) & ~GFS2_FLAGS_USER_SET)
-		goto out;
-
 	error = -EPERM;
 	if (IS_IMMUTABLE(inode) && (new_flags & GFS2_DIF_IMMUTABLE))
 		goto out;
@@ -289,19 +267,33 @@ static int do_gfs2_set_flags(struct file *filp, u32 reqflags, u32 mask)
 static int gfs2_set_flags(struct file *filp, u32 __user *ptr)
 {
 	struct inode *inode = file_inode(filp);
-	u32 fsflags, gfsflags;
+	u32 fsflags, gfsflags = 0;
+	u32 mask;
+	int i;
 
 	if (get_user(fsflags, ptr))
 		return -EFAULT;
 
-	gfsflags = fsflags_cvt(fsflags_to_gfs2, fsflags);
-	if (!S_ISDIR(inode->i_mode)) {
-		gfsflags &= ~GFS2_DIF_TOPDIR;
-		if (gfsflags & GFS2_DIF_INHERIT_JDATA)
-			gfsflags ^= (GFS2_DIF_JDATA | GFS2_DIF_INHERIT_JDATA);
-		return do_gfs2_set_flags(filp, gfsflags, ~GFS2_DIF_SYSTEM);
+	for (i = 0; i < ARRAY_SIZE(fsflag_gfs2flag); i++) {
+		if (fsflags & fsflag_gfs2flag[i].fsflag) {
+			fsflags &= ~fsflag_gfs2flag[i].fsflag;
+			gfsflags |= fsflag_gfs2flag[i].gfsflag;
+		}
+	}
+	if (fsflags || gfsflags & ~GFS2_FLAGS_USER_SET)
+		return -EINVAL;
+
+	mask = GFS2_FLAGS_USER_SET;
+	if (S_ISDIR(inode->i_mode)) {
+		mask &= ~GFS2_DIF_JDATA;
+	} else {
+		/* The GFS2_DIF_TOPDIR flag is only valid for directories. */
+		if (gfsflags & GFS2_DIF_TOPDIR)
+			return -EINVAL;
+		mask &= ~(GFS2_DIF_TOPDIR | GFS2_DIF_INHERIT_JDATA);
 	}
-	return do_gfs2_set_flags(filp, gfsflags, ~(GFS2_DIF_SYSTEM | GFS2_DIF_JDATA));
+
+	return do_gfs2_set_flags(filp, gfsflags, mask);
 }
 
 static long gfs2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
-- 
2.13.5

[Cluster-devel] [PATCH 2/2] gfs2: Add support for statx inode flags

[Andreas Gruenbacher]

Add support for the STATX_ATTR_ flags in statx.  (Compression,
encryption, and the nodump flag are not supported by gfs2.)

Partially fixes xfstest generic/424.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/inode.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 863749e29bf9..7d91e2f7f7d0 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -1986,6 +1986,7 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
 	struct inode *inode = d_inode(path->dentry);
 	struct gfs2_inode *ip = GFS2_I(inode);
 	struct gfs2_holder gh;
+	u32 gfsflags;
 	int error;
 
 	gfs2_holder_mark_uninitialized(&gh);
@@ -1995,7 +1996,20 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
 			return error;
 	}
 
+	gfsflags = ip->i_diskflags;
+	if (gfsflags & GFS2_DIF_APPENDONLY)
+		stat->attributes |= STATX_ATTR_APPEND;
+	if (gfsflags & GFS2_DIF_IMMUTABLE)
+		stat->attributes |= STATX_ATTR_IMMUTABLE;
+
+        stat->attributes_mask |= (STATX_ATTR_APPEND |
+				  STATX_ATTR_COMPRESSED |
+				  STATX_ATTR_ENCRYPTED |
+				  STATX_ATTR_IMMUTABLE |
+				  STATX_ATTR_NODUMP);
+
 	generic_fillattr(inode, stat);
+
 	if (gfs2_holder_initialized(&gh))
 		gfs2_glock_dq_uninit(&gh);
 
-- 
2.13.5

[Cluster-devel] [PATCH 2/2] gfs2: Add support for statx inode flags

[Andrew Price]

On Mon, Oct 09, 2017 at 08:37:54PM +0200, Andreas Gruenbacher wrote:
> Add support for the STATX_ATTR_ flags in statx.  (Compression,
> encryption, and the nodump flag are not supported by gfs2.)
> 
> Partially fixes xfstest generic/424.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/gfs2/inode.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index 863749e29bf9..7d91e2f7f7d0 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -1986,6 +1986,7 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
>  	struct inode *inode = d_inode(path->dentry);
>  	struct gfs2_inode *ip = GFS2_I(inode);
>  	struct gfs2_holder gh;
> +	u32 gfsflags;
>  	int error;
>  
>  	gfs2_holder_mark_uninitialized(&gh);
> @@ -1995,7 +1996,20 @@ static int gfs2_getattr(const struct path *path, struct kstat *stat,
>  			return error;
>  	}
>  
> +	gfsflags = ip->i_diskflags;
> +	if (gfsflags & GFS2_DIF_APPENDONLY)
> +		stat->attributes |= STATX_ATTR_APPEND;
> +	if (gfsflags & GFS2_DIF_IMMUTABLE)
> +		stat->attributes |= STATX_ATTR_IMMUTABLE;
> +
> +        stat->attributes_mask |= (STATX_ATTR_APPEND |

    ^^^^^^^
There's a whitespace issue here. Other than that, and perhaps gfs->gfs2 in the subject of patch 1, both patches look good to me.

Cheers,
Andy

> +				  STATX_ATTR_COMPRESSED |
> +				  STATX_ATTR_ENCRYPTED |
> +				  STATX_ATTR_IMMUTABLE |
> +				  STATX_ATTR_NODUMP);
> +
>  	generic_fillattr(inode, stat);
> +
>  	if (gfs2_holder_initialized(&gh))
>  		gfs2_glock_dq_uninit(&gh);
>  
> -- 
> 2.13.5
>