| Message ID | 1445417149-804-1-git-send-email-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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; > -- 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
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 -- 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
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 > -- 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
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 -- 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
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;
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(-)