Re: [PATCH 2/2] btrfs: sysfs: Add entry which shows rmdir(2) can work for subvolume

2018-05-16 Thread Misono Tomohiro
On 2018/05/15 22:22, David Sterba wrote:
> On Tue, May 15, 2018 at 04:33:12PM +0900, Misono Tomohiro wrote:
>> Deletion of a subvolume by rmdir(2) has become allowed by the
>> 'commit cd2decf640b1 ("btrfs: Allow rmdir(2) to delete an empty
>> subvolume")'.
>>
>> It is a kind of new feature and this commits add new sysfs entry
>>   /sys/fs/btrfs/features/rmdir_subvol
>> to indicate the feature.
>>
>> Since the behavior is independent of feature bits of superblock,
>> new type FEAT_KERNEL is added to struct btrfs_feature_set.
>> Features of FEAT_KERNEL is supposed to be visible only in /sys/fs/features
>> and not in /sys/fs/UUID/features.
> 
> As the rmdir_subvol is a static feature, depending only on the kernel
> version, it's not needed to use the same infrastructure as the optional
> features. It also makes it unnecesarily complicated, to distinguish the
> on-disk and kernel-only features and it's not a per-filesystem feature.
> 
> It should be exported among btrfs_feature_attr_group and
> btrfs_supported_feature_attrs, possibly adding a new type of helpers if
> needed.

Understood. I will send v2 which adds new attr_group for static features.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] btrfs: sysfs: Add entry which shows rmdir(2) can work for subvolume

2018-05-15 Thread David Sterba
On Tue, May 15, 2018 at 04:33:12PM +0900, Misono Tomohiro wrote:
> Deletion of a subvolume by rmdir(2) has become allowed by the
> 'commit cd2decf640b1 ("btrfs: Allow rmdir(2) to delete an empty
> subvolume")'.
> 
> It is a kind of new feature and this commits add new sysfs entry
>   /sys/fs/btrfs/features/rmdir_subvol
> to indicate the feature.
> 
> Since the behavior is independent of feature bits of superblock,
> new type FEAT_KERNEL is added to struct btrfs_feature_set.
> Features of FEAT_KERNEL is supposed to be visible only in /sys/fs/features
> and not in /sys/fs/UUID/features.

As the rmdir_subvol is a static feature, depending only on the kernel
version, it's not needed to use the same infrastructure as the optional
features. It also makes it unnecesarily complicated, to distinguish the
on-disk and kernel-only features and it's not a per-filesystem feature.

It should be exported among btrfs_feature_attr_group and
btrfs_supported_feature_attrs, possibly adding a new type of helpers if
needed.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] btrfs: sysfs: Add entry which shows rmdir(2) can work for subvolume

2018-05-15 Thread Misono Tomohiro
Deletion of a subvolume by rmdir(2) has become allowed by the
'commit cd2decf640b1 ("btrfs: Allow rmdir(2) to delete an empty
subvolume")'.

It is a kind of new feature and this commits add new sysfs entry
  /sys/fs/btrfs/features/rmdir_subvol
to indicate the feature.

Since the behavior is independent of feature bits of superblock,
new type FEAT_KERNEL is added to struct btrfs_feature_set.
Features of FEAT_KERNEL is supposed to be visible only in /sys/fs/features
and not in /sys/fs/UUID/features.

Signed-off-by: Tomohiro Misono 
---
 fs/btrfs/ctree.h   |  6 ++
 fs/btrfs/sysfs.c   | 36 +++-
 fs/btrfs/sysfs.h   |  5 -
 include/uapi/linux/btrfs.h |  2 ++
 4 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 954bfb5054b1..c98355a468a9 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -271,6 +271,12 @@ struct btrfs_super_block {
(BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF)
 #define BTRFS_FEATURE_INCOMPAT_SAFE_CLEAR  0ULL
 
+#define BTRFS_FEATURE_KERNEL_SUPP  \
+   BTRFS_FEATURE_KERNEL_RMDIR_SUBVOL
+
+#define BTRFS_FEATURE_KERNEL_SAFE_SET  0ULL
+#define BTRFS_FEATURE_KERNEL_SAFE_CLEAR0ULL
+
 /*
  * A leaf is full of items. offset and size tell us where to find
  * the item in the leaf (relative to the start of the data area)
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 217d401fe8ae..ce6ba6b204c3 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -30,20 +30,24 @@ static u64 get_features(struct btrfs_fs_info *fs_info,
return btrfs_super_compat_flags(disk_super);
else if (set == FEAT_COMPAT_RO)
return btrfs_super_compat_ro_flags(disk_super);
-   else
+   else if (set == FEAT_INCOMPAT)
return btrfs_super_incompat_flags(disk_super);
+   else
+   return BTRFS_FEATURE_KERNEL_SUPP;
 }
 
 static void set_features(struct btrfs_fs_info *fs_info,
 enum btrfs_feature_set set, u64 features)
 {
struct btrfs_super_block *disk_super = fs_info->super_copy;
+
if (set == FEAT_COMPAT)
btrfs_set_super_compat_flags(disk_super, features);
else if (set == FEAT_COMPAT_RO)
btrfs_set_super_compat_ro_flags(disk_super, features);
-   else
+   else if (set == FEAT_INCOMPAT)
btrfs_set_super_incompat_flags(disk_super, features);
+   /* Nothing for FEAT_KERNEL for now */
 }
 
 static int can_modify_feature(struct btrfs_feature_attr *fa)
@@ -63,6 +67,10 @@ static int can_modify_feature(struct btrfs_feature_attr *fa)
set = BTRFS_FEATURE_INCOMPAT_SAFE_SET;
clear = BTRFS_FEATURE_INCOMPAT_SAFE_CLEAR;
break;
+   case FEAT_KERNEL:
+   set = BTRFS_FEATURE_KERNEL_SAFE_SET;
+   clear = BTRFS_FEATURE_KERNEL_SAFE_CLEAR;
+   break;
default:
pr_warn("btrfs: sysfs: unknown feature set %d\n",
fa->feature_set);
@@ -110,6 +118,10 @@ static ssize_t btrfs_feature_attr_store(struct kobject 
*kobj,
if (sb_rdonly(fs_info->sb))
return -EROFS;
 
+   /* FEAT_KERNEL features cannot be changed at runtime */
+   if (fa->feature_set == FEAT_KERNEL)
+   return -EINVAL;
+
ret = kstrtoul(skip_spaces(buf), 0, &val);
if (ret)
return ret;
@@ -174,7 +186,10 @@ static umode_t btrfs_feature_visible(struct kobject *kobj,
fa = attr_to_btrfs_feature_attr(attr);
features = get_features(fs_info, fa->feature_set);
 
-   if (can_modify_feature(fa))
+   /* Do not show FEAT_KERNEL features in /sys/fs/UUID/features */
+   if (fa->feature_set == FEAT_KERNEL)
+   mode = 0;
+   else if (can_modify_feature(fa))
mode |= S_IWUSR;
else if (!(features & fa->feature_bit))
mode = 0;
@@ -194,6 +209,7 @@ BTRFS_FEAT_ATTR_INCOMPAT(raid56, RAID56);
 BTRFS_FEAT_ATTR_INCOMPAT(skinny_metadata, SKINNY_METADATA);
 BTRFS_FEAT_ATTR_INCOMPAT(no_holes, NO_HOLES);
 BTRFS_FEAT_ATTR_COMPAT_RO(free_space_tree, FREE_SPACE_TREE);
+BTRFS_FEAT_ATTR_KERNEL(rmdir_subvol, RMDIR_SUBVOL);
 
 static struct attribute *btrfs_supported_feature_attrs[] = {
BTRFS_FEAT_ATTR_PTR(mixed_backref),
@@ -207,6 +223,7 @@ static struct attribute *btrfs_supported_feature_attrs[] = {
BTRFS_FEAT_ATTR_PTR(skinny_metadata),
BTRFS_FEAT_ATTR_PTR(no_holes),
BTRFS_FEAT_ATTR_PTR(free_space_tree),
+   BTRFS_FEAT_ATTR_PTR(rmdir_subvol),
NULL
 };
 
@@ -515,10 +532,11 @@ static inline struct btrfs_fs_info *to_fs_info(struct 
kobject *kobj)
 
 #define NUM_FEATURE_BITS 64
 #define BTRFS_FEATURE_NAME_MAX 13
-static char 
btrfs_unknown_feature_n