On Tue, 10 May 2011, Lukas Czerner wrote:
> On Tue, 10 May 2011, Aditya Kali wrote:
>
> > This patch adds profile_get_double function in profile.c that allows reading
> > floating point values from profile files. The floating point support is used
> > to read reserved_ratio (reserved for super user block percentage) from
> > mke2fs.conf.
>
> Hi Aditya,
>
> thanks for the patch, however I have couple of comments. First of all
> the commit subject does not really reflect the change, because the point
> of this commit is not to teach e2fsprogs to be able to read double from
> profile but rather add new option to mke2fs.conf for specifying
> super-user reservation ratio. I am sorry it this seems like nitpicking,
> but it is really useful to be able to at least guess what the commit
> does from its subject.
>
> Also it would be nice to have this option described in the mke2fs
> manual page as well.
Sorry, I meant mke2fs.conf man page. Other than that the patch looks
good and works as expected.
>
> Thanks!
> -Lukas
>
> >
> > Signed-off-by: Aditya Kali <adityakali@xxxxxxxxxx>
> > ---
> > e2fsck/profile.c | 37 +++++++++++++++++++++++++++++++++++++
> > e2fsck/profile.h | 5 +++++
> > misc/mke2fs.c | 26 +++++++++++++++++++++++++-
> > tests/mke2fs.conf.in | 1 +
> > 4 files changed, 68 insertions(+), 1 deletions(-)
> >
> > diff --git a/e2fsck/profile.c b/e2fsck/profile.c
> > index 5e9dc53..327bfb4 100644
> > --- a/e2fsck/profile.c
> > +++ b/e2fsck/profile.c
> > @@ -1596,6 +1596,43 @@ profile_get_uint(profile_t profile, const char *name, const char *subname,
> > return 0;
> > }
> >
> > +errcode_t
> > +profile_get_double(profile_t profile, const char *name, const char *subname,
> > + const char *subsubname, double def_val, double *ret_double)
> > +{
> > + const char *value;
> > + errcode_t retval;
> > + char *end_value;
> > + double double_val;
> > +
> > + *ret_double = def_val;
> > + if (profile == 0)
> > + return 0;
> > +
> > + retval = profile_get_value(profile, name, subname, subsubname, &value);
> > + if (retval == PROF_NO_SECTION || retval == PROF_NO_RELATION) {
> > + *ret_double = def_val;
> > + return 0;
> > + } else if (retval)
> > + return retval;
> > +
> > + if (value[0] == 0)
> > + /* Empty string is no good. */
> > + return PROF_BAD_INTEGER;
> > + errno = 0;
> > + double_val = strtod(value, &end_value);
> > +
> > + /* Overflow or underflow. */
> > + if (errno != 0)
> > + return PROF_BAD_INTEGER;
> > + /* Garbage in string. */
> > + if (end_value != value + strlen(value))
> > + return PROF_BAD_INTEGER;
> > +
> > + *ret_double = double_val;
> > + return 0;
> > +}
> > +
> > static const char *const conf_yes[] = {
> > "y", "yes", "true", "t", "1", "on",
> > 0,
> > diff --git a/e2fsck/profile.h b/e2fsck/profile.h
> > index 0c17732..4cc10eb 100644
> > --- a/e2fsck/profile.h
> > +++ b/e2fsck/profile.h
> > @@ -78,6 +78,11 @@ long profile_get_uint
> > const char *subsubname, unsigned int def_val,
> > unsigned int *ret_int);
> >
> > +long profile_get_double
> > + (profile_t profile, const char *name, const char *subname,
> > + const char *subsubname, double def_val,
> > + double *ret_float);
> > +
> > long profile_get_boolean
> > (profile_t profile, const char *name, const char *subname,
> > const char *subsubname, int def_val,
> > diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> > index 23f8c10..313423b 100644
> > --- a/misc/mke2fs.c
> > +++ b/misc/mke2fs.c
> > @@ -1072,6 +1072,18 @@ static int get_int_from_profile(char **fs_types, const char *opt, int def_val)
> > return ret;
> > }
> >
> > +static double get_double_from_profile(char **fs_types, const char *opt,
> > + double def_val)
> > +{
> > + double ret;
> > + char **cpp;
> > +
> > + profile_get_double(profile, "defaults", opt, 0, def_val, &ret);
> > + for (cpp = fs_types; *cpp; cpp++)
> > + profile_get_double(profile, "fs_types", *cpp, opt, ret, &ret);
> > + return ret;
> > +}
> > +
> > static int get_bool_from_profile(char **fs_types, const char *opt, int def_val)
> > {
> > int ret;
> > @@ -1144,7 +1156,7 @@ static void PRS(int argc, char *argv[])
> > int inode_ratio = 0;
> > int inode_size = 0;
> > unsigned long flex_bg_size = 0;
> > - double reserved_ratio = 5.0;
> > + double reserved_ratio = -1.0;
> > int lsector_size = 0, psector_size = 0;
> > int show_version_only = 0;
> > unsigned long long num_inodes = 0; /* unsigned long long to catch too-large input */
> > @@ -1667,6 +1679,18 @@ profile_error:
> > EXT3_FEATURE_COMPAT_HAS_JOURNAL;
> > }
> >
> > + /* Get reserved_ratio from profile if not specified on cmd line. */
> > + if (reserved_ratio < 0.0) {
> > + reserved_ratio = get_double_from_profile(
> > + fs_types, "reserved_ratio", 5.0);
> > + if (reserved_ratio > 50 || reserved_ratio < 0) {
> > + com_err(program_name, 0,
> > + _("invalid reserved blocks percent - %lf"),
> > + reserved_ratio);
> > + exit(1);
> > + }
> > + }
> > +
> > if (fs_param.s_feature_incompat &
> > EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) {
> > reserved_ratio = 0;
> > diff --git a/tests/mke2fs.conf.in b/tests/mke2fs.conf.in
> > index 070d5d5..fbe2e2a 100644
> > --- a/tests/mke2fs.conf.in
> > +++ b/tests/mke2fs.conf.in
> > @@ -3,6 +3,7 @@
> > blocksize = 4096
> > inode_size = 256
> > inode_ratio = 16384
> > + reserved_ratio = 5.0
> > enable_periodic_fsck = true
> > lazy_itable_init = false
> > default_mntopts = ^acl
> >
>
>
--
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
