[RFC PATCH 1/3] btrfs-progs: introduce framework to check kernel supported features

[RFC PATCH 1/3] btrfs-progs: introduce framework to check kernel supported features

[Anand Jain]

In the newer kernel, supported kernel features can be known from
  /sys/fs/btrfs/features
however this interface was introduced only after 3.14, and most the
incompatible FS features were introduce before 3.14.

This patch proposes to maintain kernel version against the feature list,
and so that will be the minimum kernel version needed to use the feature.

Further, for features supported later than 3.14 this list can still be
updated, so it serves as a repository which can be displayed for easy
reference.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 utils.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 utils.h |  1 +
 2 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/utils.c b/utils.c
index b754686..cd5a626 100644
--- a/utils.c
+++ b/utils.c
@@ -32,10 +32,12 @@
 #include <linux/loop.h>
 #include <linux/major.h>
 #include <linux/kdev_t.h>
+#include <linux/version.h>
 #include <limits.h>
 #include <blkid/blkid.h>
 #include <sys/vfs.h>
 #include <sys/statfs.h>
+#include <sys/utsname.h>
 #include <linux/magic.h>
 
 #include "kerncompat.h"
@@ -567,21 +569,28 @@ out:
 	return ret;
 }
 
+/*
+ * min_ker_ver: update with minimum kernel version at which the feature
+ * was integrated into the mainline. For the transit period, that is
+ * feature not yet in mainline but in mailing list and for testing,
+ * please use "0.0" to indicate the same.
+ */
 static const struct btrfs_fs_feature {
 	const char *name;
 	u64 flag;
 	const char *desc;
+	const char *min_ker_ver;
 } mkfs_features[] = {
 	{ "mixed-bg", BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS,
-		"mixed data and metadata block groups" },
+		"mixed data and metadata block groups", "2.7.31"},
 	{ "extref", BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF,
-		"increased hardlink limit per file to 65536" },
+		"increased hardlink limit per file to 65536", "3.7"},
 	{ "raid56", BTRFS_FEATURE_INCOMPAT_RAID56,
-		"raid56 extended format" },
+		"raid56 extended format", "3.9"},
 	{ "skinny-metadata", BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA,
-		"reduced-size metadata extent refs" },
+		"reduced-size metadata extent refs", "3.10"},
 	{ "no-holes", BTRFS_FEATURE_INCOMPAT_NO_HOLES,
-		"no explicit hole extents for files" },
+		"no explicit hole extents for files", "3.14"},
 	/* Keep this one last */
 	{ "list-all", BTRFS_FEATURE_LIST_ALL, NULL }
 };
@@ -3077,3 +3086,53 @@ unsigned int get_unit_mode_from_arg(int *argc, char *argv[], int df_mode)
 
 	return unit_mode;
 }
+
+static int version_to_code(char *v)
+{
+	int i = 0;
+	char *b[3] = {NULL};
+	char *save_b = NULL;
+
+	for (b[i] = strtok_r(v, ".", &save_b);
+		b[i] != NULL;
+		b[i] = strtok_r(NULL, ".", &save_b))
+		i++;
+
+	if (b[2] == NULL)
+		return KERNEL_VERSION(atoi(b[0]), atoi(b[1]), 0);
+	else
+		return KERNEL_VERSION(atoi(b[0]), atoi(b[1]), atoi(b[2]));
+
+}
+
+static int get_kernel_code()
+{
+	int ret;
+	struct utsname utsbuf;
+	char *version;
+
+	ret = uname(&utsbuf);
+	if (ret)
+		return -ret;
+
+	version = strtok(utsbuf.release, "-");
+
+	return version_to_code(version);
+}
+
+u64 btrfs_features_allowed_by_kernel(void)
+{
+	int i;
+	int local_kernel_code = get_kernel_code();
+	u64 features = 0;
+
+	for (i = 0; i < ARRAY_SIZE(mkfs_features) - 1; i++) {
+		char *ver = strdup(mkfs_features[i].min_ker_ver);
+
+		if (local_kernel_code >= version_to_code(ver))
+			features |= mkfs_features[i].flag;
+
+		free(ver);
+	}
+	return (features);
+}
diff --git a/utils.h b/utils.h
index 192f3d1..9044643 100644
--- a/utils.h
+++ b/utils.h
@@ -104,6 +104,7 @@ void btrfs_list_all_fs_features(u64 mask_disallowed);
 char* btrfs_parse_fs_features(char *namelist, u64 *flags);
 void btrfs_process_fs_features(u64 flags);
 void btrfs_parse_features_to_string(char *buf, u64 flags);
+u64 btrfs_features_allowed_by_kernel(void);
 
 struct btrfs_mkfs_config {
 	char *label;
-- 
2.4.1

[RFC PATCH 2/3] btrfs-progs: kernel based default features for mkfs

[Anand Jain]

mkfs from latest btrfs-progs will enable latest default features,
and if the kernel is down-rev and does not support a latest default
feature then mount fails, as expected.

This patch disables default features based on the running kernel.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 mkfs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mkfs.c b/mkfs.c
index a5802f7..2b9d734 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -1357,10 +1357,13 @@ int main(int ac, char **av)
 	int dev_cnt = 0;
 	int saved_optind;
 	char fs_uuid[BTRFS_UUID_UNPARSED_SIZE] = { 0 };
-	u64 features = BTRFS_MKFS_DEFAULT_FEATURES;
+	u64 features;
 	struct mkfs_allocation allocation = { 0 };
 	struct btrfs_mkfs_config mkfs_cfg;
 
+	features = btrfs_features_allowed_by_kernel();
+	features &= BTRFS_MKFS_DEFAULT_FEATURES;
+
 	while(1) {
 		int c;
 		static const struct option long_options[] = {
-- 
2.4.1

[RFC PATCH 3/3] btrfs-progs: kernel based default features for btrfs-convert

[Anand Jain]

btrfs-convert convert FS with latest default features enabled, and
if the kernel is down-rev and does not support a latest feature then
mount fails, as expected.

This patch disables default features based on the running kernel.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 btrfs-convert.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/btrfs-convert.c b/btrfs-convert.c
index cb92020..edec09e 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -2890,7 +2890,10 @@ int main(int argc, char *argv[])
 	int progress = 1;
 	char *file;
 	char fslabel[BTRFS_LABEL_SIZE];
-	u64 features = BTRFS_MKFS_DEFAULT_FEATURES;
+	u64 features;
+
+	features = btrfs_features_allowed_by_kernel();
+	features &= BTRFS_MKFS_DEFAULT_FEATURES;
 
 	while(1) {
 		enum { GETOPT_VAL_NO_PROGRESS = 256 };
-- 
2.4.1

Re: [RFC PATCH 1/3] btrfs-progs: introduce framework to check kernel supported features

[Qu Wenruo]

Hi Anand,

This feature seems quite good, comment inlined below.

Anand Jain wrote on 2015/10/21 16:45 +0800:
> In the newer kernel, supported kernel features can be known from
>    /sys/fs/btrfs/features
> however this interface was introduced only after 3.14, and most the
> incompatible FS features were introduce before 3.14.
>
> This patch proposes to maintain kernel version against the feature list,
> and so that will be the minimum kernel version needed to use the feature.
>
> Further, for features supported later than 3.14 this list can still be
> updated, so it serves as a repository which can be displayed for easy
> reference.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>   utils.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>   utils.h |  1 +
>   2 files changed, 65 insertions(+), 5 deletions(-)
>
> diff --git a/utils.c b/utils.c
> index b754686..cd5a626 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -32,10 +32,12 @@
>   #include <linux/loop.h>
>   #include <linux/major.h>
>   #include <linux/kdev_t.h>
> +#include <linux/version.h>
>   #include <limits.h>
>   #include <blkid/blkid.h>
>   #include <sys/vfs.h>
>   #include <sys/statfs.h>
> +#include <sys/utsname.h>
>   #include <linux/magic.h>
>
>   #include "kerncompat.h"
> @@ -567,21 +569,28 @@ out:
>   	return ret;
>   }
>
> +/*
> + * min_ker_ver: update with minimum kernel version at which the feature
> + * was integrated into the mainline. For the transit period, that is
> + * feature not yet in mainline but in mailing list and for testing,
> + * please use "0.0" to indicate the same.
> + */
>   static const struct btrfs_fs_feature {
>   	const char *name;
>   	u64 flag;
>   	const char *desc;
> +	const char *min_ker_ver;
>   } mkfs_features[] = {
>   	{ "mixed-bg", BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS,
> -		"mixed data and metadata block groups" },
> +		"mixed data and metadata block groups", "2.7.31"},
>   	{ "extref", BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF,
> -		"increased hardlink limit per file to 65536" },
> +		"increased hardlink limit per file to 65536", "3.7"},
>   	{ "raid56", BTRFS_FEATURE_INCOMPAT_RAID56,
> -		"raid56 extended format" },
> +		"raid56 extended format", "3.9"},
>   	{ "skinny-metadata", BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA,
> -		"reduced-size metadata extent refs" },
> +		"reduced-size metadata extent refs", "3.10"},
>   	{ "no-holes", BTRFS_FEATURE_INCOMPAT_NO_HOLES,
> -		"no explicit hole extents for files" },
> +		"no explicit hole extents for files", "3.14"},
>   	/* Keep this one last */
>   	{ "list-all", BTRFS_FEATURE_LIST_ALL, NULL }
>   };
> @@ -3077,3 +3086,53 @@ unsigned int get_unit_mode_from_arg(int *argc, char *argv[], int df_mode)
>
>   	return unit_mode;
>   }
> +
> +static int version_to_code(char *v)
> +{
> +	int i = 0;
> +	char *b[3] = {NULL};
> +	char *save_b = NULL;
> +
> +	for (b[i] = strtok_r(v, ".", &save_b);
> +		b[i] != NULL;
> +		b[i] = strtok_r(NULL, ".", &save_b))
> +		i++;
> +
> +	if (b[2] == NULL)
> +		return KERNEL_VERSION(atoi(b[0]), atoi(b[1]), 0);
> +	else
> +		return KERNEL_VERSION(atoi(b[0]), atoi(b[1]), atoi(b[2]));
> +
> +}
> +
> +static int get_kernel_code()
> +{
> +	int ret;
> +	struct utsname utsbuf;
> +	char *version;
> +
> +	ret = uname(&utsbuf);
> +	if (ret)
> +		return -ret;
> +
> +	version = strtok(utsbuf.release, "-");
> +
> +	return version_to_code(version);
> +}

The only problem is, kernel version is never reliable.
If someone wants, uname output may even contain no numeric value.

IIRC, I suggest to maintain similar feature matrix in fstests, but Dave 
pointed out the above problem.

So I'm not fan of reading kernel version and generate supported features 
for that.

IMHO, just use /sys/fs/btrfs/features is good enough.
And if there is no such file, just ignore it, user is responsible for
such case.

Thanks,
Qu

> +
> +u64 btrfs_features_allowed_by_kernel(void)
> +{
> +	int i;
> +	int local_kernel_code = get_kernel_code();
> +	u64 features = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(mkfs_features) - 1; i++) {
> +		char *ver = strdup(mkfs_features[i].min_ker_ver);
> +
> +		if (local_kernel_code >= version_to_code(ver))
> +			features |= mkfs_features[i].flag;
> +
> +		free(ver);
> +	}
> +	return (features);
> +}
> diff --git a/utils.h b/utils.h
> index 192f3d1..9044643 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -104,6 +104,7 @@ void btrfs_list_all_fs_features(u64 mask_disallowed);
>   char* btrfs_parse_fs_features(char *namelist, u64 *flags);
>   void btrfs_process_fs_features(u64 flags);
>   void btrfs_parse_features_to_string(char *buf, u64 flags);
> +u64 btrfs_features_allowed_by_kernel(void);
>
>   struct btrfs_mkfs_config {
>   	char *label;
>

Re: [RFC PATCH 2/3] btrfs-progs: kernel based default features for mkfs

[Qu Wenruo]

Anand Jain wrote on 2015/10/21 16:45 +0800:
> mkfs from latest btrfs-progs will enable latest default features,
> and if the kernel is down-rev and does not support a latest default
> feature then mount fails, as expected.
>
> This patch disables default features based on the running kernel.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>   mkfs.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/mkfs.c b/mkfs.c
> index a5802f7..2b9d734 100644
> --- a/mkfs.c
> +++ b/mkfs.c
> @@ -1357,10 +1357,13 @@ int main(int ac, char **av)
>   	int dev_cnt = 0;
>   	int saved_optind;
>   	char fs_uuid[BTRFS_UUID_UNPARSED_SIZE] = { 0 };
> -	u64 features = BTRFS_MKFS_DEFAULT_FEATURES;
> +	u64 features;
>   	struct mkfs_allocation allocation = { 0 };
>   	struct btrfs_mkfs_config mkfs_cfg;
>
> +	features = btrfs_features_allowed_by_kernel();
> +	features &= BTRFS_MKFS_DEFAULT_FEATURES;
> +

Despite the problem of btrfs_features_allowed_by_kernel() I mentioned in 
previous mail,
the behavior is a little aggressive for me.

So a user with old kernel won't be able to create a filesystem with 
newer feature forever.
Maybe the user are just making btrfs for his or her newer kernel?

IMHO, it's better to output a warning other than just change features 
without any information.

Thanks,
Qu

>   	while(1) {
>   		int c;
>   		static const struct option long_options[] = {
>

Re: [RFC PATCH 3/3] btrfs-progs: kernel based default features for btrfs-convert

[Qu Wenruo]

Anand Jain wrote on 2015/10/21 16:45 +0800:
> btrfs-convert convert FS with latest default features enabled, and
> if the kernel is down-rev and does not support a latest feature then
> mount fails, as expected.
>
> This patch disables default features based on the running kernel.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>   btrfs-convert.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/btrfs-convert.c b/btrfs-convert.c
> index cb92020..edec09e 100644
> --- a/btrfs-convert.c
> +++ b/btrfs-convert.c
> @@ -2890,7 +2890,10 @@ int main(int argc, char *argv[])
>   	int progress = 1;
>   	char *file;
>   	char fslabel[BTRFS_LABEL_SIZE];
> -	u64 features = BTRFS_MKFS_DEFAULT_FEATURES;
> +	u64 features;
> +
> +	features = btrfs_features_allowed_by_kernel();
> +	features &= BTRFS_MKFS_DEFAULT_FEATURES;
Same with previous patch.

Thanks,
Qu
>
>   	while(1) {
>   		enum { GETOPT_VAL_NO_PROGRESS = 256 };
>

Re: [RFC PATCH 2/3] btrfs-progs: kernel based default features for mkfs

[Anand Jain]

Thanks for the comments.. more below.

On 10/21/2015 05:12 PM, Qu Wenruo wrote:
>
>
> Anand Jain wrote on 2015/10/21 16:45 +0800:
>> mkfs from latest btrfs-progs will enable latest default features,
>> and if the kernel is down-rev and does not support a latest default
>> feature then mount fails, as expected.
>>
>> This patch disables default features based on the running kernel.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   mkfs.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/mkfs.c b/mkfs.c
>> index a5802f7..2b9d734 100644
>> --- a/mkfs.c
>> +++ b/mkfs.c
>> @@ -1357,10 +1357,13 @@ int main(int ac, char **av)
>>       int dev_cnt = 0;
>>       int saved_optind;
>>       char fs_uuid[BTRFS_UUID_UNPARSED_SIZE] = { 0 };
>> -    u64 features = BTRFS_MKFS_DEFAULT_FEATURES;
>> +    u64 features;
>>       struct mkfs_allocation allocation = { 0 };
>>       struct btrfs_mkfs_config mkfs_cfg;
>>
>> +    features = btrfs_features_allowed_by_kernel();
>> +    features &= BTRFS_MKFS_DEFAULT_FEATURES;
>> +
>
> Despite the problem of btrfs_features_allowed_by_kernel() I mentioned in
> previous mail,
> the behavior is a little aggressive for me.
>
> So a user with old kernel won't be able to create a filesystem with
> newer feature forever.
> Maybe the user are just making btrfs for his or her newer kernel?

   I am not understanding the complete picture here, is there any 
example that you can quote. ?

Thanks, Anand

> IMHO, it's better to output a warning other than just change features
> without any information.
>
> Thanks,
> Qu
>
>>       while(1) {
>>           int c;
>>           static const struct option long_options[] = {
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 2/3] btrfs-progs: kernel based default features for mkfs

[Qu Wenruo]

在 2015年10月21日 22:15, Anand Jain 写道:
>
> Thanks for the comments.. more below.
>
> On 10/21/2015 05:12 PM, Qu Wenruo wrote:
>>
>>
>> Anand Jain wrote on 2015/10/21 16:45 +0800:
>>> mkfs from latest btrfs-progs will enable latest default features,
>>> and if the kernel is down-rev and does not support a latest default
>>> feature then mount fails, as expected.
>>>
>>> This patch disables default features based on the running kernel.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>>   mkfs.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mkfs.c b/mkfs.c
>>> index a5802f7..2b9d734 100644
>>> --- a/mkfs.c
>>> +++ b/mkfs.c
>>> @@ -1357,10 +1357,13 @@ int main(int ac, char **av)
>>>       int dev_cnt = 0;
>>>       int saved_optind;
>>>       char fs_uuid[BTRFS_UUID_UNPARSED_SIZE] = { 0 };
>>> -    u64 features = BTRFS_MKFS_DEFAULT_FEATURES;
>>> +    u64 features;
>>>       struct mkfs_allocation allocation = { 0 };
>>>       struct btrfs_mkfs_config mkfs_cfg;
>>>
>>> +    features = btrfs_features_allowed_by_kernel();
>>> +    features &= BTRFS_MKFS_DEFAULT_FEATURES;
>>> +
>>
>> Despite the problem of btrfs_features_allowed_by_kernel() I mentioned in
>> previous mail,
>> the behavior is a little aggressive for me.
>>
>> So a user with old kernel won't be able to create a filesystem with
>> newer feature forever.
>> Maybe the user are just making btrfs for his or her newer kernel?
>
>    I am not understanding the complete picture here, is there any
> example that you can quote. ?
>
> Thanks, Anand
>

Sorry my fault.
Your features are set before parse_fs_features()

So it's just my misunderstanding.

However, I still prefer warning other than just changing default features.

Thanks,
Qu

>> IMHO, it's better to output a warning other than just change features
>> without any information.
>>
>> Thanks,
>> Qu
>>
>>>       while(1) {
>>>           int c;
>>>           static const struct option long_options[] = {
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/3] btrfs-progs: introduce framework to check kernel supported features

[Eric Sandeen]

On 10/21/15 4:09 AM, Qu Wenruo wrote:
>> +static int get_kernel_code()
>> +{
>> +    int ret;
>> +    struct utsname utsbuf;
>> +    char *version;
>> +
>> +    ret = uname(&utsbuf);
>> +    if (ret)
>> +        return -ret;
>> +
>> +    version = strtok(utsbuf.release, "-");
>> +
>> +    return version_to_code(version);
>> +}
> 
> The only problem is, kernel version is never reliable.
> If someone wants, uname output may even contain no numeric value.

yep, I agree.  This will be misery for any custom kernel.

> IIRC, I suggest to maintain similar feature matrix in fstests, but Dave pointed out the above problem.
> 
> So I'm not fan of reading kernel version and generate supported features for that.
> 
> IMHO, just use /sys/fs/btrfs/features is good enough.

*nod*

> And if there is no such file, just ignore it, user is responsible for
> such case.

Yep, 3.14 was over a year and a half ago, I don't see much point in
hardcoding kernel versions for such old kernels in the current
upstream codebase.

The only kernels that old still running are likely distro kernels, and
they can solve this problem by backporting the /sys/fs/btrfs/features
patch.

-Eric
 
> Thanks,
> Qu

Re: [RFC PATCH 1/3] btrfs-progs: introduce framework to check kernel supported features

[Anand Jain]

On 10/21/2015 10:41 PM, Eric Sandeen wrote:
> On 10/21/15 4:09 AM, Qu Wenruo wrote:
>>> +static int get_kernel_code()
>>> +{
>>> +    int ret;
>>> +    struct utsname utsbuf;
>>> +    char *version;
>>> +
>>> +    ret = uname(&utsbuf);
>>> +    if (ret)
>>> +        return -ret;
>>> +
>>> +    version = strtok(utsbuf.release, "-");
>>> +
>>> +    return version_to_code(version);
>>> +}
>>
>> The only problem is, kernel version is never reliable.
>> If someone wants, uname output may even contain no numeric value.
>
> yep, I agree.  This will be misery for any custom kernel.

  How if we apply this only when kernel version is available ?
  Otherwise progs will assume all features are supported as in
  the current design.

Thanks, Anand


>> IIRC, I suggest to maintain similar feature matrix in fstests, but Dave pointed out the above problem.
>>
>> So I'm not fan of reading kernel version and generate supported features for that.
>>
>> IMHO, just use /sys/fs/btrfs/features is good enough.
>
> *nod*
>
>> And if there is no such file, just ignore it, user is responsible for
>> such case.
>
> Yep, 3.14 was over a year and a half ago, I don't see much point in
> hardcoding kernel versions for such old kernels in the current
> upstream codebase.
>
> The only kernels that old still running are likely distro kernels, and
> they can solve this problem by backporting the /sys/fs/btrfs/features
> patch.
>
> -Eric
>
>> Thanks,
>> Qu
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [RFC PATCH 1/3] btrfs-progs: introduce framework to check kernel supported features

[Eric Sandeen]

On 10/21/15 9:49 AM, Anand Jain wrote:
> 
> 
> On 10/21/2015 10:41 PM, Eric Sandeen wrote:
>> On 10/21/15 4:09 AM, Qu Wenruo wrote:
>>>> +static int get_kernel_code()
>>>> +{
>>>> +    int ret;
>>>> +    struct utsname utsbuf;
>>>> +    char *version;
>>>> +
>>>> +    ret = uname(&utsbuf);
>>>> +    if (ret)
>>>> +        return -ret;
>>>> +
>>>> +    version = strtok(utsbuf.release, "-");
>>>> +
>>>> +    return version_to_code(version);
>>>> +}
>>>
>>> The only problem is, kernel version is never reliable.
>>> If someone wants, uname output may even contain no numeric value.
>>
>> yep, I agree.  This will be misery for any custom kernel.
> 
>  How if we apply this only when kernel version is available ?

The problem is "kernel version" may not match btrfs version.
Distros backport and update subsystems without changing the
kernel version.

>  Otherwise progs will assume all features are supported as in
>  the current design.
> 
> Thanks, Anand

This is only a concern for kernels prior to 3.14, right?
v3.13 was released Sun Jan 19 18:40:23 2014, almost 2 years ago.

What has raised the current concern about these old kernels?
Why does this need fixing in upstream code?

-Eric

Re: [RFC PATCH 2/3] btrfs-progs: kernel based default features for mkfs

[Anand Jain]

Hi,

  more details about this RFC as below..


> So a user with old kernel won't be able to create a filesystem with
> newer feature forever.

  Thats not true. Here below is an example of the problem and fix
  in action.

> Maybe the user are just making btrfs for his or her newer kernel?

  Yes. Thats still possible as below.



[[current problem]]

I am on 3.8 kernel which does not support the skinny-metadata feature.
---------
# uname -r
3.8.13-98.4.1.el6uek.x86_64
-------------

Lets say customer upgraded the latest btrfs-progs as they wanted better
cli interface, reporting and document which is only available in the
latest btrfs-progs.

As btrfs-progs is backward kernel compatible, they don't have to
upgrade the kernel.

But as skinny-metadata is part of the "mkfs default feature", thats get
enabled by default during mkfs.

----------
# mkfs.btrfs -f /dev/sdc
btrfs-progs v4.2.2-7-g03cf344
See http://btrfs.wiki.kernel.org for more information.

Label:              (null)
UUID:               bb85b156-35eb-49a5-be5a-512fec1abab4
Node size:          16384
Sector size:        4096
Filesystem size:    3.00GiB
Block group profiles:
   Data:             single            8.00MiB
   Metadata:         DUP             161.56MiB
   System:           DUP              12.00MiB
SSD detected:       no
Incompat features:  extref, skinny-metadata         <-----
Number of devices:  1
Devices:
    ID        SIZE  PATH
     1     3.00GiB  /dev/sdc
-------------------

mount on the running kernel 3.8 fails, as there is no kernel support
for skinny-metadata
-----------
# mount /dev/sdc /btrfs
mount: wrong fs type, bad option, bad superblock on /dev/sdc,
        missing codepage or helper program, or other error
        In some cases useful info is found in syslog - try
        dmesg | tail  or so

BTRFS: couldn't mount because of unsupported optional features (100).
btrfs: open_ctree failed
----------

Customers need to upgrade the kernel also to mount this FS.


[[current solution]]

User must know that their running kernel does not support
skinny metadata and disable features accordingly as below.

------------
# mkfs.btrfs -f -O ^skinny-metadata /dev/sdc > /dev/null && mount 
/dev/sdc /btrfs
# echo $?
0
------------


[[problem with the current solution]]

btrfs-progs is backward kernel compatible.

But the default feature as set during mkfs is very btrfs-progs centric,
and is not in sync with the current running kernel.


[[new proposed, with the patch]]

- the default-features that are not supported by the running kernel
  are masked.

  So now the default mkfs.btrfs and mount works. As progs finds that
  kernel version is too old to support skinny-metadata and remotes is
  from the default feature list.

-------------------
# uname -r
3.8.13-98.4.1.el6uek.x86_64


# mkfs.btrfs -f /dev/sdc
btrfs-progs v4.2.2-7-g03cf344
See http://btrfs.wiki.kernel.org for more information.

Label:              (null)
UUID:               f2513ff0-6d94-4b6a-8bbf-ff53d343fa62
Node size:          16384
Sector size:        4096
Filesystem size:    3.00GiB
Block group profiles:
   Data:             single            8.00MiB
   Metadata:         DUP             161.56MiB
   System:           DUP              12.00MiB
SSD detected:       no
Incompat features:  extref  <-----
Number of devices:  1
Devices:
    ID        SIZE  PATH
     1     3.00GiB  /dev/sdc

# mount /dev/sdc /btrfs
-------------------


However as Qu pointed out if user wants to create FS with a feature that
is not supported by running kernel. They still have the choice to enable
it using -O option. as below. however they won't be able to mount it
unless kernel is upgraded as well.

-------
# mkfs.btrfs -f -O skinny-metadata /dev/sdc
btrfs-progs v4.2.2-7-g03cf344
See http://btrfs.wiki.kernel.org for more information.

Label:              (null)
UUID:               30c018ab-3d2a-4acc-8287-3f28c5324fca
Node size:          16384
Sector size:        4096
Filesystem size:    3.00GiB
Block group profiles:
   Data:             single            8.00MiB
   Metadata:         DUP             161.56MiB
   System:           DUP              12.00MiB
SSD detected:       no
Incompat features:  extref, skinny-metadata   <----
Number of devices:  1
Devices:
    ID        SIZE  PATH
     1     3.00GiB  /dev/sdc

# mount /dev/sdc /btrfs
mount: wrong fs type, bad option, bad superblock on /dev/sdc,
        missing codepage or helper program, or other error
        In some cases useful info is found in syslog - try
        dmesg | tail  or so
#
---------

In nut shell this patch makes _mkfs and btrfs-convert default features_
dynamically aligned with the running kernel.

Thanks, Anand

Re: [RFC PATCH 2/3] btrfs-progs: kernel based default features for mkfs

[Qu Wenruo]

Anand Jain wrote on 2015/10/22 11:09 +0800:
>
> Hi,
>
>   more details about this RFC as below..
>
>
>> So a user with old kernel won't be able to create a filesystem with
>> newer feature forever.
>
>   Thats not true. Here below is an example of the problem and fix
>   in action.
>
>> Maybe the user are just making btrfs for his or her newer kernel?
>
>   Yes. Thats still possible as below.
>
>
>
> [[current problem]]
>
> I am on 3.8 kernel which does not support the skinny-metadata feature.
> ---------
> # uname -r
> 3.8.13-98.4.1.el6uek.x86_64
> -------------
>
> Lets say customer upgraded the latest btrfs-progs as they wanted better
> cli interface, reporting and document which is only available in the
> latest btrfs-progs.
>
> As btrfs-progs is backward kernel compatible, they don't have to
> upgrade the kernel.
>
> But as skinny-metadata is part of the "mkfs default feature", thats get
> enabled by default during mkfs.

For real customer case, that's really a problem.
Customer is god, right?

But on the other hand, normally support team only provides support for 
their own product.
Here "product" may be their distribution or other software, but never 
any software customer build from source by themselves.

If customer want to upgrade btrfs-progs, they should use the repo from 
distribution, not compile by themselves.

And it will be the responsibility of backport team to backport needed 
features for kernel/btrfs-progs or change default features.

But unfortunately(or fortunately?), we are not backport team.

>
> ----------
> # mkfs.btrfs -f /dev/sdc
> btrfs-progs v4.2.2-7-g03cf344
> See http://btrfs.wiki.kernel.org for more information.
>
> Label:              (null)
> UUID:               bb85b156-35eb-49a5-be5a-512fec1abab4
> Node size:          16384
> Sector size:        4096
> Filesystem size:    3.00GiB
> Block group profiles:
>    Data:             single            8.00MiB
>    Metadata:         DUP             161.56MiB
>    System:           DUP              12.00MiB
> SSD detected:       no
> Incompat features:  extref, skinny-metadata         <-----
> Number of devices:  1
> Devices:
>     ID        SIZE  PATH
>      1     3.00GiB  /dev/sdc
> -------------------
>
> mount on the running kernel 3.8 fails, as there is no kernel support
> for skinny-metadata
> -----------
> # mount /dev/sdc /btrfs
> mount: wrong fs type, bad option, bad superblock on /dev/sdc,
>         missing codepage or helper program, or other error
>         In some cases useful info is found in syslog - try
>         dmesg | tail  or so
>
> BTRFS: couldn't mount because of unsupported optional features (100).
> btrfs: open_ctree failed
> ----------
>
> Customers need to upgrade the kernel also to mount this FS.
>
>
> [[current solution]]
>
> User must know that their running kernel does not support
> skinny metadata and disable features accordingly as below.
>
> ------------
> # mkfs.btrfs -f -O ^skinny-metadata /dev/sdc > /dev/null && mount
> /dev/sdc /btrfs
> # echo $?
> 0
> ------------
>
>
> [[problem with the current solution]]
>
> btrfs-progs is backward kernel compatible.
>
> But the default feature as set during mkfs is very btrfs-progs centric,
> and is not in sync with the current running kernel.
>
>
> [[new proposed, with the patch]]
>
> - the default-features that are not supported by the running kernel
>   are masked.
>
>   So now the default mkfs.btrfs and mount works. As progs finds that
>   kernel version is too old to support skinny-metadata and remotes is
>   from the default feature list.
>
> -------------------
> # uname -r
> 3.8.13-98.4.1.el6uek.x86_64
>
>
> # mkfs.btrfs -f /dev/sdc
> btrfs-progs v4.2.2-7-g03cf344
> See http://btrfs.wiki.kernel.org for more information.
>
> Label:              (null)
> UUID:               f2513ff0-6d94-4b6a-8bbf-ff53d343fa62
> Node size:          16384
> Sector size:        4096
> Filesystem size:    3.00GiB
> Block group profiles:
>    Data:             single            8.00MiB
>    Metadata:         DUP             161.56MiB
>    System:           DUP              12.00MiB
> SSD detected:       no
> Incompat features:  extref  <-----
> Number of devices:  1
> Devices:
>     ID        SIZE  PATH
>      1     3.00GiB  /dev/sdc
>
> # mount /dev/sdc /btrfs
> -------------------
>
>
> However as Qu pointed out if user wants to create FS with a feature that
> is not supported by running kernel. They still have the choice to enable
> it using -O option. as below. however they won't be able to mount it
> unless kernel is upgraded as well.
>
> -------
> # mkfs.btrfs -f -O skinny-metadata /dev/sdc
> btrfs-progs v4.2.2-7-g03cf344
> See http://btrfs.wiki.kernel.org for more information.
>
> Label:              (null)
> UUID:               30c018ab-3d2a-4acc-8287-3f28c5324fca
> Node size:          16384
> Sector size:        4096
> Filesystem size:    3.00GiB
> Block group profiles:
>    Data:             single            8.00MiB
>    Metadata:         DUP             161.56MiB
>    System:           DUP              12.00MiB
> SSD detected:       no
> Incompat features:  extref, skinny-metadata   <----
> Number of devices:  1
> Devices:
>     ID        SIZE  PATH
>      1     3.00GiB  /dev/sdc
>
> # mount /dev/sdc /btrfs
> mount: wrong fs type, bad option, bad superblock on /dev/sdc,
>         missing codepage or helper program, or other error
>         In some cases useful info is found in syslog - try
>         dmesg | tail  or so
> #
> ---------
>
> In nut shell this patch makes _mkfs and btrfs-convert default features_
> dynamically aligned with the running kernel.

Maybe just personal choice, I still prefer not to change default 
features, output warning should be good enough for such case,
due to the unreliableness of version check.

Thanks,
Qu

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

Re: [RFC PATCH 2/3] btrfs-progs: kernel based default features for mkfs

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/21/15 4:45 AM, Anand Jain wrote:
> mkfs from latest btrfs-progs will enable latest default features, 
> and if the kernel is down-rev and does not support a latest
> default feature then mount fails, as expected.
> 
> This patch disables default features based on the running kernel.

I like the idea generally based on comments further in the thread, but
what I don't like is:

1) It's silent.
2) There's no way to override it.

If we're going to change the defaults at runtime, we should tell the
user what has changed and why.  Otherwise, an identical mkfs.btrfs
binary will behave differently on different systems without feedback
and that violates the principle of least surprise.  If they want to do
what Qu suggests later in the thread, where the device is being
prepared for use on a newer kernel, it should be possible to force it.
 The normal -f should be fine there.

- -Jeff

> Signed-off-by: Anand Jain <anand.jain@oracle.com> --- mkfs.c | 5
> ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/mkfs.c b/mkfs.c index a5802f7..2b9d734 100644 ---
> a/mkfs.c +++ b/mkfs.c @@ -1357,10 +1357,13 @@ int main(int ac, char
> **av) int dev_cnt = 0; int saved_optind; char
> fs_uuid[BTRFS_UUID_UNPARSED_SIZE] = { 0 }; -	u64 features =
> BTRFS_MKFS_DEFAULT_FEATURES; +	u64 features; struct mkfs_allocation
> allocation = { 0 }; struct btrfs_mkfs_config mkfs_cfg;
> 
> +	features = btrfs_features_allowed_by_kernel(); +	features &=
> BTRFS_MKFS_DEFAULT_FEATURES; + while(1) { int c; static const
> struct option long_options[] = {
> 


- -- 
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.19 (Darwin)

iQIcBAEBAgAGBQJWKlE/AAoJEB57S2MheeWy2mAP/33RvA9174u0PRmh+RBorZDC
p3nDFFxS7pI5u7rSkFqUvbsKy9AoblUvgMYS8pDNkFokDML2hbH3HaYWFEmqvMch
mp9DQ+wKz5hI5fYt/wgDdtVO6X0E3TCm2Cj1Uw4fl7E0bMzgNgio8tnOoGTrHGa5
YkZ96L9UWzEScv9EtesO3DLbUC+O3pokyHsHCdBRVgEwLcLB1AtmPrQmhc2a1+M4
sfzElmbo9Rld/xmtI4ecHl1sWbpfrYcKimzV32Jdv/SNhEyPuFOcN6/GUDOrGE7o
Vs87+HtuXUr+CbFUM9r9wB1Nqj4yYJ78LnBfepBMjY9vWyAgPR49WFPRA/uhkvu/
uOd4DNgUbLktakztsMb1GRiS/6AEj6s8mHFzkOrS5b9E/RbwegWgcnpnWCveFcDO
Nsfa6Mg99X7ojuXeMi8c00Jins70uSnh/dLOtP5JYkxTAf8v5znbouYYawBZLHAi
P0KbIpQFmW+Qm9is1CDVZktnj79BFMcd+twMFQu/m9jhYdLUFqeEFCJ+sxCGcmoM
n18ayAzbvCQCYz5dBOk2EQPgQoQKJGEOdc4IY0GdRcOwNcbw2hWbwbfGjLAKpLrA
PVC8YmRsyT1CotXBXJEpn7jYFR2fnDOyO/5jq1JRDa6Mxeq3dECIRWof3pwQLnpI
boQXIGHUlVWltF+hla3C
=TG+F
-----END PGP SIGNATURE-----

Re: [RFC PATCH 2/3] btrfs-progs: kernel based default features for mkfs

[Anand Jain]

Thanks Jeff for the comments.

On 10/23/2015 11:24 PM, Jeff Mahoney wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 10/21/15 4:45 AM, Anand Jain wrote:
>> mkfs from latest btrfs-progs will enable latest default features,
>> and if the kernel is down-rev and does not support a latest
>> default feature then mount fails, as expected.
>>
>> This patch disables default features based on the running kernel.
>
> I like the idea generally based on comments further in the thread, but
> what I don't like is:
>
> 1) It's silent.

  Will add warning.

> 2) There's no way to override it.

> If we're going to change the defaults at runtime, we should tell the
> user what has changed and why.  Otherwise, an identical mkfs.btrfs
> binary will behave differently on different systems without feedback
> and that violates the principle of least surprise.  If they want to do
> what Qu suggests later in the thread, where the device is being
> prepared for use on a newer kernel, it should be possible to force it.
>   The normal -f should be fine there.

  Sill have no idea how to get this, trying.

Thanks, Anand


> - -Jeff
>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com> --- mkfs.c | 5
>> ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/mkfs.c b/mkfs.c index a5802f7..2b9d734 100644 ---
>> a/mkfs.c +++ b/mkfs.c @@ -1357,10 +1357,13 @@ int main(int ac, char
>> **av) int dev_cnt = 0; int saved_optind; char
>> fs_uuid[BTRFS_UUID_UNPARSED_SIZE] = { 0 }; -	u64 features =
>> BTRFS_MKFS_DEFAULT_FEATURES; +	u64 features; struct mkfs_allocation
>> allocation = { 0 }; struct btrfs_mkfs_config mkfs_cfg;
>>
>> +	features = btrfs_features_allowed_by_kernel(); +	features &=
>> BTRFS_MKFS_DEFAULT_FEATURES; + while(1) { int c; static const
>> struct option long_options[] = {
>>
>
>
> - --
> Jeff Mahoney
> SUSE Labs
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG/MacGPG2 v2.0.19 (Darwin)
>
> iQIcBAEBAgAGBQJWKlE/AAoJEB57S2MheeWy2mAP/33RvA9174u0PRmh+RBorZDC
> p3nDFFxS7pI5u7rSkFqUvbsKy9AoblUvgMYS8pDNkFokDML2hbH3HaYWFEmqvMch
> mp9DQ+wKz5hI5fYt/wgDdtVO6X0E3TCm2Cj1Uw4fl7E0bMzgNgio8tnOoGTrHGa5
> YkZ96L9UWzEScv9EtesO3DLbUC+O3pokyHsHCdBRVgEwLcLB1AtmPrQmhc2a1+M4
> sfzElmbo9Rld/xmtI4ecHl1sWbpfrYcKimzV32Jdv/SNhEyPuFOcN6/GUDOrGE7o
> Vs87+HtuXUr+CbFUM9r9wB1Nqj4yYJ78LnBfepBMjY9vWyAgPR49WFPRA/uhkvu/
> uOd4DNgUbLktakztsMb1GRiS/6AEj6s8mHFzkOrS5b9E/RbwegWgcnpnWCveFcDO
> Nsfa6Mg99X7ojuXeMi8c00Jins70uSnh/dLOtP5JYkxTAf8v5znbouYYawBZLHAi
> P0KbIpQFmW+Qm9is1CDVZktnj79BFMcd+twMFQu/m9jhYdLUFqeEFCJ+sxCGcmoM
> n18ayAzbvCQCYz5dBOk2EQPgQoQKJGEOdc4IY0GdRcOwNcbw2hWbwbfGjLAKpLrA
> PVC8YmRsyT1CotXBXJEpn7jYFR2fnDOyO/5jq1JRDa6Mxeq3dECIRWof3pwQLnpI
> boQXIGHUlVWltF+hla3C
> =TG+F
> -----END PGP SIGNATURE-----
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>