[PATCH 0/7] Add ioctl to support extended inode flags

[PATCH 0/7] Add ioctl to support extended inode flags

[David Sterba]

The patchset implements the existing VFS ioctls for reading extended
ioctl flags by btrfs.

There are many flags/attributes/extended/combined, the naming is
confusing, so let's recap what we have:

* generic VFS inode flags (i_flags)
  - S_* namespace
  https://elixir.bootlin.com/linux/v4.17-rc1/source/include/linux/fs.h#L1850
  - FS_IOC_GETFLAGS, FS_IOC_SETFLAGS
  - tools: lsatrr, chattr

* btrfs inode flags, on-disk format, independent of the above, with
  to/from conversions
  https://elixir.bootlin.com/linux/v4.17-rc1/source/fs/btrfs/ctree.h#L1416

* extended attributes, also called XATTR, but they're different entity,
  stored by an inode as key:value pairs
  - tools: getfattr, setfattr

* XFLAGs, another interface to the generic S_* flags, new ioctl added
  because [GS]ETFLAGS is frozen, new bits added, eg. for project quotas
  or DAX, and more that originate in XFS features
  https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/fs.h#L168
  - tools: xfs_io -c lsattr, xfs_io -c chattr

In the future, btrfs will probably get:

- nodefrag -- eg. to disable autodefrag or defrag ioctl
- nosymlinks -- for directories, prevent creating new symlinks
- dax

git://github.com/kdave/btrfs-devel dev/xflags

David Sterba (7):
  btrfs: rename btrfs_update_iflags to reflect which flags it touches
  btrfs: rename btrfs_mask_flags to reflect which flags it touches
  btrfs: rename check_flags to reflect which flags it touches
  btrfs: rename btrfs_flags_to_ioctl to reflect which flags it touches
  btrfs: add helpers for FS_XFLAG_* conversion
  btrfs: add FS_IOC_FSGETXATTR ioctl
  btrfs: add FS_IOC_FSSETXATTR ioctl

 fs/btrfs/ctree.h |   2 +-
 fs/btrfs/inode.c |   4 +-
 fs/btrfs/ioctl.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 170 insertions(+), 21 deletions(-)

-- 
2.16.2

[PATCH 1/7] btrfs: rename btrfs_update_iflags to reflect which flags it touches

[David Sterba]

The btrfs inode flag flavour is now simply called 'inode flags' and the
vfs inode are i_flags. Also rename the internal variable to something
less confusing than 'ip'.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ctree.h |  2 +-
 fs/btrfs/inode.c |  4 ++--
 fs/btrfs/ioctl.c | 16 ++++++++--------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 5474ef14d6e6..08a96e594097 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3235,7 +3235,7 @@ void btrfs_test_inode_set_ops(struct inode *inode);
 long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
 long btrfs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
 int btrfs_ioctl_get_supported_features(void __user *arg);
-void btrfs_update_iflags(struct inode *inode);
+void btrfs_sync_inode_flags_to_i_flags(struct inode *inode);
 int btrfs_is_empty_uuid(u8 *uuid);
 int btrfs_defrag_file(struct inode *inode, struct file *file,
 		      struct btrfs_ioctl_defrag_range_args *range,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index e064c49c9a9a..6a24b159aeeb 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3923,7 +3923,7 @@ static int btrfs_read_locked_inode(struct inode *inode)
 		break;
 	}
 
-	btrfs_update_iflags(inode);
+	btrfs_sync_inode_flags_to_i_flags(inode);
 	return 0;
 
 make_bad:
@@ -6259,7 +6259,7 @@ static void btrfs_inherit_iflags(struct inode *inode, struct inode *dir)
 			BTRFS_I(inode)->flags |= BTRFS_INODE_NODATASUM;
 	}
 
-	btrfs_update_iflags(inode);
+	btrfs_sync_inode_flags_to_i_flags(inode);
 }
 
 static struct inode *btrfs_new_inode(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 9cc4fd25f83d..c3189ebf7c88 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -136,20 +136,20 @@ static unsigned int btrfs_flags_to_ioctl(unsigned int flags)
 /*
  * Update inode->i_flags based on the btrfs internal flags.
  */
-void btrfs_update_iflags(struct inode *inode)
+void btrfs_sync_inode_flags_to_i_flags(struct inode *inode)
 {
-	struct btrfs_inode *ip = BTRFS_I(inode);
+	struct btrfs_inode *binode = BTRFS_I(inode);
 	unsigned int new_fl = 0;
 
-	if (ip->flags & BTRFS_INODE_SYNC)
+	if (binode->flags & BTRFS_INODE_SYNC)
 		new_fl |= S_SYNC;
-	if (ip->flags & BTRFS_INODE_IMMUTABLE)
+	if (binode->flags & BTRFS_INODE_IMMUTABLE)
 		new_fl |= S_IMMUTABLE;
-	if (ip->flags & BTRFS_INODE_APPEND)
+	if (binode->flags & BTRFS_INODE_APPEND)
 		new_fl |= S_APPEND;
-	if (ip->flags & BTRFS_INODE_NOATIME)
+	if (binode->flags & BTRFS_INODE_NOATIME)
 		new_fl |= S_NOATIME;
-	if (ip->flags & BTRFS_INODE_DIRSYNC)
+	if (binode->flags & BTRFS_INODE_DIRSYNC)
 		new_fl |= S_DIRSYNC;
 
 	set_mask_bits(&inode->i_flags,
@@ -317,7 +317,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
 		goto out_drop;
 	}
 
-	btrfs_update_iflags(inode);
+	btrfs_sync_inode_flags_to_i_flags(inode);
 	inode_inc_iversion(inode);
 	inode->i_ctime = current_time(inode);
 	ret = btrfs_update_inode(trans, root, inode);
-- 
2.16.2

[PATCH 2/7] btrfs: rename btrfs_mask_flags to reflect which flags it touches

[David Sterba]

The FS_*_FL flags cannot be easily identified by a variable name prefix
but we still need to recognize them so the 'fsflags' should be closer to
the naming scheme but again the 'fs' part sounds like it's a filesystem
flag. I don't have a better idea for now.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index c3189ebf7c88..b2c26beffd82 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -93,11 +93,12 @@ static int btrfs_clone(struct inode *src, struct inode *inode,
 		       int no_time_update);
 
 /* Mask out flags that are inappropriate for the given type of inode. */
-static unsigned int btrfs_mask_flags(umode_t mode, unsigned int flags)
+static unsigned int btrfs_mask_fsflags_for_type(struct inode *inode,
+		unsigned int flags)
 {
-	if (S_ISDIR(mode))
+	if (S_ISDIR(inode->i_mode))
 		return flags;
-	else if (S_ISREG(mode))
+	else if (S_ISREG(inode->i_mode))
 		return flags & ~FS_DIRSYNC_FL;
 	else
 		return flags & (FS_NODUMP_FL | FS_NOATIME_FL);
@@ -218,7 +219,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
 	i_oldflags = inode->i_flags;
 	mode = inode->i_mode;
 
-	flags = btrfs_mask_flags(inode->i_mode, flags);
+	flags = btrfs_mask_fsflags_for_type(inode, flags);
 	oldflags = btrfs_flags_to_ioctl(ip->flags);
 	if ((flags ^ oldflags) & (FS_APPEND_FL | FS_IMMUTABLE_FL)) {
 		if (!capable(CAP_LINUX_IMMUTABLE)) {
-- 
2.16.2

[PATCH 3/7] btrfs: rename check_flags to reflect which flags it touches

[David Sterba]

The FS_*_FL flags cannot be easily identified by a prefix but we still
need to recognize them so the 'fsflags' should be closer to the naming
scheme but again the 'fs' part sounds like it's a filesystem flag. I
don't have a better idea for now.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index b2c26beffd82..953473f2a136 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -168,7 +168,8 @@ static int btrfs_ioctl_getflags(struct file *file, void __user *arg)
 	return 0;
 }
 
-static int check_flags(unsigned int flags)
+/* Check if @flags are a supported and valid set of FS_*_FL flags */
+static int check_fsflags(unsigned int flags)
 {
 	if (flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | \
 		      FS_NOATIME_FL | FS_NODUMP_FL | \
@@ -205,7 +206,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
 	if (copy_from_user(&flags, arg, sizeof(flags)))
 		return -EFAULT;
 
-	ret = check_flags(flags);
+	ret = check_fsflags(flags);
 	if (ret)
 		return ret;
 
-- 
2.16.2

[PATCH 4/7] btrfs: rename btrfs_flags_to_ioctl to reflect which flags it touches

[David Sterba]

Converts btrfs_inode::flags to the FS_*_FL flags.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 953473f2a136..f0e6074233fa 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -105,9 +105,10 @@ static unsigned int btrfs_mask_fsflags_for_type(struct inode *inode,
 }
 
 /*
- * Export inode flags to the format expected by the FS_IOC_GETFLAGS ioctl.
+ * Export internal inode flags to the format expected by the FS_IOC_GETFLAGS
+ * ioctl.
  */
-static unsigned int btrfs_flags_to_ioctl(unsigned int flags)
+static unsigned int btrfs_inode_flags_to_fsflags(unsigned int flags)
 {
 	unsigned int iflags = 0;
 
@@ -161,7 +162,7 @@ void btrfs_sync_inode_flags_to_i_flags(struct inode *inode)
 static int btrfs_ioctl_getflags(struct file *file, void __user *arg)
 {
 	struct btrfs_inode *ip = BTRFS_I(file_inode(file));
-	unsigned int flags = btrfs_flags_to_ioctl(ip->flags);
+	unsigned int flags = btrfs_inode_flags_to_fsflags(ip->flags);
 
 	if (copy_to_user(arg, &flags, sizeof(flags)))
 		return -EFAULT;
@@ -221,7 +222,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
 	mode = inode->i_mode;
 
 	flags = btrfs_mask_fsflags_for_type(inode, flags);
-	oldflags = btrfs_flags_to_ioctl(ip->flags);
+	oldflags = btrfs_inode_flags_to_fsflags(ip->flags);
 	if ((flags ^ oldflags) & (FS_APPEND_FL | FS_IMMUTABLE_FL)) {
 		if (!capable(CAP_LINUX_IMMUTABLE)) {
 			ret = -EPERM;
-- 
2.16.2

[PATCH 5/7] btrfs: add helpers for FS_XFLAG_* conversion

[David Sterba]

Preparatory work for the FS_IOC_FSGETXATTR ioctl, basic conversions and
checking helpers.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index f0e6074233fa..a25ad4e6a478 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -338,6 +338,38 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
 	return ret;
 }
 
+/*
+ * Translate btrfs internal inode flags to xflags as expected by the
+ * FS_IOC_FSGETXATT ioctl. Filter only the supported ones, unknown flags are
+ * silently dropped.
+ */
+static unsigned int btrfs_inode_flags_to_xflags(unsigned int flags)
+{
+	unsigned int xflags = 0;
+
+	if (flags & BTRFS_INODE_APPEND)
+		xflags |= FS_XFLAG_APPEND;
+	if (flags & BTRFS_INODE_IMMUTABLE)
+		xflags |= FS_XFLAG_IMMUTABLE;
+	if (flags & BTRFS_INODE_NOATIME)
+		xflags |= FS_XFLAG_NOATIME;
+	if (flags & BTRFS_INODE_NODUMP)
+		xflags |= FS_XFLAG_NODUMP;
+	if (flags & BTRFS_INODE_SYNC)
+		xflags |= FS_XFLAG_SYNC;
+
+	return xflags;
+}
+
+/* Check if @flags are a supported and valid set of FS_XFLAGS_* flags */
+static int check_xflags(unsigned int flags)
+{
+	if (flags & ~(FS_XFLAG_APPEND | FS_XFLAG_IMMUTABLE | FS_XFLAG_NOATIME |
+		      FS_XFLAG_NODUMP | FS_XFLAG_SYNC))
+		return -EOPNOTSUPP;
+	return 0;
+}
+
 static int btrfs_ioctl_getversion(struct file *file, int __user *arg)
 {
 	struct inode *inode = file_inode(file);
-- 
2.16.2

[PATCH 6/7] btrfs: add FS_IOC_FSGETXATTR ioctl

[David Sterba]

The new ioctl is an extension to the FS_IOC_GETFLAGS and adds new
flags and is extensible. This patch allows to return the xflags portion
of the fsxattr structure, other items have no meaning for btrfs or can
be added later.

The original patch was written by Chandan Jay Sharma but was incomplete
and no further revision has been sent. Several cleanups were necessary
to avoid confusion with other ioctls, as we have another flavor of
flags.

Based-on-patches-by: Chandan Jay Sharma <chandansbg@gmail.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index a25ad4e6a478..52b12ab9b82b 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -370,6 +370,24 @@ static int check_xflags(unsigned int flags)
 	return 0;
 }
 
+/*
+ * Set the xflags from the internal inode flags. The remaining items of fsxattr
+ * are zeroed.
+ */
+static int btrfs_ioctl_fsgetxattr(struct file *file, void __user *arg)
+{
+	struct btrfs_inode *binode = BTRFS_I(file_inode(file));
+	struct fsxattr fa;
+
+	memset(&fa, 0, sizeof(fa));
+	fa.fsx_xflags = btrfs_inode_flags_to_xflags(binode->flags);
+
+	if (copy_to_user(arg, &fa, sizeof(fa)))
+		return -EFAULT;
+
+	return 0;
+}
+
 static int btrfs_ioctl_getversion(struct file *file, int __user *arg)
 {
 	struct inode *inode = file_inode(file);
@@ -5600,6 +5618,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_get_features(file, argp);
 	case BTRFS_IOC_SET_FEATURES:
 		return btrfs_ioctl_set_features(file, argp);
+	case FS_IOC_FSGETXATTR:
+		return btrfs_ioctl_fsgetxattr(file, argp);
 	}
 
 	return -ENOTTY;
-- 
2.16.2

[PATCH 7/7] btrfs: add FS_IOC_FSSETXATTR ioctl

[David Sterba]

The new ioctl is an extension to the FS_IOC_SETFLAGS and adds new
flags and is extensible. Don't get fooled by the XATTR in the name, it
does not have anything in common with the extended attributes,
incidentally also abbreviated as XATTRs.

This patch allows to set the xflags portion of the fsxattr structure,
other items have no meaning and non-zero values will result in
EOPNOTSUPP.

Currently supported xflags:

- APPEND
- IMMUTABLE
- NOATIME
- NODUMP
- SYNC

The structure of btrfs_ioctl_fssetxattr copies btrfs_ioctl_setflags but
is simpler on the flag setting side.

The original patch was written by Chandan Jay Sharma but was incomplete
and no further revision has been sent.

Based-on-patches-by: Chandan Jay Sharma <chandansbg@gmail.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 52b12ab9b82b..4fd61f191bba 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -388,6 +388,98 @@ static int btrfs_ioctl_fsgetxattr(struct file *file, void __user *arg)
 	return 0;
 }
 
+static int btrfs_ioctl_fssetxattr(struct file *file, void __user *arg)
+{
+	struct inode *inode = file_inode(file);
+	struct btrfs_inode *binode = BTRFS_I(inode);
+	struct btrfs_root *root = binode->root;
+	struct btrfs_trans_handle *trans;
+	struct fsxattr fa;
+	unsigned oldflags;
+	unsigned old_i_flags;
+	int ret = 0;
+
+	if (!inode_owner_or_capable(inode))
+		return -EPERM;
+
+	if (btrfs_root_readonly(root))
+		return -EROFS;
+
+	memset(&fa, 0, sizeof(fa));
+	if (copy_from_user(&fa, arg, sizeof(fa)))
+		return -EFAULT;
+
+	ret = check_xflags(fa.fsx_xflags);
+	if (ret)
+		return ret;
+
+	if (fa.fsx_extsize != 0 || fa.fsx_projid != 0 || fa.fsx_cowextsize != 0)
+		return -EOPNOTSUPP;
+
+	ret = mnt_want_write_file(file);
+	if (ret)
+		return ret;
+
+	inode_lock(inode);
+
+	oldflags = binode->flags;
+	old_i_flags = inode->i_flags;
+
+	/* We need the capabilities to change append-only or immutable inode */
+	if (((oldflags & (BTRFS_INODE_APPEND | BTRFS_INODE_IMMUTABLE)) ||
+	     (fa.fsx_xflags & (FS_XFLAG_APPEND | FS_XFLAG_IMMUTABLE))) &&
+	    !capable(CAP_LINUX_IMMUTABLE)) {
+		ret = -EPERM;
+		goto out_unlock;
+	}
+
+	if (fa.fsx_xflags & FS_XFLAG_SYNC)
+		binode->flags |= BTRFS_INODE_SYNC;
+	else
+		binode->flags &= ~BTRFS_INODE_SYNC;
+	if (fa.fsx_xflags & FS_XFLAG_IMMUTABLE)
+		binode->flags |= BTRFS_INODE_IMMUTABLE;
+	else
+		binode->flags &= ~BTRFS_INODE_IMMUTABLE;
+	if (fa.fsx_xflags & FS_XFLAG_APPEND)
+		binode->flags |= BTRFS_INODE_APPEND;
+	else
+		binode->flags &= ~BTRFS_INODE_APPEND;
+	if (fa.fsx_xflags & FS_XFLAG_NODUMP)
+		binode->flags |= BTRFS_INODE_NODUMP;
+	else
+		binode->flags &= ~BTRFS_INODE_NODUMP;
+	if (fa.fsx_xflags & FS_XFLAG_NOATIME)
+		binode->flags |= BTRFS_INODE_NOATIME;
+	else
+		binode->flags &= ~BTRFS_INODE_NOATIME;
+
+	/* 1 item for the inode */
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		goto out_drop;
+	}
+
+	btrfs_sync_inode_flags_to_i_flags(inode);
+	inode_inc_iversion(inode);
+	inode->i_ctime = current_time(inode);
+	ret = btrfs_update_inode(trans, root, inode);
+
+	btrfs_end_transaction(trans);
+
+	if (ret) {
+		binode->flags = oldflags;
+		inode->i_flags = old_i_flags;
+	}
+
+out_unlock:
+	inode_unlock(inode);
+out_drop:
+	mnt_drop_write_file(file);
+	return ret;
+}
+
 static int btrfs_ioctl_getversion(struct file *file, int __user *arg)
 {
 	struct inode *inode = file_inode(file);
@@ -5620,6 +5712,8 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_set_features(file, argp);
 	case FS_IOC_FSGETXATTR:
 		return btrfs_ioctl_fsgetxattr(file, argp);
+	case FS_IOC_FSSETXATTR:
+		return btrfs_ioctl_fssetxattr(file, argp);
 	}
 
 	return -ENOTTY;
-- 
2.16.2

Re: [PATCH 4/7] btrfs: rename btrfs_flags_to_ioctl to reflect which flags it touches

[Misono Tomohiro]

On 2018/04/21 2:02, David Sterba wrote:
> Converts btrfs_inode::flags to the FS_*_FL flags.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ioctl.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 953473f2a136..f0e6074233fa 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -105,9 +105,10 @@ static unsigned int btrfs_mask_fsflags_for_type(struct inode *inode,
>  }
>  
>  /*
> - * Export inode flags to the format expected by the FS_IOC_GETFLAGS ioctl.
> + * Export internal inode flags to the format expected by the FS_IOC_GETFLAGS
> + * ioctl.
>   */
> -static unsigned int btrfs_flags_to_ioctl(unsigned int flags)
> +static unsigned int btrfs_inode_flags_to_fsflags(unsigned int flags)
>  {
>  	unsigned int iflags = 0;
>  
> @@ -161,7 +162,7 @@ void btrfs_sync_inode_flags_to_i_flags(struct inode *inode)
>  static int btrfs_ioctl_getflags(struct file *file, void __user *arg)
>  {
>  	struct btrfs_inode *ip = BTRFS_I(file_inode(file));

*ip here and in btrfs_ioctl_setflags() should be changed to *binode as in 1st patch?

> -	unsigned int flags = btrfs_flags_to_ioctl(ip->flags);
> +	unsigned int flags = btrfs_inode_flags_to_fsflags(ip->flags);
>  
>  	if (copy_to_user(arg, &flags, sizeof(flags)))
>  		return -EFAULT;
> @@ -221,7 +222,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
>  	mode = inode->i_mode;
>  
>  	flags = btrfs_mask_fsflags_for_type(inode, flags);
> -	oldflags = btrfs_flags_to_ioctl(ip->flags);
> +	oldflags = btrfs_inode_flags_to_fsflags(ip->flags);
>  	if ((flags ^ oldflags) & (FS_APPEND_FL | FS_IMMUTABLE_FL)) {
>  		if (!capable(CAP_LINUX_IMMUTABLE)) {
>  			ret = -EPERM;
> 

Re: [PATCH 7/7] btrfs: add FS_IOC_FSSETXATTR ioctl

[Misono Tomohiro]

On 2018/04/21 2:02, David Sterba wrote:
> The new ioctl is an extension to the FS_IOC_SETFLAGS and adds new
> flags and is extensible. Don't get fooled by the XATTR in the name, it
> does not have anything in common with the extended attributes,
> incidentally also abbreviated as XATTRs.
> 
> This patch allows to set the xflags portion of the fsxattr structure,
> other items have no meaning and non-zero values will result in
> EOPNOTSUPP.
> 
> Currently supported xflags:
> 
> - APPEND
> - IMMUTABLE
> - NOATIME
> - NODUMP
> - SYNC
> 
> The structure of btrfs_ioctl_fssetxattr copies btrfs_ioctl_setflags but
> is simpler on the flag setting side.
> 
> The original patch was written by Chandan Jay Sharma but was incomplete
> and no further revision has been sent.
> 
> Based-on-patches-by: Chandan Jay Sharma <chandansbg@gmail.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/ioctl.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 94 insertions(+)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 52b12ab9b82b..4fd61f191bba 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -388,6 +388,98 @@ static int btrfs_ioctl_fsgetxattr(struct file *file, void __user *arg)
>  	return 0;
>  }
>  
> +static int btrfs_ioctl_fssetxattr(struct file *file, void __user *arg)
> +{
> +	struct inode *inode = file_inode(file);
> +	struct btrfs_inode *binode = BTRFS_I(inode);
> +	struct btrfs_root *root = binode->root;
> +	struct btrfs_trans_handle *trans;
> +	struct fsxattr fa;
> +	unsigned oldflags;
> +	unsigned old_i_flags;
> +	int ret = 0;
> +
> +	if (!inode_owner_or_capable(inode))
> +		return -EPERM;
> +
> +	if (btrfs_root_readonly(root))
> +		return -EROFS;
> +
> +	memset(&fa, 0, sizeof(fa));
> +	if (copy_from_user(&fa, arg, sizeof(fa)))
> +		return -EFAULT;
> +
> +	ret = check_xflags(fa.fsx_xflags);
> +	if (ret)
> +		return ret;
> +
> +	if (fa.fsx_extsize != 0 || fa.fsx_projid != 0 || fa.fsx_cowextsize != 0)
> +		return -EOPNOTSUPP;
> +
> +	ret = mnt_want_write_file(file);
> +	if (ret)
> +		return ret;
> +
> +	inode_lock(inode);
> +
> +	oldflags = binode->flags;
> +	old_i_flags = inode->i_flags;
> +
> +	/* We need the capabilities to change append-only or immutable inode */
> +	if (((oldflags & (BTRFS_INODE_APPEND | BTRFS_INODE_IMMUTABLE)) ||
> +	     (fa.fsx_xflags & (FS_XFLAG_APPEND | FS_XFLAG_IMMUTABLE))) &&
> +	    !capable(CAP_LINUX_IMMUTABLE)) {
> +		ret = -EPERM;
> +		goto out_unlock;
> +	}
> +
> +	if (fa.fsx_xflags & FS_XFLAG_SYNC)
> +		binode->flags |= BTRFS_INODE_SYNC;
> +	else
> +		binode->flags &= ~BTRFS_INODE_SYNC;
> +	if (fa.fsx_xflags & FS_XFLAG_IMMUTABLE)
> +		binode->flags |= BTRFS_INODE_IMMUTABLE;
> +	else
> +		binode->flags &= ~BTRFS_INODE_IMMUTABLE;
> +	if (fa.fsx_xflags & FS_XFLAG_APPEND)
> +		binode->flags |= BTRFS_INODE_APPEND;
> +	else
> +		binode->flags &= ~BTRFS_INODE_APPEND;
> +	if (fa.fsx_xflags & FS_XFLAG_NODUMP)
> +		binode->flags |= BTRFS_INODE_NODUMP;
> +	else
> +		binode->flags &= ~BTRFS_INODE_NODUMP;
> +	if (fa.fsx_xflags & FS_XFLAG_NOATIME)
> +		binode->flags |= BTRFS_INODE_NOATIME;
> +	else
> +		binode->flags &= ~BTRFS_INODE_NOATIME;
> +
> +	/* 1 item for the inode */
> +	trans = btrfs_start_transaction(root, 1);
> +	if (IS_ERR(trans)) {
> +		ret = PTR_ERR(trans);
> +		goto out_drop;
> +	}
> +
> +	btrfs_sync_inode_flags_to_i_flags(inode);
> +	inode_inc_iversion(inode);
> +	inode->i_ctime = current_time(inode);
> +	ret = btrfs_update_inode(trans, root, inode);
> +
> +	btrfs_end_transaction(trans);

Shouldn't out_drop label be here?
Otherwise, above "goto out_drop" won't unlock nor restore the flag value.

> +
> +	if (ret) {
> +		binode->flags = oldflags;
> +		inode->i_flags = old_i_flags;
> +	}
> +
> +out_unlock:
> +	inode_unlock(inode);
> +out_drop:
> +	mnt_drop_write_file(file);
> +	return ret;
> +}
> +
>  static int btrfs_ioctl_getversion(struct file *file, int __user *arg)
>  {
>  	struct inode *inode = file_inode(file);
> @@ -5620,6 +5712,8 @@ long btrfs_ioctl(struct file *file, unsigned int
>  		return btrfs_ioctl_set_features(file, argp);
>  	case FS_IOC_FSGETXATTR:
>  		return btrfs_ioctl_fsgetxattr(file, argp);
> +	case FS_IOC_FSSETXATTR:
> +		return btrfs_ioctl_fssetxattr(file, argp);
>  	}
>  
>  	return -ENOTTY;
> 

Re: [PATCH 4/7] btrfs: rename btrfs_flags_to_ioctl to reflect which flags it touches

[David Sterba]

On Mon, Apr 23, 2018 at 11:37:08AM +0900, Misono Tomohiro wrote:
> On 2018/04/21 2:02, David Sterba wrote:
> > Converts btrfs_inode::flags to the FS_*_FL flags.
> > 
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >  fs/btrfs/ioctl.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > index 953473f2a136..f0e6074233fa 100644
> > --- a/fs/btrfs/ioctl.c
> > +++ b/fs/btrfs/ioctl.c
> > @@ -105,9 +105,10 @@ static unsigned int btrfs_mask_fsflags_for_type(struct inode *inode,
> >  }
> >  
> >  /*
> > - * Export inode flags to the format expected by the FS_IOC_GETFLAGS ioctl.
> > + * Export internal inode flags to the format expected by the FS_IOC_GETFLAGS
> > + * ioctl.
> >   */
> > -static unsigned int btrfs_flags_to_ioctl(unsigned int flags)
> > +static unsigned int btrfs_inode_flags_to_fsflags(unsigned int flags)
> >  {
> >  	unsigned int iflags = 0;
> >  
> > @@ -161,7 +162,7 @@ void btrfs_sync_inode_flags_to_i_flags(struct inode *inode)
> >  static int btrfs_ioctl_getflags(struct file *file, void __user *arg)
> >  {
> >  	struct btrfs_inode *ip = BTRFS_I(file_inode(file));
> 
> *ip here and in btrfs_ioctl_setflags() should be changed to *binode as in 1st patch?

Yeah this would be at least consistent with the 1st patch, on a second
look I'm not sure if the variable renaming should be part of the patch,
so I'll move it to another patch.

Re: [PATCH 7/7] btrfs: add FS_IOC_FSSETXATTR ioctl

[David Sterba]

On Mon, Apr 23, 2018 at 11:42:42AM +0900, Misono Tomohiro wrote:
> On 2018/04/21 2:02, David Sterba wrote:
> > The new ioctl is an extension to the FS_IOC_SETFLAGS and adds new
> > flags and is extensible. Don't get fooled by the XATTR in the name, it
> > does not have anything in common with the extended attributes,
> > incidentally also abbreviated as XATTRs.
> > 
> > This patch allows to set the xflags portion of the fsxattr structure,
> > other items have no meaning and non-zero values will result in
> > EOPNOTSUPP.
> > 
> > Currently supported xflags:
> > 
> > - APPEND
> > - IMMUTABLE
> > - NOATIME
> > - NODUMP
> > - SYNC
> > 
> > The structure of btrfs_ioctl_fssetxattr copies btrfs_ioctl_setflags but
> > is simpler on the flag setting side.
> > 
> > The original patch was written by Chandan Jay Sharma but was incomplete
> > and no further revision has been sent.
> > 
> > Based-on-patches-by: Chandan Jay Sharma <chandansbg@gmail.com>
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> >  fs/btrfs/ioctl.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 94 insertions(+)
> > 
> > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > index 52b12ab9b82b..4fd61f191bba 100644
> > --- a/fs/btrfs/ioctl.c
> > +++ b/fs/btrfs/ioctl.c
> > @@ -388,6 +388,98 @@ static int btrfs_ioctl_fsgetxattr(struct file *file, void __user *arg)
> >  	return 0;
> >  }
> >  
> > +static int btrfs_ioctl_fssetxattr(struct file *file, void __user *arg)
> > +{
> > +	struct inode *inode = file_inode(file);
> > +	struct btrfs_inode *binode = BTRFS_I(inode);
> > +	struct btrfs_root *root = binode->root;
> > +	struct btrfs_trans_handle *trans;
> > +	struct fsxattr fa;
> > +	unsigned oldflags;
> > +	unsigned old_i_flags;
> > +	int ret = 0;
> > +
> > +	if (!inode_owner_or_capable(inode))
> > +		return -EPERM;
> > +
> > +	if (btrfs_root_readonly(root))
> > +		return -EROFS;
> > +
> > +	memset(&fa, 0, sizeof(fa));
> > +	if (copy_from_user(&fa, arg, sizeof(fa)))
> > +		return -EFAULT;
> > +
> > +	ret = check_xflags(fa.fsx_xflags);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (fa.fsx_extsize != 0 || fa.fsx_projid != 0 || fa.fsx_cowextsize != 0)
> > +		return -EOPNOTSUPP;
> > +
> > +	ret = mnt_want_write_file(file);
> > +	if (ret)
> > +		return ret;
> > +
> > +	inode_lock(inode);
> > +
> > +	oldflags = binode->flags;
> > +	old_i_flags = inode->i_flags;
> > +
> > +	/* We need the capabilities to change append-only or immutable inode */
> > +	if (((oldflags & (BTRFS_INODE_APPEND | BTRFS_INODE_IMMUTABLE)) ||
> > +	     (fa.fsx_xflags & (FS_XFLAG_APPEND | FS_XFLAG_IMMUTABLE))) &&
> > +	    !capable(CAP_LINUX_IMMUTABLE)) {
> > +		ret = -EPERM;
> > +		goto out_unlock;
> > +	}
> > +
> > +	if (fa.fsx_xflags & FS_XFLAG_SYNC)
> > +		binode->flags |= BTRFS_INODE_SYNC;
> > +	else
> > +		binode->flags &= ~BTRFS_INODE_SYNC;
> > +	if (fa.fsx_xflags & FS_XFLAG_IMMUTABLE)
> > +		binode->flags |= BTRFS_INODE_IMMUTABLE;
> > +	else
> > +		binode->flags &= ~BTRFS_INODE_IMMUTABLE;
> > +	if (fa.fsx_xflags & FS_XFLAG_APPEND)
> > +		binode->flags |= BTRFS_INODE_APPEND;
> > +	else
> > +		binode->flags &= ~BTRFS_INODE_APPEND;
> > +	if (fa.fsx_xflags & FS_XFLAG_NODUMP)
> > +		binode->flags |= BTRFS_INODE_NODUMP;
> > +	else
> > +		binode->flags &= ~BTRFS_INODE_NODUMP;
> > +	if (fa.fsx_xflags & FS_XFLAG_NOATIME)
> > +		binode->flags |= BTRFS_INODE_NOATIME;
> > +	else
> > +		binode->flags &= ~BTRFS_INODE_NOATIME;
> > +
> > +	/* 1 item for the inode */
> > +	trans = btrfs_start_transaction(root, 1);
> > +	if (IS_ERR(trans)) {
> > +		ret = PTR_ERR(trans);
> > +		goto out_drop;
> > +	}
> > +
> > +	btrfs_sync_inode_flags_to_i_flags(inode);
> > +	inode_inc_iversion(inode);
> > +	inode->i_ctime = current_time(inode);
> > +	ret = btrfs_update_inode(trans, root, inode);
> > +
> > +	btrfs_end_transaction(trans);
> 
> Shouldn't out_drop label be here?
> Otherwise, above "goto out_drop" won't unlock nor restore the flag value.

Right, will fix, thanks.