[PATCH 0/7] Let user specify the kernel version for features

[PATCH 0/7] Let user specify the kernel version for features

[Anand Jain]

Sometimes users may want to have a btrfs to be supported on multiple
kernel version. A simple example, USB drive can be used with multiple
system running different kernel versions. Or in a data center a SAN
LUN could be mounted on any system with different kernel version.

Thanks for providing comments and feedback.
Further to it, here below is a set of patch which will introduce, to
specify a kernel version so that default features can be set based on
what features were supported at that kernel version.

First of all to let user know what features was supported at what kernel
version. Patch 1/7 updates -O list-all which will list the feature with
version.

As we didn't maintain the sysfs and progs feature names consistent, so
to avoid confusion Patch 2/7 displays sysfs feature name as well again
in the list-all output.

Next, Patch 3,4,5/7 are helper functions.

Patch 6,7/7 provides the -O comp=<version> for mkfs.btrfs and
btrfs-convert respectively

Thanks, Anand

Anand Jain (7):
  btrfs-progs: show the version for -O list-all
  btrfs-progs: add kernel alias for each of the features in the list
  btrfs-progs: make is_numerical non static
  btrfs-progs: check for numerical in version_to_code()
  btrfs-progs: introduce framework version to features
  btrfs-progs: add -O comp= option for mkfs.btrfs
  btrfs-progs: add -O comp= option for btrfs-convert

 btrfs-convert.c | 21 +++++++++++++++++++++
 cmds-replace.c  | 11 -----------
 mkfs.c          | 24 ++++++++++++++++++++++--
 utils.c         | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 utils.h         |  2 ++
 5 files changed, 98 insertions(+), 18 deletions(-)

-- 
2.6.2

[PATCH 1/7] btrfs-progs: show the version for -O list-all

[Anand Jain]

Shows min kernel version in the -O list-all output

eg: (version is show with in ())
btrfs-convert -O list-all
Filesystem features available:
extref          - increased hardlink limit per file to 65536 (0x40, 3.7, default)
skinny-metadata - reduced-size metadata extent refs (0x100, 3.10, default)
no-holes        - no explicit hole extents for files (0x200, 3.14)

mkfs.btrfs -O list-all
Filesystem features available:
mixed-bg        - mixed data and metadata block groups (0x4, 2.7.37)
extref          - increased hardlink limit per file to 65536 (0x40, 3.7, default)
raid56          - raid56 extended format (0x80, 3.9)
skinny-metadata - reduced-size metadata extent refs (0x100, 3.10, default)
no-holes        - no explicit hole extents for files (0x200, 3.14)

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

diff --git a/utils.c b/utils.c
index 2710ed7..0163915 100644
--- a/utils.c
+++ b/utils.c
@@ -657,10 +657,11 @@ void btrfs_list_all_fs_features(u64 mask_disallowed)
 			continue;
 		if (mkfs_features[i].flag & BTRFS_MKFS_DEFAULT_FEATURES)
 			is_default = ", default";
-		fprintf(stderr, "%-20s- %s (0x%llx%s)\n",
+		fprintf(stderr, "%-20s- %s (0x%llx, %s%s)\n",
 				mkfs_features[i].name,
 				mkfs_features[i].desc,
 				mkfs_features[i].flag,
+				mkfs_features[i].min_ker_ver,
 				is_default);
 	}
 }
-- 
2.6.2

[PATCH 2/7] btrfs-progs: add kernel alias for each of the features in the list

[Anand Jain]

We should have maintained feature's name same across progs UI and sysfs UI.
For example, progs mixed-bg is /sys/fs/btrfs/features/mixed_groups
in sysfs. As these are already released and is UIs, there is nothing much
can be done about it, except for creating the alias and making it aware.

Add kernel alias for each of the features in the list.

eg: The string with in () is the sysfs name for the same feaure

mkfs.btrfs -O list-all
Filesystem features available:
mixed-bg (mixed_groups)           - mixed data and metadata block groups (0x4, 2.7.37)
extref (extended_iref)            - increased hardlink limit per file to 65536 (0x40, 3.7, default)
raid56 (raid56)                   - raid56 extended format (0x80, 3.9)
skinny-metadata (skinny_metadata) - reduced-size metadata extent refs (0x100, 3.10, default)
no-holes (no_holes)               - no explicit hole extents for files (0x200, 3.14)

btrfs-convert -O list-all
Filesystem features available:
extref (extended_iref)            - increased hardlink limit per file to 65536 (0x40, 3.7, default)
skinny-metadata (skinny_metadata) - reduced-size metadata extent refs (0x100, 3.10, default)
no-holes (no_holes)               - no explicit hole extents for files (0x200, 3.14)
---
 utils.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/utils.c b/utils.c
index 0163915..6d2675d 100644
--- a/utils.c
+++ b/utils.c
@@ -648,17 +648,26 @@ void btrfs_process_fs_features(u64 flags)
 void btrfs_list_all_fs_features(u64 mask_disallowed)
 {
 	int i;
+	u64 feature_per_sysfs;
+
+	btrfs_features_allowed_by_sysfs(&feature_per_sysfs);
 
 	fprintf(stderr, "Filesystem features available:\n");
 	for (i = 0; i < ARRAY_SIZE(mkfs_features) - 1; i++) {
 		char *is_default = "";
+		char name[256];
 
 		if (mkfs_features[i].flag & mask_disallowed)
 			continue;
 		if (mkfs_features[i].flag & BTRFS_MKFS_DEFAULT_FEATURES)
 			is_default = ", default";
-		fprintf(stderr, "%-20s- %s (0x%llx, %s%s)\n",
-				mkfs_features[i].name,
+		if (mkfs_features[i].flag & feature_per_sysfs)
+			sprintf(name, "%s (%s)",
+				mkfs_features[i].name, mkfs_features[i].name_ker);
+		else
+			sprintf(name, "%s", mkfs_features[i].name);
+		fprintf(stderr, "%-34s- %s (0x%llx, %s%s)\n",
+				name,
 				mkfs_features[i].desc,
 				mkfs_features[i].flag,
 				mkfs_features[i].min_ker_ver,
-- 
2.6.2

[PATCH V1.1 2/7] btrfs-progs: add kernel alias for each of the features in the list

[Anand Jain]

We should have maintained feature's name same across progs UI and sysfs UI.
For example, progs mixed-bg is /sys/fs/btrfs/features/mixed_groups
in sysfs. As these are already released and is UIs, there is nothing much
can be done about it, except for creating the alias and making it aware.

Add kernel alias for each of the features in the list.

eg: The string with in () is the sysfs name for the same feaure

mkfs.btrfs -O list-all
Filesystem features available:
mixed-bg (mixed_groups)           - mixed data and metadata block groups (0x4, 2.7.37)
extref (extended_iref)            - increased hardlink limit per file to 65536 (0x40, 3.7, default)
raid56 (raid56)                   - raid56 extended format (0x80, 3.9)
skinny-metadata (skinny_metadata) - reduced-size metadata extent refs (0x100, 3.10, default)
no-holes (no_holes)               - no explicit hole extents for files (0x200, 3.14)

btrfs-convert -O list-all
Filesystem features available:
extref (extended_iref)            - increased hardlink limit per file to 65536 (0x40, 3.7, default)
skinny-metadata (skinny_metadata) - reduced-size metadata extent refs (0x100, 3.10, default)
no-holes (no_holes)               - no explicit hole extents for files (0x200, 3.14)

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
V1.1 add signed-off-by

 utils.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/utils.c b/utils.c
index 0163915..6d2675d 100644
--- a/utils.c
+++ b/utils.c
@@ -648,17 +648,26 @@ void btrfs_process_fs_features(u64 flags)
 void btrfs_list_all_fs_features(u64 mask_disallowed)
 {
 	int i;
+	u64 feature_per_sysfs;
+
+	btrfs_features_allowed_by_sysfs(&feature_per_sysfs);
 
 	fprintf(stderr, "Filesystem features available:\n");
 	for (i = 0; i < ARRAY_SIZE(mkfs_features) - 1; i++) {
 		char *is_default = "";
+		char name[256];
 
 		if (mkfs_features[i].flag & mask_disallowed)
 			continue;
 		if (mkfs_features[i].flag & BTRFS_MKFS_DEFAULT_FEATURES)
 			is_default = ", default";
-		fprintf(stderr, "%-20s- %s (0x%llx, %s%s)\n",
-				mkfs_features[i].name,
+		if (mkfs_features[i].flag & feature_per_sysfs)
+			sprintf(name, "%s (%s)",
+				mkfs_features[i].name, mkfs_features[i].name_ker);
+		else
+			sprintf(name, "%s", mkfs_features[i].name);
+		fprintf(stderr, "%-34s- %s (0x%llx, %s%s)\n",
+				name,
 				mkfs_features[i].desc,
 				mkfs_features[i].flag,
 				mkfs_features[i].min_ker_ver,
-- 
2.6.2

Re: [PATCH 2/7] btrfs-progs: add kernel alias for each of the features in the list

[Liu Bo]

On Wed, Nov 25, 2015 at 08:08:15PM +0800, Anand Jain wrote:
> We should have maintained feature's name same across progs UI and sysfs UI.
> For example, progs mixed-bg is /sys/fs/btrfs/features/mixed_groups
> in sysfs. As these are already released and is UIs, there is nothing much
> can be done about it, except for creating the alias and making it aware.
> 
> Add kernel alias for each of the features in the list.
> 
> eg: The string with in () is the sysfs name for the same feaure
> 
> mkfs.btrfs -O list-all
> Filesystem features available:
> mixed-bg (mixed_groups)           - mixed data and metadata block groups (0x4, 2.7.37)
> extref (extended_iref)            - increased hardlink limit per file to 65536 (0x40, 3.7, default)
> raid56 (raid56)                   - raid56 extended format (0x80, 3.9)
> skinny-metadata (skinny_metadata) - reduced-size metadata extent refs (0x100, 3.10, default)
> no-holes (no_holes)               - no explicit hole extents for files (0x200, 3.14)
> 
> btrfs-convert -O list-all
> Filesystem features available:
> extref (extended_iref)            - increased hardlink limit per file to 65536 (0x40, 3.7, default)
> skinny-metadata (skinny_metadata) - reduced-size metadata extent refs (0x100, 3.10, default)
> no-holes (no_holes)               - no explicit hole extents for files (0x200, 3.14)

You miss a signed-off-by here.

Thanks,

-liubo
> ---
>  utils.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/utils.c b/utils.c
> index 0163915..6d2675d 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -648,17 +648,26 @@ void btrfs_process_fs_features(u64 flags)
>  void btrfs_list_all_fs_features(u64 mask_disallowed)
>  {
>  	int i;
> +	u64 feature_per_sysfs;
> +
> +	btrfs_features_allowed_by_sysfs(&feature_per_sysfs);
>  
>  	fprintf(stderr, "Filesystem features available:\n");
>  	for (i = 0; i < ARRAY_SIZE(mkfs_features) - 1; i++) {
>  		char *is_default = "";
> +		char name[256];
>  
>  		if (mkfs_features[i].flag & mask_disallowed)
>  			continue;
>  		if (mkfs_features[i].flag & BTRFS_MKFS_DEFAULT_FEATURES)
>  			is_default = ", default";
> -		fprintf(stderr, "%-20s- %s (0x%llx, %s%s)\n",
> -				mkfs_features[i].name,
> +		if (mkfs_features[i].flag & feature_per_sysfs)
> +			sprintf(name, "%s (%s)",
> +				mkfs_features[i].name, mkfs_features[i].name_ker);
> +		else
> +			sprintf(name, "%s", mkfs_features[i].name);
> +		fprintf(stderr, "%-34s- %s (0x%llx, %s%s)\n",
> +				name,
>  				mkfs_features[i].desc,
>  				mkfs_features[i].flag,
>  				mkfs_features[i].min_ker_ver,
> -- 
> 2.6.2
> 
> --
> 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: [PATCH 2/7] btrfs-progs: add kernel alias for each of the features in the list

[Anand Jain]

Liu Bo wrote:
> On Wed, Nov 25, 2015 at 08:08:15PM +0800, Anand Jain wrote:
>> We should have maintained feature's name same across progs UI and sysfs UI.
>> For example, progs mixed-bg is /sys/fs/btrfs/features/mixed_groups
>> in sysfs. As these are already released and is UIs, there is nothing much
>> can be done about it, except for creating the alias and making it aware.
>>
>> Add kernel alias for each of the features in the list.
>>
>> eg: The string with in () is the sysfs name for the same feaure
>>
>> mkfs.btrfs -O list-all
>> Filesystem features available:
>> mixed-bg (mixed_groups)           - mixed data and metadata block groups (0x4, 2.7.37)
>> extref (extended_iref)            - increased hardlink limit per file to 65536 (0x40, 3.7, default)
>> raid56 (raid56)                   - raid56 extended format (0x80, 3.9)
>> skinny-metadata (skinny_metadata) - reduced-size metadata extent refs (0x100, 3.10, default)
>> no-holes (no_holes)               - no explicit hole extents for files (0x200, 3.14)
>>
>> btrfs-convert -O list-all
>> Filesystem features available:
>> extref (extended_iref)            - increased hardlink limit per file to 65536 (0x40, 3.7, default)
>> skinny-metadata (skinny_metadata) - reduced-size metadata extent refs (0x100, 3.10, default)
>> no-holes (no_holes)               - no explicit hole extents for files (0x200, 3.14)
>
> You miss a signed-off-by here.

  oh no. thanks for the catch.

> Thanks,
>
> -liubo
>> ---
>>   utils.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/utils.c b/utils.c
>> index 0163915..6d2675d 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -648,17 +648,26 @@ void btrfs_process_fs_features(u64 flags)
>>   void btrfs_list_all_fs_features(u64 mask_disallowed)
>>   {
>>   	int i;
>> +	u64 feature_per_sysfs;
>> +
>> +	btrfs_features_allowed_by_sysfs(&feature_per_sysfs);
>>
>>   	fprintf(stderr, "Filesystem features available:\n");
>>   	for (i = 0; i < ARRAY_SIZE(mkfs_features) - 1; i++) {
>>   		char *is_default = "";
>> +		char name[256];
>>
>>   		if (mkfs_features[i].flag & mask_disallowed)
>>   			continue;
>>   		if (mkfs_features[i].flag & BTRFS_MKFS_DEFAULT_FEATURES)
>>   			is_default = ", default";
>> -		fprintf(stderr, "%-20s- %s (0x%llx, %s%s)\n",
>> -				mkfs_features[i].name,
>> +		if (mkfs_features[i].flag & feature_per_sysfs)
>> +			sprintf(name, "%s (%s)",
>> +				mkfs_features[i].name, mkfs_features[i].name_ker);
>> +		else
>> +			sprintf(name, "%s", mkfs_features[i].name);
>> +		fprintf(stderr, "%-34s- %s (0x%llx, %s%s)\n",
>> +				name,
>>   				mkfs_features[i].desc,
>>   				mkfs_features[i].flag,
>>   				mkfs_features[i].min_ker_ver,
>> --
>> 2.6.2
>>
>> --
>> 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
>

[PATCH 3/7] btrfs-progs: make is_numerical non static

[Anand Jain]

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-replace.c | 11 -----------
 utils.c        | 11 +++++++++++
 utils.h        |  1 +
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/cmds-replace.c b/cmds-replace.c
index 9ab8438..86162b6 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -65,17 +65,6 @@ static const char * const replace_cmd_group_usage[] = {
 	NULL
 };
 
-static int is_numerical(const char *str)
-{
-	if (!(*str >= '0' && *str <= '9'))
-		return 0;
-	while (*str >= '0' && *str <= '9')
-		str++;
-	if (*str != '\0')
-		return 0;
-	return 1;
-}
-
 static int dev_replace_cancel_fd = -1;
 static void dev_replace_sigint_handler(int signal)
 {
diff --git a/utils.c b/utils.c
index 6d2675d..0e66e2b 100644
--- a/utils.c
+++ b/utils.c
@@ -178,6 +178,17 @@ int test_uuid_unique(char *fs_uuid)
 	return unique;
 }
 
+int is_numerical(const char *str)
+{
+	if (!(*str >= '0' && *str <= '9'))
+		return 0;
+	while (*str >= '0' && *str <= '9')
+		str++;
+	if (*str != '\0')
+		return 0;
+	return 1;
+}
+
 /*
  * @fs_uuid - if NULL, generates a UUID, returns back the new filesystem UUID
  */
diff --git a/utils.h b/utils.h
index af0aa31..cb20d73 100644
--- a/utils.h
+++ b/utils.h
@@ -271,5 +271,6 @@ const char *get_argv0_buf(void);
 	"-t|--tbytes        show sizes in TiB, or TB with --si"
 
 unsigned int get_unit_mode_from_arg(int *argc, char *argv[], int df_mode);
+int is_numerical(const char *str);
 
 #endif
-- 
2.6.2

[PATCH 4/7] btrfs-progs: check for numerical in version_to_code()

[Anand Jain]

As the version is now being passed by user it should be checked
if its numerical. We didn't need this before as version wasn't
passed by used. So this is not a bug fix.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 utils.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/utils.c b/utils.c
index 0e66e2b..216efa6 100644
--- a/utils.c
+++ b/utils.c
@@ -3119,14 +3119,18 @@ static int version_to_code(char *v)
 
 	for (b[i] = strtok_r(v, ".", &save_b);
 		b[i] != NULL;
-		b[i] = strtok_r(NULL, ".", &save_b))
+		b[i] = strtok_r(NULL, ".", &save_b)) {
+		if (!is_numerical(b[i]))
+			return -EINVAL;
 		i++;
+	}
 
+	if (b[1] == NULL)
+		return KERNEL_VERSION(atoi(b[0]), 0, 0);
 	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]));
 
+	return KERNEL_VERSION(atoi(b[0]), atoi(b[1]), atoi(b[2]));
 }
 
 static int get_kernel_code()
-- 
2.6.2

[PATCH 5/7] btrfs-progs: introduce framework version to features

[Anand Jain]

As discussed in the mailing list this provides a framework to introduce
the feature where mkfs and btrfs-convert can set the default features
as per the given mainline kernel version.

Suggested-by: David Sterba <dsterba@suse.cz>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 utils.c | 23 +++++++++++++++++++++++
 utils.h |  1 +
 2 files changed, 24 insertions(+)

diff --git a/utils.c b/utils.c
index 216efa6..a9b46b8 100644
--- a/utils.c
+++ b/utils.c
@@ -3222,3 +3222,26 @@ int btrfs_features_allowed_by_sysfs(u64 *features)
 	closedir(dir);
 	return 0;
 }
+
+int btrfs_features_allowed_by_version(char *version, u64 *features)
+{
+	int i;
+	int code;
+	char *ver = strdup(version);
+
+	*features = 0;
+	code = version_to_code(ver);
+	free(ver);
+	if (code < 0)
+		return code;
+
+	for (i = 0; i < ARRAY_SIZE(mkfs_features) - 1; i++) {
+		ver = strdup(mkfs_features[i].min_ker_ver);
+
+		if (code >= version_to_code(ver))
+			*features |= mkfs_features[i].flag;
+
+		free(ver);
+	}
+	return 0;
+}
diff --git a/utils.h b/utils.h
index cb20d73..1418e84 100644
--- a/utils.h
+++ b/utils.h
@@ -106,6 +106,7 @@ void btrfs_process_fs_features(u64 flags);
 void btrfs_parse_features_to_string(char *buf, u64 flags);
 u64 btrfs_features_allowed_by_kernel(void);
 int btrfs_features_allowed_by_sysfs(u64 *features);
+int btrfs_features_allowed_by_version(char *version, u64 *features);
 
 struct btrfs_mkfs_config {
 	char *label;
-- 
2.6.2

[PATCH 6/7] btrfs-progs: add -O comp= option for mkfs.btrfs

[Anand Jain]

This provides default feature set by version, for mkfs.btrfs
through the new option '-O comp=<x.y>|<x.y.z>', where x.y.z is the
minimum kernel version that should be supported.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 mkfs.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/mkfs.c b/mkfs.c
index 6cb998b..34ba77d 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -324,7 +324,9 @@ static void print_usage(int ret)
 	fprintf(stderr, "\t-s|--sectorsize SIZE    min block allocation (may not mountable by current kernel)\n");
 	fprintf(stderr, "\t-r|--rootdir DIR        the source directory\n");
 	fprintf(stderr, "\t-K|--nodiscard          do not perform whole device TRIM\n");
-	fprintf(stderr, "\t-O|--features LIST      comma separated list of filesystem features, use '-O list-all' to list features\n");
+	fprintf(stderr, "\t-O|--features LIST      comma separated list of filesystem features\n");
+	fprintf(stderr, "\t                          use '-O list-all' to list features\n");
+	fprintf(stderr, "\t                          use '-O comp=<x.y>|<x.y.z>' x.y.z is the minimum kernel version to be supported\n");
 	fprintf(stderr, "\t-U|--uuid UUID          specify the filesystem UUID\n");
 	fprintf(stderr, "\t-q|--quiet              no messages except errors\n");
 	fprintf(stderr, "\t-V|--version            print the mkfs.btrfs version and exit\n");
@@ -1439,7 +1441,24 @@ int main(int ac, char **av)
 			case 'O': {
 				char *orig = strdup(optarg);
 				char *tmp = orig;
-
+				char *tok;
+
+				tok = strtok(tmp, "=");
+				if (!strcmp(tok, "comp")) {
+					tok = strtok(NULL, "=");
+					if (!tok) {
+						fprintf(stderr,
+				"Provide a version for 'comp=' option, ref to 'mkfs.btrfs -O list-all'\n");
+						exit(1);
+					}
+					if (btrfs_features_allowed_by_version(tok, &features) < 0) {
+						fprintf(stderr, "Wrong version format: '%s'\n", tok);
+						exit(1);
+					}
+					features &= BTRFS_MKFS_DEFAULT_FEATURES;
+					goto cont;
+				}
+				tmp = orig;
 				tmp = btrfs_parse_fs_features(tmp, &features);
 				if (tmp) {
 					fprintf(stderr,
@@ -1448,6 +1467,7 @@ int main(int ac, char **av)
 					free(orig);
 					exit(1);
 				}
+cont:
 				free(orig);
 				if (features & BTRFS_FEATURE_LIST_ALL) {
 					btrfs_list_all_fs_features(0);
-- 
2.6.2

[PATCH 7/7] btrfs-progs: add -O comp= option for btrfs-convert

[Anand Jain]

User may want to convert the FS to a minimum kernel version. As they may need
to use btrfs on a set of known kernel versions. And have the disk layout compatible.

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

diff --git a/btrfs-convert.c b/btrfs-convert.c
index b0a998b..01b8940 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -2879,6 +2879,8 @@ static void print_usage(void)
 	printf("\t-L|--copy-label        use label from converted filesystem\n");
 	printf("\t-p|--progress          show converting progress (default)\n");
 	printf("\t-O|--features LIST     comma separated list of filesystem features\n");
+	printf("\t                         use '-O list-all' to list features\n");
+	printf("\t                         use '-O comp=<x.y>|<x.y.z>' x.y.z is the minimum kernel version to be supported\n");
 	printf("\t--no-progress          show only overview, not the detailed progress\n");
 }
 
@@ -2970,6 +2972,24 @@ int main(int argc, char *argv[])
 			case 'O': {
 				char *orig = strdup(optarg);
 				char *tmp = orig;
+				char *tok;
+
+				tok = strtok(tmp, "=");
+				if (!strcmp(tok, "comp")) {
+					tok = strtok(NULL, "=");
+					if (!tok) {
+						fprintf(stderr,
+				"Provide a version for 'comp=' option, ref to 'mkfs.btrfs -O list-all'\n");
+						exit(1);
+					}
+					if (btrfs_features_allowed_by_version(tok, &features) < 0) {
+						fprintf(stderr, "Wrong version format: '%s'\n", tok);
+						exit(1);
+					}
+					features &= BTRFS_MKFS_DEFAULT_FEATURES;
+					goto cont;
+				}
+				tmp = orig;
 
 				tmp = btrfs_parse_fs_features(tmp, &features);
 				if (tmp) {
@@ -2979,6 +2999,7 @@ int main(int argc, char *argv[])
 					free(orig);
 					exit(1);
 				}
+cont:
 				free(orig);
 				if (features & BTRFS_FEATURE_LIST_ALL) {
 					btrfs_list_all_fs_features(
-- 
2.6.2

Re: [PATCH 0/7] Let user specify the kernel version for features

[Qu Wenruo]

Anand Jain wrote on 2015/11/25 20:08 +0800:
> Sometimes users may want to have a btrfs to be supported on multiple
> kernel version. A simple example, USB drive can be used with multiple
> system running different kernel versions. Or in a data center a SAN
> LUN could be mounted on any system with different kernel version.
>
> Thanks for providing comments and feedback.
> Further to it, here below is a set of patch which will introduce, to
> specify a kernel version so that default features can be set based on
> what features were supported at that kernel version.

With the new -O comp= option, the concern on user who want to make a 
btrfs for newer kernel is hugely reduced.

But I still prefer such feature align to be done only when specified by 
user, instead of automatically. (yeah, already told for several times 
though)
Warning should be enough for user, sometimes too automatic is not  good, 
especially for tests.

A lot of btrfs-progs change, like recent disabling mixed-bg for small 
volume has already cause regression in generic/077 testcase.
And Dave is already fed up with such problem from btrfs...

Especially such auto-detection will make default behavior more unstable, 
at least not a good idea for me.



Beside this, I'm curious how other filesystm user tools handle such 
kernel mismatch, or do they?

Thanks,
Qu


>
> First of all to let user know what features was supported at what kernel
> version. Patch 1/7 updates -O list-all which will list the feature with
> version.
>
> As we didn't maintain the sysfs and progs feature names consistent, so
> to avoid confusion Patch 2/7 displays sysfs feature name as well again
> in the list-all output.
>
> Next, Patch 3,4,5/7 are helper functions.
>
> Patch 6,7/7 provides the -O comp=<version> for mkfs.btrfs and
> btrfs-convert respectively
>
> Thanks, Anand
>
> Anand Jain (7):
>    btrfs-progs: show the version for -O list-all
>    btrfs-progs: add kernel alias for each of the features in the list
>    btrfs-progs: make is_numerical non static
>    btrfs-progs: check for numerical in version_to_code()
>    btrfs-progs: introduce framework version to features
>    btrfs-progs: add -O comp= option for mkfs.btrfs
>    btrfs-progs: add -O comp= option for btrfs-convert
>
>   btrfs-convert.c | 21 +++++++++++++++++++++
>   cmds-replace.c  | 11 -----------
>   mkfs.c          | 24 ++++++++++++++++++++++--
>   utils.c         | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>   utils.h         |  2 ++
>   5 files changed, 98 insertions(+), 18 deletions(-)
>

Re: [PATCH 0/7] Let user specify the kernel version for features

[Anand Jain]

On 11/26/2015 10:02 AM, Qu Wenruo wrote:
>
>
> Anand Jain wrote on 2015/11/25 20:08 +0800:
>> Sometimes users may want to have a btrfs to be supported on multiple
>> kernel version. A simple example, USB drive can be used with multiple
>> system running different kernel versions. Or in a data center a SAN
>> LUN could be mounted on any system with different kernel version.
>>
>> Thanks for providing comments and feedback.
>> Further to it, here below is a set of patch which will introduce, to
>> specify a kernel version so that default features can be set based on
>> what features were supported at that kernel version.
>
> With the new -O comp= option, the concern on user who want to make a
> btrfs for newer kernel is hugely reduced.

NO!. actually new option -O comp= provides no concern for users who
want to create _a btrfs disk layout which is compatible with more
than one kernel_.  above there are two examples of it.

> But I still prefer such feature align to be done only when specified by
> user, instead of automatically. (yeah, already told for several times
> though)
> Warning should be enough for user, sometimes too automatic is not  good,

As said before.
We need latest btrfs-progs on older kernels, for obvious reasons of
btrfs-progs bug fixes. We don't have to back port fixes even on
btrfs-progs as we already do it in btrfs kernel. A btrfs-progs should
work on any kernel with the "default features as prescribed for that
kernel".

Let's say if we don't do this automatic then, latest btrfs-progs
with default mkfs.btfs && mount fails. But a user upgrading btrfs-progs
for fsck bug fixes, shouldn't find 'default mkfs.btfs && mount'
failing. Nor they have to use a "new" set of mkfs option to create all
default FS for a LTS kernel.

Default features based on btrfs-progs version instead of kernel
version- makes NO sense. And adding a warning for not using latest
features which is not in their running kernel is pointless. That's
_not_ a backward kernel compatible tool.

btrfs-progs should work "for the kernel". We should avoid adding too
much intelligence into btrfs-progs. I have fixed too many issues and
redesigned progs in this area. Too many bugs were mainly because of the
idea of copy and maintain same code on btrfs-progs and btrfs-kernel
approach for progs. (ref wiki and my email before). Thats a wrong
approach. I don't understand- if the purpose of both of these isn't
same what is the point in maintaining same code? It won't save in
efforts mainly because its like developing a distributed FS where
two parties has to be communicated to be in sync. Which is like using
the canon to shoo a crow.
But if the reason was fuse like kernel-free FS (no one said that
though) then its better to do it as a separate project.

> especially for tests.

It depends whats being tested kernel OR progs? Its kernel not progs.
Automatic will keep default feature constant for a given kernel
version. Further, for testing using a known set of options is even
better.

> A lot of btrfs-progs change, like recent disabling mixed-bg for small
> volume has already cause regression in generic/077 testcase.
> And Dave is already fed up with such problem from btrfs...

I don't know what's the regression about. But in my experience with
some xfstest test cases.. xfstests depend too much on cli output
strings which is easy thing to do but a wrong approach.
Those cli outputs and its format are NOT APIs, those are UIs. Instead
it should have used return code/ FS test interface. This will let
developers with free hands to change, otherwise you need to update the
test cases every time you change the cli _output_.

> Especially such auto-detection will make default behavior more unstable,
> at least not a good idea for me.

As above. We design with end-user and their use cases in mind. Not for
a test suite. If test suite breaks.. fix it.

Thanks, Anand

> Beside this, I'm curious how other filesystm user tools handle such
> kernel mismatch, or do they?

> Thanks,
> Qu
>
>
>>
>> First of all to let user know what features was supported at what kernel
>> version. Patch 1/7 updates -O list-all which will list the feature with
>> version.
>>
>> As we didn't maintain the sysfs and progs feature names consistent, so
>> to avoid confusion Patch 2/7 displays sysfs feature name as well again
>> in the list-all output.
>>
>> Next, Patch 3,4,5/7 are helper functions.
>>
>> Patch 6,7/7 provides the -O comp=<version> for mkfs.btrfs and
>> btrfs-convert respectively
>>
>> Thanks, Anand
>>
>> Anand Jain (7):
>>    btrfs-progs: show the version for -O list-all
>>    btrfs-progs: add kernel alias for each of the features in the list
>>    btrfs-progs: make is_numerical non static
>>    btrfs-progs: check for numerical in version_to_code()
>>    btrfs-progs: introduce framework version to features
>>    btrfs-progs: add -O comp= option for mkfs.btrfs
>>    btrfs-progs: add -O comp= option for btrfs-convert
>>
>>   btrfs-convert.c | 21 +++++++++++++++++++++
>>   cmds-replace.c  | 11 -----------
>>   mkfs.c          | 24 ++++++++++++++++++++++--
>>   utils.c         | 58
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>>   utils.h         |  2 ++
>>   5 files changed, 98 insertions(+), 18 deletions(-)
>>
>
>
> --
> 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: [PATCH 0/7] Let user specify the kernel version for features

[Qu Wenruo]

Anand Jain wrote on 2015/11/26 14:07 +0800:
>
>
> On 11/26/2015 10:02 AM, Qu Wenruo wrote:
>>
>>
>> Anand Jain wrote on 2015/11/25 20:08 +0800:
>>> Sometimes users may want to have a btrfs to be supported on multiple
>>> kernel version. A simple example, USB drive can be used with multiple
>>> system running different kernel versions. Or in a data center a SAN
>>> LUN could be mounted on any system with different kernel version.
>>>
>>> Thanks for providing comments and feedback.
>>> Further to it, here below is a set of patch which will introduce, to
>>> specify a kernel version so that default features can be set based on
>>> what features were supported at that kernel version.
>>
>> With the new -O comp= option, the concern on user who want to make a
>> btrfs for newer kernel is hugely reduced.
>
> NO!. actually new option -O comp= provides no concern for users who
> want to create _a btrfs disk layout which is compatible with more
> than one kernel_.  above there are two examples of it.

Why you can't give a higher kernel version than current kernel?

>
>> But I still prefer such feature align to be done only when specified by
>> user, instead of automatically. (yeah, already told for several times
>> though)
>> Warning should be enough for user, sometimes too automatic is not  good,
>
> As said before.
> We need latest btrfs-progs on older kernels, for obvious reasons of
> btrfs-progs bug fixes. We don't have to back port fixes even on
> btrfs-progs as we already do it in btrfs kernel. A btrfs-progs should
> work on any kernel with the "default features as prescribed for that
> kernel".
>
> Let's say if we don't do this automatic then, latest btrfs-progs
> with default mkfs.btfs && mount fails. But a user upgrading btrfs-progs
> for fsck bug fixes, shouldn't find 'default mkfs.btfs && mount'
> failing. Nor they have to use a "new" set of mkfs option to create all
> default FS for a LTS kernel.
>
> Default features based on btrfs-progs version instead of kernel
> version- makes NO sense.

Kernel version never makes sense, especially for non-vanilla.
And unfortunately, most of kernels used in stable distribution is not 
vanilla.
And that's the *POINT1*.

That's why I stand against kernel version based detection.
You can use stable /sys/fs/btrfs/features/, but kernel version?
Not an option even as fallback.

> And adding a warning for not using latest
> features which is not in their running kernel is pointless.

You didn't get the point of what to WARN.

Not warning user they are not using latest features, but warning some 
features may prevent the fs being mounted for current kernel.

>That's _not_ a backward kernel compatible tool.
>
> btrfs-progs should work "for the kernel". We should avoid adding too
> much intelligence into btrfs-progs. I have fixed too many issues and
> redesigned progs in this area. Too many bugs were mainly because of the
> idea of copy and maintain same code on btrfs-progs and btrfs-kernel
> approach for progs. (ref wiki and my email before). Thats a wrong
> approach.

Totally agree with this point. Too many non-sense in btrfs-progs codes 
copied from kernel, and due to lack of update, it's very buggy now.
Just check volume.c for allocating data chunk.

But I didn't see the point related to the feature auto align here.

> I don't understand- if the purpose of both of these isn't
> same what is the point in maintaining same code? It won't save in
> efforts mainly because its like developing a distributed FS where
> two parties has to be communicated to be in sync. Which is like using
> the canon to shoo a crow.
> But if the reason was fuse like kernel-free FS (no one said that
> though) then its better to do it as a separate project.
>
>> especially for tests.
>
> It depends whats being tested kernel OR progs? Its kernel not progs.

No, both kernel and progs. Just from Dave, even with his typo:

"xfstests is not jsut for testing kernel changes - it tests all of
the filesystem utilities for regressions, too. And so when
inadvertant changes in default behaviour occur, it detects those
regressions too."


> Automatic will keep default feature constant for a given kernel
> version. Further, for testing using a known set of options is even
> better.

Yeah, known set of options get unknown on different kernels, thanks to 
the hidden feature align. Unless you specify it by -O options.

That's the *POINT2*:
Default auto feature align are making mkfs.btrfs behavior *unpredictable*.

Before auto feature align, QA/end-user only needs to check the 
btrfs-progs announcement to know the default behavior change.

And after it, wow, QA testers will need to check the feature matrix to 
know what's the default feature on their kernel, not to mention it may 
even be wrong due to more unpredictable kernel version.

That's why I strongly recommend to make it just a warning other than 
default behavior.

>
>> A lot of btrfs-progs change, like recent disabling mixed-bg for small
>> volume has already cause regression in generic/077 testcase.
>> And Dave is already fed up with such problem from btrfs...
>
> I don't know what's the regression about. But in my experience with
> some xfstest test cases.. xfstests depend too much on cli output
> strings which is easy thing to do but a wrong approach.

Check this patch and its following, definitely not UI but default 
behavior, just as you are going to change.

https://patchwork.kernel.org/patch/7679381/

Thanks,
Qu

> Those cli outputs and its format are NOT APIs, those are UIs. Instead
> it should have used return code/ FS test interface. This will let
> developers with free hands to change, otherwise you need to update the
> test cases every time you change the cli _output_.
>
>> Especially such auto-detection will make default behavior more unstable,
>> at least not a good idea for me.
>
> As above. We design with end-user and their use cases in mind. Not for
> a test suite. If test suite breaks.. fix it.
>
> Thanks, Anand
>
>> Beside this, I'm curious how other filesystm user tools handle such
>> kernel mismatch, or do they?
>
>> Thanks,
>> Qu
>>
>>
>>>
>>> First of all to let user know what features was supported at what kernel
>>> version. Patch 1/7 updates -O list-all which will list the feature with
>>> version.
>>>
>>> As we didn't maintain the sysfs and progs feature names consistent, so
>>> to avoid confusion Patch 2/7 displays sysfs feature name as well again
>>> in the list-all output.
>>>
>>> Next, Patch 3,4,5/7 are helper functions.
>>>
>>> Patch 6,7/7 provides the -O comp=<version> for mkfs.btrfs and
>>> btrfs-convert respectively
>>>
>>> Thanks, Anand
>>>
>>> Anand Jain (7):
>>>    btrfs-progs: show the version for -O list-all
>>>    btrfs-progs: add kernel alias for each of the features in the list
>>>    btrfs-progs: make is_numerical non static
>>>    btrfs-progs: check for numerical in version_to_code()
>>>    btrfs-progs: introduce framework version to features
>>>    btrfs-progs: add -O comp= option for mkfs.btrfs
>>>    btrfs-progs: add -O comp= option for btrfs-convert
>>>
>>>   btrfs-convert.c | 21 +++++++++++++++++++++
>>>   cmds-replace.c  | 11 -----------
>>>   mkfs.c          | 24 ++++++++++++++++++++++--
>>>   utils.c         | 58
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>>>   utils.h         |  2 ++
>>>   5 files changed, 98 insertions(+), 18 deletions(-)
>>>
>>
>>
>> --
>> 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: [PATCH 0/7] Let user specify the kernel version for features

[Anand Jain]

>>> With the new -O comp= option, the concern on user who want to make a
>>> btrfs for newer kernel is hugely reduced.
>>
>> NO!. actually new option -O comp= provides no concern for users who
>> want to create _a btrfs disk layout which is compatible with more
>> than one kernel_.  above there are two examples of it.
>
> Why you can't give a higher kernel version than current kernel?

  mount fails.  Pls try !!


>>
>>> But I still prefer such feature align to be done only when specified by
>>> user, instead of automatically. (yeah, already told for several times
>>> though)
>>> Warning should be enough for user, sometimes too automatic is not  good,
>>
>> As said before.
>> We need latest btrfs-progs on older kernels, for obvious reasons of
>> btrfs-progs bug fixes. We don't have to back port fixes even on
>> btrfs-progs as we already do it in btrfs kernel. A btrfs-progs should
>> work on any kernel with the "default features as prescribed for that
>> kernel".
>>
>> Let's say if we don't do this automatic then, latest btrfs-progs
>> with default mkfs.btfs && mount fails. But a user upgrading btrfs-progs
>> for fsck bug fixes, shouldn't find 'default mkfs.btfs && mount'
>> failing. Nor they have to use a "new" set of mkfs option to create all
>> default FS for a LTS kernel.
>>
>> Default features based on btrfs-progs version instead of kernel
>> version- makes NO sense.
>
> Kernel version never makes sense, especially for non-vanilla.
 >
> And unfortunately, most of kernels used in stable distribution is not
> vanilla.
> And that's the *POINT1*.
>
> That's why I stand against kernel version based detection.
> You can use stable /sys/fs/btrfs/features/, but kernel version?
> Not an option even as fallback.

yep. thats the reason someone invented sysfs/features, but
unfortunately from 3.14. If not version, pls suggest best suitable,
_without_ transferring problem to solve to the user end.


>> And adding a warning for not using latest
>> features which is not in their running kernel is pointless.

  In the context of default features.


>
> You didn't get the point of what to WARN.
>
> Not warning user they are not using latest features, but warning some
> features may prevent the fs being mounted for current kernel.

  It was there before. some patch in the past removed it. hope you
  remember "Turning on incompatible..."


>> That's _not_ a backward kernel compatible tool.
>>
>> btrfs-progs should work "for the kernel". We should avoid adding too
>> much intelligence into btrfs-progs. I have fixed too many issues and
>> redesigned progs in this area. Too many bugs were mainly because of the
>> idea of copy and maintain same code on btrfs-progs and btrfs-kernel
>> approach for progs. (ref wiki and my email before). Thats a wrong
>> approach.
>
> Totally agree with this point. Too many non-sense in btrfs-progs codes
> copied from kernel, and due to lack of update, it's very buggy now.
> Just check volume.c for allocating data chunk.
>
> But I didn't see the point related to the feature auto align here.

  The whole point is don't add more intelligence into progs than what
  is required.

  Here its about default features. And the questions are
  Default against of what ? To the btrfs-progs version itself?
  Why do you want to add another attribute of btrfs-progs-version
  being relevant at the user end ? For users that's like progs not
  being inline with kernel, a very strong problem statement.

  (bit vague as of now) potentially there are chances that someday
  we would move mkfs part into the kernel itself, makes progs as
  slick as possible.


>> I don't understand- if the purpose of both of these isn't
>> same what is the point in maintaining same code? It won't save in
>> efforts mainly because its like developing a distributed FS where
>> two parties has to be communicated to be in sync. Which is like using
>> the canon to shoo a crow.
>> But if the reason was fuse like kernel-free FS (no one said that
>> though) then its better to do it as a separate project.
>>
>>> especially for tests.
>>
>> It depends whats being tested kernel OR progs? Its kernel not progs.
>
> No, both kernel and progs. Just from Dave, even with his typo:
>
> "xfstests is not jsut for testing kernel changes - it tests all of
> the filesystem utilities for regressions, too. And so when
> inadvertant changes in default behaviour occur, it detects those
> regressions too."

  Now in this context if you are testing latest btrfs-progs (without
  these patches) on an old LTS kernel, and using default mkfs option
  all tests fails. That's something to fix. Without transferring
  implementation difficulties to the user end. And without changing
  xfstests mkfs_options. Because we claim progs is backward kernel
  compatible.


>> Automatic will keep default feature constant for a given kernel
>> version. Further, for testing using a known set of options is even
>> better.
>
> Yeah, known set of options get unknown on different kernels, thanks to
> the hidden feature align. Unless you specify it by -O options.
>
> That's the *POINT2*:
> Default auto feature align are making mkfs.btrfs behavior *unpredictable*.
 >
> Before auto feature align, QA/end-user only needs to check the
> btrfs-progs announcement to know the default behavior change.
>
> And after it, wow, QA testers will need to check the feature matrix to
> know what's the default feature on their kernel, not to mention it may
> even be wrong due to more unpredictable kernel version.
>
> That's why I strongly recommend to make it just a warning other than
> default behavior.

  The use cases also involves old LTS kernels on newer progs.
  which your point2 does _not_ cover and where the whole point
  of discussion is.

>>
>>> A lot of btrfs-progs change, like recent disabling mixed-bg for small
>>> volume has already cause regression in generic/077 testcase.
>>> And Dave is already fed up with such problem from btrfs...
>>
>> I don't know what's the regression about. But in my experience with
>> some xfstest test cases.. xfstests depend too much on cli output
>> strings which is easy thing to do but a wrong approach.
>
> Check this patch and its following, definitely not UI but default
> behavior, just as you are going to change.
>
> https://patchwork.kernel.org/patch/7679381/

  Reviewed. I understand xfstest didn't fail for an UI change.
  So how does that apply here in this context ?

  Note:
  Before
  'A progs and A kernel' set = default features are constant.
  Now
  'ANY progs and A kernel' set = default features are constant.

  So what is that it breaks ? I want to know as well.

Thanks, Anand


> Thanks,
> Qu
>
>> Those cli outputs and its format are NOT APIs, those are UIs. Instead
>> it should have used return code/ FS test interface. This will let
>> developers with free hands to change, otherwise you need to update the
>> test cases every time you change the cli _output_.
>>
>>> Especially such auto-detection will make default behavior more unstable,
>>> at least not a good idea for me.
>>
>> As above. We design with end-user and their use cases in mind. Not for
>> a test suite. If test suite breaks.. fix it.
>>
>> Thanks, Anand
>>
>>> Beside this, I'm curious how other filesystm user tools handle such
>>> kernel mismatch, or do they?
>>
>>> Thanks,
>>> Qu
>>>
>>>
>>>>
>>>> First of all to let user know what features was supported at what
>>>> kernel
>>>> version. Patch 1/7 updates -O list-all which will list the feature with
>>>> version.
>>>>
>>>> As we didn't maintain the sysfs and progs feature names consistent, so
>>>> to avoid confusion Patch 2/7 displays sysfs feature name as well again
>>>> in the list-all output.
>>>>
>>>> Next, Patch 3,4,5/7 are helper functions.
>>>>
>>>> Patch 6,7/7 provides the -O comp=<version> for mkfs.btrfs and
>>>> btrfs-convert respectively
>>>>
>>>> Thanks, Anand
>>>>
>>>> Anand Jain (7):
>>>>    btrfs-progs: show the version for -O list-all
>>>>    btrfs-progs: add kernel alias for each of the features in the list
>>>>    btrfs-progs: make is_numerical non static
>>>>    btrfs-progs: check for numerical in version_to_code()
>>>>    btrfs-progs: introduce framework version to features
>>>>    btrfs-progs: add -O comp= option for mkfs.btrfs
>>>>    btrfs-progs: add -O comp= option for btrfs-convert
>>>>
>>>>   btrfs-convert.c | 21 +++++++++++++++++++++
>>>>   cmds-replace.c  | 11 -----------
>>>>   mkfs.c          | 24 ++++++++++++++++++++++--
>>>>   utils.c         | 58
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>>>>   utils.h         |  2 ++
>>>>   5 files changed, 98 insertions(+), 18 deletions(-)
>>>>
>>>
>>>
>>> --
>>> 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: [PATCH 0/7] Let user specify the kernel version for features

[Qu Wenruo]

On 11/26/2015 07:18 PM, Anand Jain wrote:
>
>
>
>>>> With the new -O comp= option, the concern on user who want to make a
>>>> btrfs for newer kernel is hugely reduced.
>>>
>>> NO!. actually new option -O comp= provides no concern for users who
>>> want to create _a btrfs disk layout which is compatible with more
>>> than one kernel_.  above there are two examples of it.
>>
>> Why you can't give a higher kernel version than current kernel?
>
>   mount fails.  Pls try !!

But that's what user want to do. He/she knows what he is doing.
Maybe he is testing btrfs-progs self test without the need to mount 
it(at least some of the tests doesn't require mount)

>
>
>>>
>>>> But I still prefer such feature align to be done only when specified by
>>>> user, instead of automatically. (yeah, already told for several times
>>>> though)
>>>> Warning should be enough for user, sometimes too automatic is not
>>>> good,
>>>
>>> As said before.
>>> We need latest btrfs-progs on older kernels, for obvious reasons of
>>> btrfs-progs bug fixes. We don't have to back port fixes even on
>>> btrfs-progs as we already do it in btrfs kernel. A btrfs-progs should
>>> work on any kernel with the "default features as prescribed for that
>>> kernel".
>>>
>>> Let's say if we don't do this automatic then, latest btrfs-progs
>>> with default mkfs.btfs && mount fails. But a user upgrading btrfs-progs
>>> for fsck bug fixes, shouldn't find 'default mkfs.btfs && mount'
>>> failing. Nor they have to use a "new" set of mkfs option to create all
>>> default FS for a LTS kernel.
>>>
>>> Default features based on btrfs-progs version instead of kernel
>>> version- makes NO sense.
>>
>> Kernel version never makes sense, especially for non-vanilla.
>  >
>> And unfortunately, most of kernels used in stable distribution is not
>> vanilla.
>> And that's the *POINT1*.
>>
>> That's why I stand against kernel version based detection.
>> You can use stable /sys/fs/btrfs/features/, but kernel version?
>> Not an option even as fallback.
>
> yep. thats the reason someone invented sysfs/features, but
> unfortunately from 3.14. If not version, pls suggest best suitable,
> _without_ transferring problem to solve to the user end.

A solution which may cause wrong result is never a solution.

No matter if it is better than any other one.

Do it wrong(even sometimes it's OK) is never good than doing nothing.

>
>
>>> And adding a warning for not using latest
>>> features which is not in their running kernel is pointless.
>
>   In the context of default features.
>
>
>>
>> You didn't get the point of what to WARN.
>>
>> Not warning user they are not using latest features, but warning some
>> features may prevent the fs being mounted for current kernel.
>
>   It was there before. some patch in the past removed it. hope you
>   remember "Turning on incompatible..."

Then, add it back, and not such informative, but warning.
The old output is so easy to ignore.

Or you can just stop continuing mkfs if you detect such incompact 
feature with current kernel, only continue if "-f" is given.

>
>
>>> That's _not_ a backward kernel compatible tool.
>>>
>>> btrfs-progs should work "for the kernel". We should avoid adding too
>>> much intelligence into btrfs-progs. I have fixed too many issues and
>>> redesigned progs in this area. Too many bugs were mainly because of the
>>> idea of copy and maintain same code on btrfs-progs and btrfs-kernel
>>> approach for progs. (ref wiki and my email before). Thats a wrong
>>> approach.
>>
>> Totally agree with this point. Too many non-sense in btrfs-progs codes
>> copied from kernel, and due to lack of update, it's very buggy now.
>> Just check volume.c for allocating data chunk.
>>
>> But I didn't see the point related to the feature auto align here.
>
>   The whole point is don't add more intelligence into progs than what
>   is required.
>
>   Here its about default features. And the questions are
>   Default against of what ? To the btrfs-progs version itself?
>   Why do you want to add another attribute of btrfs-progs-version
>   being relevant at the user end ?

End user of what? A single package?

No, end user of the *whole distribution*.
That's the packager/backport guys responsible for not combining mismatch 
kernel(-LTS) and progs

Now we need to auto align feature with kernel, who know one day we will 
need to auto align our libs to upstream package?
Keeping a matrix with different packages like libuuid/acl/attr with 
different Makefile?
At least this is not a good idea for me, and that's the work of 
autoconfig IIRC.

And if I'm a package and face such problem, I'll choose the simplest 
solution, just add a line in PKGBUILD(package system of Archlinux) of btrfs.
------
depends=('linux>=3.14')
------
(Yeah, such simple and slick packaging solution is the reason I like 
Arch over other rolling distribution)

Not every thing really needed to be done in code level.


  For users that's like progs not
>   being inline with kernel, a very strong problem statement.
>
>   (bit vague as of now) potentially there are chances that someday
>   we would move mkfs part into the kernel itself, makes progs as
>   slick as possible.

>
>>> I don't understand- if the purpose of both of these isn't
>>> same what is the point in maintaining same code? It won't save in
>>> efforts mainly because its like developing a distributed FS where
>>> two parties has to be communicated to be in sync. Which is like using
>>> the canon to shoo a crow.
>>> But if the reason was fuse like kernel-free FS (no one said that
>>> though) then its better to do it as a separate project.
>>>
>>>> especially for tests.
>>>
>>> It depends whats being tested kernel OR progs? Its kernel not progs.
>>
>> No, both kernel and progs. Just from Dave, even with his typo:
>>
>> "xfstests is not jsut for testing kernel changes - it tests all of
>> the filesystem utilities for regressions, too. And so when
>> inadvertant changes in default behaviour occur, it detects those
>> regressions too."
>
>   Now in this context if you are testing latest btrfs-progs (without
>   these patches) on an old LTS kernel, and using default mkfs option
>   all tests fails. That's something to fix. Without transferring
>   implementation difficulties to the user end. And without changing
>   xfstests mkfs_options. Because we claim progs is backward kernel
>   compatible.

Why no to changing mkfs_options? We know the reason and it should be OK 
to change it.

>
>
>>> Automatic will keep default feature constant for a given kernel
>>> version. Further, for testing using a known set of options is even
>>> better.
>>
>> Yeah, known set of options get unknown on different kernels, thanks to
>> the hidden feature align. Unless you specify it by -O options.
>>
>> That's the *POINT2*:
>> Default auto feature align are making mkfs.btrfs behavior
>> *unpredictable*.
>  >
>> Before auto feature align, QA/end-user only needs to check the
>> btrfs-progs announcement to know the default behavior change.
>>
>> And after it, wow, QA testers will need to check the feature matrix to
>> know what's the default feature on their kernel, not to mention it may
>> even be wrong due to more unpredictable kernel version.
>>
>> That's why I strongly recommend to make it just a warning other than
>> default behavior.
>
>   The use cases also involves old LTS kernels on newer progs.

In this case, it's definitely a *distribution* bug.
Please report to distribution bugzilla, blaming the guys who chose the 
wrong LTS kernel and btrfs-progs combination.

And that's why default mkfs features won't change in a short time, to 
give distribution guys enough time buffer to choose good kernel/progs 
combination.

>   which your point2 does _not_ cover and where the whole point
>   of discussion is.
>
>>>
>>>> A lot of btrfs-progs change, like recent disabling mixed-bg for small
>>>> volume has already cause regression in generic/077 testcase.
>>>> And Dave is already fed up with such problem from btrfs...
>>>
>>> I don't know what's the regression about. But in my experience with
>>> some xfstest test cases.. xfstests depend too much on cli output
>>> strings which is easy thing to do but a wrong approach.
>>
>> Check this patch and its following, definitely not UI but default
>> behavior, just as you are going to change.
>>
>> https://patchwork.kernel.org/patch/7679381/
>
>   Reviewed. I understand xfstest didn't fail for an UI change.
>   So how does that apply here in this context ?
>
>   Note:
>   Before
>   'A progs and A kernel' set = default features are constant.
>   Now
>   'ANY progs and A kernel' set = default features are constant.

And 'A progs and an old but wrong version-ed kernel'.
The kernel supports for example skinny-metadata, and mkfs decides to 
disable it, causing all test cases passed.
With skinny-metadata branch not covered, bugs hidden.

Nothing broken, but worse.

Thanks,
Qu

>
>   So what is that it breaks ? I want to know as well.
>
> Thanks, Anand
>
>
>> Thanks,
>> Qu
>>
>>> Those cli outputs and its format are NOT APIs, those are UIs. Instead
>>> it should have used return code/ FS test interface. This will let
>>> developers with free hands to change, otherwise you need to update the
>>> test cases every time you change the cli _output_.
>>>
>>>> Especially such auto-detection will make default behavior more
>>>> unstable,
>>>> at least not a good idea for me.
>>>
>>> As above. We design with end-user and their use cases in mind. Not for
>>> a test suite. If test suite breaks.. fix it.
>>>
>>> Thanks, Anand
>>>
>>>> Beside this, I'm curious how other filesystm user tools handle such
>>>> kernel mismatch, or do they?
>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>
>>>>>
>>>>> First of all to let user know what features was supported at what
>>>>> kernel
>>>>> version. Patch 1/7 updates -O list-all which will list the feature
>>>>> with
>>>>> version.
>>>>>
>>>>> As we didn't maintain the sysfs and progs feature names consistent, so
>>>>> to avoid confusion Patch 2/7 displays sysfs feature name as well again
>>>>> in the list-all output.
>>>>>
>>>>> Next, Patch 3,4,5/7 are helper functions.
>>>>>
>>>>> Patch 6,7/7 provides the -O comp=<version> for mkfs.btrfs and
>>>>> btrfs-convert respectively
>>>>>
>>>>> Thanks, Anand
>>>>>
>>>>> Anand Jain (7):
>>>>>    btrfs-progs: show the version for -O list-all
>>>>>    btrfs-progs: add kernel alias for each of the features in the list
>>>>>    btrfs-progs: make is_numerical non static
>>>>>    btrfs-progs: check for numerical in version_to_code()
>>>>>    btrfs-progs: introduce framework version to features
>>>>>    btrfs-progs: add -O comp= option for mkfs.btrfs
>>>>>    btrfs-progs: add -O comp= option for btrfs-convert
>>>>>
>>>>>   btrfs-convert.c | 21 +++++++++++++++++++++
>>>>>   cmds-replace.c  | 11 -----------
>>>>>   mkfs.c          | 24 ++++++++++++++++++++++--
>>>>>   utils.c         | 58
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>>>>>   utils.h         |  2 ++
>>>>>   5 files changed, 98 insertions(+), 18 deletions(-)
>>>>>
>>>>
>>>>
>>>> --
>>>> 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
> --
> 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: [PATCH 0/7] Let user specify the kernel version for features

[Anand Jain]

Hope we are in sync on..

1.
The term auto that you are using here refs to
  'Progs default-features being updated at the _run time_'.

2.
In the long run, mostly it would be:
   progs-version > LTS-kernel-version
(for the reason that user would need fsck,tools.. etc)


>>>>> With the new -O comp= option, the concern on user who want to make a
>>>>> btrfs for newer kernel is hugely reduced.
>>>>
>>>> NO!. actually new option -O comp= provides no concern for users who
>>>> want to create _a btrfs disk layout which is compatible with more
>>>> than one kernel_.  above there are two examples of it.
>>>
>>> Why you can't give a higher kernel version than current kernel?
>>
>>   mount fails.  Pls try !!
>
> But that's what user want to do. He/she knows what he is doing.
> Maybe he is testing btrfs-progs self test without the need to mount
> it(at least some of the tests doesn't require mount)

  right. It will continue to fail even with this patch set.


> Now we need to auto align feature with kernel, who know one day we will
> need to auto align our libs to upstream package?

  align libs to upstream package ? is there any eg you could provide ?


> Keeping a matrix with different packages like libuuid/acl/attr with
> different Makefile?
> At least this is not a good idea for me, and that's the work of
> autoconfig IIRC.
>
> And if I'm a package and face such problem, I'll choose the simplest
> solution, just add a line in PKGBUILD(package system of Archlinux) of
> btrfs.
> ------
> depends=('linux>=3.14')
> ------
> (Yeah, such simple and slick packaging solution is the reason I like
> Arch over other rolling distribution)
>
> Not every thing really needed to be done in code level.

  As we are handling default features at the run time, how is
  this relevant in this context. ?

Thanks, Anand

Re: [PATCH 0/7] Let user specify the kernel version for features

[Qu Wenruo]

Anand Jain wrote on 2015/11/27 06:17 +0800:
>
> Hope we are in sync on..
>
> 1.
> The term auto that you are using here refs to
>   'Progs default-features being updated at the _run time_'.

Yes.

>
> 2.
> In the long run, mostly it would be:
>    progs-version > LTS-kernel-version
> (for the reason that user would need fsck,tools.. etc)

Also true.

But mkfs default features won't change during one or two LTS kernels.

>
>
>>>>>> With the new -O comp= option, the concern on user who want to make a
>>>>>> btrfs for newer kernel is hugely reduced.
>>>>>
>>>>> NO!. actually new option -O comp= provides no concern for users who
>>>>> want to create _a btrfs disk layout which is compatible with more
>>>>> than one kernel_.  above there are two examples of it.
>>>>
>>>> Why you can't give a higher kernel version than current kernel?
>>>
>>>   mount fails.  Pls try !!
>>
>> But that's what user want to do. He/she knows what he is doing.
>> Maybe he is testing btrfs-progs self test without the need to mount
>> it(at least some of the tests doesn't require mount)
>
>   right. It will continue to fail even with this patch set.
>
>
>> Now we need to auto align feature with kernel, who know one day we will
>> need to auto align our libs to upstream package?
>
>   align libs to upstream package ? is there any eg you could provide ?

IIRC, for the ancient time, libblkid is still included in e2fsprogs and 
its API is different from nowadays.

Will us need to support that one with different blkid calls?

>
>
>> Keeping a matrix with different packages like libuuid/acl/attr with
>> different Makefile?
>> At least this is not a good idea for me, and that's the work of
>> autoconfig IIRC.
>>
>> And if I'm a package and face such problem, I'll choose the simplest
>> solution, just add a line in PKGBUILD(package system of Archlinux) of
>> btrfs.
>> ------
>> depends=('linux>=3.14')
>> ------
>> (Yeah, such simple and slick packaging solution is the reason I like
>> Arch over other rolling distribution)
>>
>> Not every thing really needed to be done in code level.
>
>   As we are handling default features at the run time, how is
>   this relevant in this context. ?

I meant, it can be done in packaging level and it's much easier to do.
One dependency line vs near 200 codes.
And it's much predictable than version based detection.

Thanks,
Qu

>
> Thanks, Anand
>
>
>
>

Re: [PATCH 0/7] Let user specify the kernel version for features

[Anand Jain]

> I meant, it can be done in packaging level and it's much easier to do.

  Its all about trade off, and there is no right or wrong, so is tough
  to arrive at a conclusion even before this was implemented. Below are
  the choices considered, now putting in the order of least suitable
  to most suitable after reviewing their advantages and disadvantages.

  . Update default features at the compile time, so to have a define
    set for the default choices per release/distro. (not sure how to
    do that), But with this, you can not solve the problem for which
    current "-O comp=" is provided (i.e to provide features which are
    compatible across a set of known kernels). And mainly, you are
    letting the control of such a discussion out of btrfs/mainline.
    Then distros will have own set of default features instead of having
    such an effort discussed and converged at the mainline.

  . /etc/btrfs.conf file to hold the default features (same as ext4)
    I have to drop this idea since from the user point of view you are
    creating another source of config/input that user/distro has to
    care about.

  . Do it at run time for the running kernel. Current. In the order of
    priority .. check sysfs, if not check kernel-version, if not use
    progs-version-based-defaults (original).


> And it's much predictable than version based detection.

  I think you mean to say its not predictable because it follows
  a priority list, and we won't know which system used sysfs/version/
  progs-version. Concern is valid. But I feel its trivial, unless
  you have some strong reason. Further still its only a trade off that
  could achieve.

  Other concern that others commented..

  if sysfs is not there and feature is backported, for this we should
  ensure sysfs feature is also backported along with the feature it
  self, without that feature back port is incomplete. If distro does not
  want to backport sysfs, the other choice that disto has is to update
  the feature-to-version table in the progs.

  For the system which does not provide version at all, it will fail
  back to the original progs-version-based-defaults.


  Hope I have captured all the concerns and addressed them.


Thanks, Anand

Re: [PATCH 0/7] Let user specify the kernel version for features

[Qu Wenruo]

On 11/27/2015 04:41 PM, Anand Jain wrote:
>
>
>
>
>> I meant, it can be done in packaging level and it's much easier to do.
>
>   Its all about trade off, and there is no right or wrong, so is tough
>   to arrive at a conclusion even before this was implemented. Below are
>   the choices considered, now putting in the order of least suitable
>   to most suitable after reviewing their advantages and disadvantages.
>
>   . Update default features at the compile time, so to have a define
>     set for the default choices per release/distro. (not sure how to
>     do that), But with this, you can not solve the problem for which
>     current "-O comp=" is provided (i.e to provide features which are
>     compatible across a set of known kernels). And mainly, you are
>     letting the control of such a discussion out of btrfs/mainline.
>     Then distros will have own set of default features instead of having
>     such an effort discussed and converged at the mainline.

No need to set default feature per distro, we have make it clear, our 
btrfs-progs defaults feature are these, if the distro doesn't like it, 
do the backport yourself.

>
>   . /etc/btrfs.conf file to hold the default features (same as ext4)
>     I have to drop this idea since from the user point of view you are
>     creating another source of config/input that user/distro has to
>     care about.
>

No need as described above.

>   . Do it at run time for the running kernel. Current. In the order of
>     priority .. check sysfs, if not check kernel-version, if not use
>     progs-version-based-defaults (original).
>

As stated several times, kernel version based probe is inaccurate, as 
long as your goal is to ensure the fs can be mounted, you are wasting 
time as such probe won't meet your goal.

Even as a fallback method, it's not reliable and super easy to avoid.
Just remove the btrfs module.
Then all the features are just based on vanilla kernel version, which is 
far from the real kernel.


As long as your goal is to ensure a fs after mkfs can be used without 
error, you're already in a nightmare.
What if the kernel doesn't has btrfs compiled?
What if the fs is not only for kernel to mount, but also a boot 
partition for grub?
Do you need to check the grub2 version? Check if this is a /boot partition?

You want to handle them all? This is not the right place, you're in the 
field of distro, not btrfs-progs.


Recently I just encountered such problem. Latest xfs-progs makes xfs 
version 5 by default, but grub2 can't handle version 5 yet in latest 
stable version.
Then system can't even boot into grub2.

Did you see Dave trying to add such grub2 version based probe to change 
mkfs.xfs features?
No, just because that's not the way things should be done.

>
>> And it's much predictable than version based detection.
>
>   I think you mean to say its not predictable because it follows
>   a priority list, and we won't know which system used sysfs/version/
>   progs-version. Concern is valid. But I feel its trivial, unless
>   you have some strong reason. Further still its only a trade off that
>   could achieve.
>
>   Other concern that others commented..
>
>   if sysfs is not there and feature is backported, for this we should
>   ensure sysfs feature is also backported along with the feature it
>   self, without that feature back port is incomplete.

Just remove the btrfs module, then sysfs probe just become meaningless.

If something wrong may happen, then it will happen.
And the wrong thing is vanilla version based probe.

You are already messing up what btrfs-progs should do and what a distro 
should do.

Thanks,
Qu

> If distro does not
>   want to backport sysfs, the other choice that disto has is to update
>   the feature-to-version table in the progs.
>
>   For the system which does not provide version at all, it will fail
>   back to the original progs-version-based-defaults.
>
>
>   Hope I have captured all the concerns and addressed them.
>
>
> Thanks, Anand
>
>

Re: [PATCH 0/7] Let user specify the kernel version for features

[Anand Jain]

  (Most of the technical reasoning were already discussed so I won't
  repeat them here).

  And jolting for new technical reasons finds only these..

> What if the fs is not only for kernel to mount, but also a boot
> partition for grub?
> Do you need to check the grub2 version? Check if this is a /boot partition?


  In the above context, which is better..
  this :

    mkfs.btrfs -O ^skinny-metadata,^mixed-bg,^extref,^raid56,^no-holes
    btrfs-convert -O ^skinny-metadata,^extref,^no-holes

  or this:

    mkfs.btrfs -O comp=<X>
    btrfs-convert -O comp=<X>

    X = some number below 2.7.37 OR grub2 (planned) (thanks to you,
    to bring this up).

  ?

> Recently I just encountered such problem. Latest xfs-progs makes xfs
> version 5 by default, but grub2 can't handle version 5 yet in latest
> stable version.
> Then system can't even boot into grub2.

   Good example. Appears that user didn't know what latest features
   to disable? so to be compatible with grub2 ? OR they have to read
   couple of grub documents to fix.

   Imagine the pain while using btrfs-convert. That means you need to
   restore Hope you have not deleted the ext_saved subvol. And then
   again run btrfs-convert.


> Did you see Dave trying to add such grub2 version based probe to change
> mkfs.xfs features?
> No, just because that's not the way things should be done.

   So what was fix in this case ? Or is there any fix-patch rejected ?

Cheers, Anand

Re: [PATCH 0/7] Let user specify the kernel version for features

[Qu Wenruo]

Anand Jain wrote on 2015/11/30 12:54 +0800:
>
>
>   (Most of the technical reasoning were already discussed so I won't
>   repeat them here).
>
>   And jolting for new technical reasons finds only these..
>
>> What if the fs is not only for kernel to mount, but also a boot
>> partition for grub?
>> Do you need to check the grub2 version? Check if this is a /boot
>> partition?
>
>
>   In the above context, which is better..
>   this :
>
>     mkfs.btrfs -O ^skinny-metadata,^mixed-bg,^extref,^raid56,^no-holes
>     btrfs-convert -O ^skinny-metadata,^extref,^no-holes
>
>   or this:
>
>     mkfs.btrfs -O comp=<X>
>     btrfs-convert -O comp=<X>
>
>     X = some number below 2.7.37 OR grub2 (planned) (thanks to you,
>     to bring this up).
>
>   ?

Yeah, 2.6.37. What a idiot to use Btrfs on that old kernel?
For stability?

>
>> Recently I just encountered such problem. Latest xfs-progs makes xfs
>> version 5 by default, but grub2 can't handle version 5 yet in latest
>> stable version.
>> Then system can't even boot into grub2.
>
>    Good example. Appears that user didn't know what latest features
>    to disable? so to be compatible with grub2 ? OR they have to read
>    couple of grub documents to fix.

Oh, so we should add btrfs-progs grub2 feature auto align?
And that's what documents is for, I just opened Arch wiki page of xfs, 
and found the solution.

If you don't ever like to read the document, why read the codes and 
enhance it?

>
>    Imagine the pain while using btrfs-convert. That means you need to
>    restore Hope you have not deleted the ext_saved subvol. And then
>    again run btrfs-convert.

Wow, how do you delete the ext_saved subvolume without mounting it?

>
>
>> Did you see Dave trying to add such grub2 version based probe to change
>> mkfs.xfs features?
>> No, just because that's not the way things should be done.
>
>    So what was fix in this case ? Or is there any fix-patch rejected ?

No fix in xfsprogs.
Only support in grub2 is added and already in grub2 git repo.
No stable released at that time.

For xfsprogs, just 4 lines in its announcement:
"There is a notable change in
defaults for mkfs.xfs - it will now make CRC enabled filesystems by
default, and you will need to use "mkfs.xfs -m crc=0" to make
filesystems compatible with production kernels older than 3.15."


Eager to see you to add such feature align patch for xfsprogs and how 
Dave will treat it.



To be honest, how many guys really unhappy with current default features 
behavior *except* you?
Or how many bug report in maillist/bugzilla is about that?

Introducing a new feature matrix only to change the behavior to your flavor?
Isn't this what you called "bloated intelligence"?

Ext2/3/4 and xfs progs are already using such behavior for years and I 
didn't ever see they need anything like btrfs sysfs features interface.
No to mention ext4 and xfs is already used in production environment.

If you are so unhappy with current behavior, why not start from xfs and 
I think Dave will be very "happy" with it.

Thanks,
Qu
>
> Cheers, Anand

Re: [PATCH 0/7] Let user specify the kernel version for features

[David Sterba]

Going back to this discussion,

On Mon, Nov 30, 2015 at 01:46:15PM +0800, Qu Wenruo wrote:
> To be honest, how many guys really unhappy with current default features 
> behavior *except* you?

Me too.

Anand's summary matches my view of how we should do it:

". Do it at run time for the running kernel. Current. In the order of
   priority .. check sysfs, if not check kernel-version, if not use
   progs-version-based-defaults (original)."

I understand that the the kernel version is not always reliable, but
when we use it only as a fallback we know that at least kernel of this
upstream version can mount the filesystem.

And becase we can turn on additional features later this should not be a
blocker.

I disagree with your view that the weight should be put on the distro
integrators. I've been packaging btrfs-progs for the SLES and openSUSE,
several product lines with different kernels (upstream and backports).
Keeping the features in sync with kernel started to be an issue with the
first incompat feature that was different among the kernels. And we want
change the defaults further.

The main user bases that I take into account:

* upstream community, ie. the latest stable version or the git version
* older stable trees, ie. 3.2, 3.14 up to 4.1
* heavy backports to enterprise kernels, arbitrary base version

My idea of the default behaviour is to be able to use latest btrfs-progs
(released or git) on any of them and minimize possible damage. Ie.
respect the running kernel and try to best detect the features.

Besides ancient kernels and the 3.2 that's still in use, the sysfs based
detection should work.

> Or how many bug report in maillist/bugzilla is about that?
> 
> Introducing a new feature matrix only to change the behavior to your flavor?
> Isn't this what you called "bloated intelligence"?

I'm not sure why you see the feature matrix getting more complicated.
The testeres and developers know what features to turn on if desired,
testing recent kernel with recent progs will do the right thing on
itself.  The rest of the user are supposed to just install and use it.

Re: [PATCH 0/7] Let user specify the kernel version for features

[Qu Wenruo]

David Sterba wrote on 2016/02/03 11:50 +0100:
> Going back to this discussion,
>
> On Mon, Nov 30, 2015 at 01:46:15PM +0800, Qu Wenruo wrote:
>> To be honest, how many guys really unhappy with current default features
>> behavior *except* you?
>
> Me too.
>
> Anand's summary matches my view of how we should do it:
>
> ". Do it at run time for the running kernel. Current. In the order of
>     priority .. check sysfs, if not check kernel-version, if not use
>     progs-version-based-defaults (original)."

I'm curious under which case btrfs will fallback to 
progs-version-based-default, as I though kernel-version probe will make 
it never go to progs-version-based one. (Maybe non-numeric kernel version?)

>
> I understand that the the kernel version is not always reliable, but
> when we use it only as a fallback we know that at least kernel of this
> upstream version can mount the filesystem.

Yes, upstream version can mount it, but it does not always make sense.

For example, the kernel used by user may doesn't contain btrfs 
modules/built-in.
We shouldn't do extra check even trying to ensure something we can't ensure.

>
> And becase we can turn on additional features later this should not be a
> blocker.
>
> I disagree with your view that the weight should be put on the distro
> integrators. I've been packaging btrfs-progs for the SLES and openSUSE,
> several product lines with different kernels (upstream and backports).
> Keeping the features in sync with kernel started to be an issue with the
> first incompat feature that was different among the kernels. And we want
> change the defaults further.
>
> The main user bases that I take into account:
>
> * upstream community, ie. the latest stable version or the git version
> * older stable trees, ie. 3.2, 3.14 up to 4.1
> * heavy backports to enterprise kernels, arbitrary base version
>
> My idea of the default behaviour is to be able to use latest btrfs-progs
> (released or git) on any of them and minimize possible damage. Ie.
> respect the running kernel and try to best detect the features.

OK, but I didn't see anything sysfs-based probe can't handle.
(Except kernel doesn't support btrfs sysfs interface)

Personally, I just prefer to deprecate any kernel doesn't support btrfs 
features sysfs interface, and give a strong enough warning.

So my idea about the workflow would be:
1) Check sysfs feature interface and use it.
2) Fallback to progs-default-feature unless '-f' is given.

Much more straightforward and makes sense for me.
If kernel is too old without the sysfs intreface backported (or without 
btrfs modules/built-in), it's a very bad idea to use btrfs.

>
> Besides ancient kernels and the 3.2 that's still in use, the sysfs based
> detection should work.

Ancient kernels (including 3.2 though) without heavy backports are never 
a good idea to run btrfs on.
So I just like to deprecate them in btrfs-progs.

>
>> Or how many bug report in maillist/bugzilla is about that?
>>
>> Introducing a new feature matrix only to change the behavior to your flavor?
>> Isn't this what you called "bloated intelligence"?
>
> I'm not sure why you see the feature matrix getting more complicated.
> The testeres and developers know what features to turn on if desired,
> testing recent kernel with recent progs will do the right thing on
> itself.  The rest of the user are supposed to just install and use it.
>
>
Your point also shows that, -O ver=X.XX is not needed.
Testers/developers know which exactly feature they need to enable/disable.

And end user just use it, sysfs-based probe handles them quite well.

Ancient kernel users also get warned, so I see no need to use feature 
matrix.

Thanks,
Qu

Re: [PATCH 0/7] Let user specify the kernel version for features

[Chris Mason]

On Wed, Feb 03, 2016 at 11:50:38AM +0100, David Sterba wrote:
> Going back to this discussion,
> 
> On Mon, Nov 30, 2015 at 01:46:15PM +0800, Qu Wenruo wrote:
> > To be honest, how many guys really unhappy with current default features 
> > behavior *except* you?
> 
> Me too.
> 
> Anand's summary matches my view of how we should do it:
> 
> ". Do it at run time for the running kernel. Current. In the order of
>    priority .. check sysfs, if not check kernel-version, if not use
>    progs-version-based-defaults (original)."
> 
> I understand that the the kernel version is not always reliable, but
> when we use it only as a fallback we know that at least kernel of this
> upstream version can mount the filesystem.
> 
> And becase we can turn on additional features later this should not be a
> blocker.
> 
> I disagree with your view that the weight should be put on the distro
> integrators. I've been packaging btrfs-progs for the SLES and openSUSE,
> several product lines with different kernels (upstream and backports).
> Keeping the features in sync with kernel started to be an issue with the
> first incompat feature that was different among the kernels. And we want
> change the defaults further.
> 
> The main user bases that I take into account:
> 
> * upstream community, ie. the latest stable version or the git version
> * older stable trees, ie. 3.2, 3.14 up to 4.1
> * heavy backports to enterprise kernels, arbitrary base version
> 
> My idea of the default behaviour is to be able to use latest btrfs-progs
> (released or git) on any of them and minimize possible damage. Ie.
> respect the running kernel and try to best detect the features.
> 
> Besides ancient kernels and the 3.2 that's still in use, the sysfs based
> detection should work.

I do agree with Dave here, and usually defer to the people making
distros on these kinds of things.  I want to make sure it is very
easy for distros to upgrade to the latest progs without worrying about
configuring the exact set of features their kernels currently support.
Yes, they can spent extra time patching progs, but we want to limit the
amount of work each progs update forces them to do.

But we also need to make sure it's obvious to the user which features
have been enabled, and how btrfs-progs went about making that decision.
It should also be easy for people packaging btrfs to override those
decisions based on their distro kernel.

-chris

potential btrfs-progs clean up

[Anand Jain]

(this is a different topic, updated the subject)

> Totally agree with this point. Too many non-sense in btrfs-progs codes
> copied from kernel, and due to lack of update, it's very buggy now.
> Just check volume.c for allocating data chunk.

   Which functions in volume.c in particular ?

Thanks, Anand

Re: potential btrfs-progs clean up

[Qu Wenruo]

Anand Jain wrote on 2015/11/30 13:44 +0800:
>
>
> (this is a different topic, updated the subject)
>
>> Totally agree with this point. Too many non-sense in btrfs-progs codes
>> copied from kernel, and due to lack of update, it's very buggy now.
>> Just check volume.c for allocating data chunk.
>
>    Which functions in volume.c in particular ?

btrfs_alloc_chunk().

1) Fixed SINGLE DATA chunk size
The point is very easy, for SINGLE and DATA chunk, its size is always 
fixed to 8M. (below minimal data chunk size already)
Normally it's OK, but not the normal 10% of filesystem.

The bug is hidden by the complicated PROFILE if sentences.

2) Not that clear chunk size limitation.
And the logical of btrfs_alloc_chunk() is not that clear to for the 
minimal *stripe* size(which is physical) and minimal *chunk* size(which 
is logical).

Not a real problem yet.

3) Duplicated chunk allocation functions
Btrfs_alloc_chunk(), btrfs_alloc_data_chunk().

But don't cleanup the latter one yet, as btrfs-convert rework still 
needs it to avoid some chunk allocation check.

Thanks,
Qu


>
> Thanks, Anand
>
>