The following patch is a work in progress, but I'm sending it out so
folks can take a look at it and comment on the general approach.
What this does is change how mke2fs -T works so it can take a comma
separated list, so you can do things like this:
mke2fs -T ext4,small,news
(which probably doesn't make any sense :-)
This makes it easier for us to specify what the behaviour of mke2fs is
via mke2fs.conf, and mke2fs will default the size and filesystem types
based on contextual hints (i.e., if the user calls mke2fs via
/sbin/mkfs.ext4, or if the -j flag is requested to request a journal,
etc.).
It's pretty flexible in allowing distirbutions and system administrtors
to control the behaviour of mke2fs via the config file, so that we
aren't hardcoding policy.
There are still a lot of debugging printfs in the code, which may be
helpful in seeing how this works. BTW, This patch is based on some
recently committed changes which allows mke2fs to accept "mke2fs -O
extents", so if you want to actually compile and play with it you'll
probably want to pull the latest master or next branch from git and then
apply this patch against that.
- Ted
commit 09138f9e096af809f1ff02df9194c9e0dad186db
Author: Theodore Ts'o <[email protected]>
Date: Tue Feb 19 08:32:58 2008 -0500
New mke2fs types parsing --- IN PROGRESS
diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index 9d14bdd..a906797 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -926,6 +926,7 @@ static void edit_feature(const char *str, __u32 *compat_array)
if (!str)
return;
+ printf("Editing feature: '%s'\n", str);
if (e2p_edit_feature(str, compat_array, ok_features)) {
fprintf(stderr, _("Invalid filesystem option set: %s\n"),
str);
@@ -933,6 +934,201 @@ static void edit_feature(const char *str, __u32 *compat_array)
}
}
+struct str_list {
+ char **list;
+ int num;
+ int max;
+};
+
+static errcode_t init_list(struct str_list *sl)
+{
+ sl->num = 0;
+ sl->max = 0;
+ sl->list = malloc((sl->max+1) * sizeof(char *));
+ if (!sl->list)
+ return ENOMEM;
+ sl->list[0] = 0;
+ return 0;
+}
+
+static errcode_t push_string(struct str_list *sl, const char *str)
+{
+ char **new_list;
+
+ if (sl->num >= sl->max) {
+ sl->max += 2;
+ new_list = realloc(sl->list, (sl->max+1) * sizeof(char *));
+ if (!new_list)
+ return ENOMEM;
+ sl->list = new_list;
+ }
+ sl->list[sl->num] = malloc(strlen(str)+1);
+ if (sl->list[sl->num] == 0)
+ return ENOMEM;
+ strcpy(sl->list[sl->num], str);
+ sl->num++;
+ sl->list[sl->num] = 0;
+ return 0;
+}
+
+static void print_str_list(char **list)
+{
+ char **cpp;
+
+ for (cpp = list; *cpp; cpp++) {
+ printf("'%s'", *cpp);
+ if (cpp[1])
+ fputs(", ", stdout);
+ }
+ fputc('\n', stdout);
+}
+
+static char **parse_fs_type(const char *fs_type,
+ struct ext2_super_block *fs_param,
+ char *progname)
+{
+ char *ext_type = 0;
+ char *parse_str;
+ char *cp, *t;
+ const char *size_type;
+ struct str_list list;
+ int state = 0;
+ unsigned long meg;
+
+ if (init_list(&list))
+ return 0;
+
+ if (progname) {
+ ext_type = strrchr(progname, '/');
+ if (ext_type)
+ ext_type++;
+ else
+ ext_type = progname;
+
+ if (!strncmp(ext_type, "mkfs.", 5)) {
+ ext_type += 5;
+ if (ext_type[0] == 0)
+ ext_type = 0;
+ } else
+ ext_type = 0;
+ }
+
+ if (!ext_type && (journal_size != 0))
+ ext_type = "ext3";
+
+ /* Make a copy so we can free it safely later */
+ if (ext_type) {
+ t = ext_type;
+ ext_type = malloc(strlen(t)+1);
+ if (ext_type)
+ strcpy(ext_type, t);
+ }
+
+ if (!ext_type)
+ profile_get_string(profile, "defaults", "fs_type", 0,
+ "ext2", &ext_type);
+
+ if (ext_type)
+ printf("Using ext_type: '%s'\n", ext_type);
+
+ meg = (1024 * 1024) / EXT2_BLOCK_SIZE(fs_param);
+ printf("Size is %lu (%lu).\n", fs_param->s_blocks_count, meg);
+ if (fs_param->s_blocks_count < 3 * meg)
+ size_type = "floppy";
+ else if (fs_param->s_blocks_count < 512 * meg)
+ size_type = "small";
+ else
+ size_type = "default";
+ printf("Size type is %s.\n", size_type);
+
+ parse_str = malloc(fs_type ? strlen(fs_type)+1 : 1);
+ if (!parse_str) {
+ free(list.list);
+ return 0;
+ }
+ if (fs_type)
+ strcpy(parse_str, fs_type);
+ else
+ *parse_str = '\0';
+
+ cp = parse_str;
+ while (1) {
+ t = strchr(cp, ',');
+ if (t)
+ *t = '\0';
+ again:
+ state++;
+ if (state == 1) {
+ if (strcmp(cp, "ext2") &&
+ strcmp(cp, "ext3") &&
+ strcmp(cp, "ext4") &&
+ strcmp(cp, "ext4dev")) {
+ printf("No filesystem type, adding %s\n",
+ ext_type);
+ push_string(&list, ext_type);
+ goto again;
+ }
+ }
+ if (state == 2) {
+ if (strcmp(cp, "floppy") &&
+ strcmp(cp, "small") &&
+ strcmp(cp, "default")) {
+ printf("No size type, adding %s\n",
+ size_type);
+ push_string(&list, size_type);
+ goto again;
+ }
+ }
+ if (*cp) {
+ printf("fs_type: '%s'\n", cp);
+ push_string(&list, cp);
+ }
+ if (t)
+ cp = t+1;
+ else {
+ cp = "";
+ if (state < 2)
+ goto again;
+ break;
+ }
+ }
+ free(parse_str);
+ if (ext_type)
+ free(ext_type);
+ return (list.list);
+}
+
+static char *get_string_from_profile(char **fs_types, const char *opt,
+ const char *def_val)
+{
+ char *ret = 0;
+ char **cpp;
+
+ profile_get_string(profile, "defaults", opt, 0, 0, &ret);
+ if (ret)
+ return ret;
+
+ for (cpp = fs_types; *cpp; cpp++) {
+ profile_get_string(profile, "fs_types", *cpp, 0,
+ cpp[1] ? 0 : def_val, &ret);
+ if (ret)
+ return ret;
+ }
+ return (ret);
+}
+
+static int get_int_from_profile(char **fs_types, const char *opt, int def_val)
+{
+ int ret;
+ char **cpp;
+
+ profile_get_integer(profile, "defaults", opt, 0, def_val, &ret);
+ for (cpp = fs_types; *cpp; cpp++)
+ profile_get_integer(profile, "fs_types", *cpp, opt, ret, &ret);
+ return ret;
+}
+
+
extern const char *mke2fs_default_profile;
static const char *default_files[] = { "<default>", 0 };
@@ -952,6 +1148,7 @@ static void PRS(int argc, char *argv[])
char * oldpath = getenv("PATH");
char * extended_opts = 0;
const char * fs_type = 0;
+ char **fs_types;
blk_t dev_size;
#ifdef __linux__
struct utsname ut;
@@ -1317,6 +1514,14 @@ static void PRS(int argc, char *argv[])
proceed_question();
}
+ fs_types = parse_fs_type(fs_type, &fs_param, argv[0]);
+ if (!fs_types) {
+ fprintf(stderr, _("Failed to parse fs types list\n"));
+ exit(1);
+ }
+ printf("fs_types: ");
+ print_str_list(fs_types);
+
if (!fs_type) {
int megs = (__u64)fs_param.s_blocks_count *
(EXT2_BLOCK_SIZE(&fs_param) / 1024) / 1024;
@@ -1334,29 +1539,33 @@ static void PRS(int argc, char *argv[])
/* Figure out what features should be enabled */
- tmp = tmp2 = NULL;
+ tmp = NULL;
if (fs_param.s_rev_level != EXT2_GOOD_OLD_REV) {
- profile_get_string(profile, "defaults", "base_features", 0,
- "sparse_super,filetype,resize_inode,dir_index",
- &tmp);
- profile_get_string(profile, "fs_types", fs_type,
- "base_features", tmp, &tmp2);
- edit_feature(tmp2, &fs_param.s_feature_compat);
+ char **cpp;
+
+ tmp = get_string_from_profile(fs_types, "base_features",
+ "sparse_super,filetype,resize_inode,dir_index");
+ printf("base features: %s\n", tmp);
+ edit_feature(tmp, &fs_param.s_feature_compat);
free(tmp);
- free(tmp2);
- tmp = tmp2 = NULL;
- profile_get_string(profile, "defaults", "default_features", 0,
- "", &tmp);
- profile_get_string(profile, "fs_types", fs_type,
- "default_features", tmp, &tmp2);
+ for (cpp = fs_types; *cpp; cpp++) {
+ tmp = NULL;
+ profile_get_string(profile, "fs_types", *cpp,
+ "features", "", &tmp);
+ if (tmp && *tmp)
+ edit_feature(tmp, &fs_param.s_feature_compat);
+ if (tmp)
+ free(tmp);
+ }
+ tmp = get_string_from_profile(fs_types, "default_features",
+ "");
+ printf("default features: %s\n", tmp);
}
- edit_feature(fs_features ? fs_features : tmp2,
+ edit_feature(fs_features ? fs_features : tmp,
&fs_param.s_feature_compat);
if (tmp)
free(tmp);
- if (tmp2)
- free(tmp2);
if (r_opt == EXT2_GOOD_OLD_REV &&
(fs_param.s_feature_compat || fs_param.s_feature_incompat ||
@@ -1414,10 +1623,8 @@ static void PRS(int argc, char *argv[])
sector_size = atoi(tmp);
if (blocksize <= 0) {
- profile_get_integer(profile, "defaults", "blocksize", 0,
- 4096, &use_bsize);
- profile_get_integer(profile, "fs_types", fs_type,
- "blocksize", use_bsize, &use_bsize);
+ use_bsize = get_int_from_profile(fs_types, "blocksize", 4096);
+ printf("profile blocksize: %d\n", use_bsize);
if (use_bsize == -1) {
use_bsize = sys_page_size;
@@ -1434,12 +1641,9 @@ static void PRS(int argc, char *argv[])
}
if (inode_ratio == 0) {
- profile_get_integer(profile, "defaults", "inode_ratio", 0,
- 8192, &inode_ratio);
- profile_get_integer(profile, "fs_types", fs_type,
- "inode_ratio", inode_ratio,
- &inode_ratio);
-
+ inode_ratio = get_int_from_profile(fs_types, "inode_ratio",
+ 8192);
+ printf("profile inode_ratio: %d\n", inode_ratio);
if (inode_ratio < blocksize)
inode_ratio = blocksize;
}
@@ -1486,6 +1690,9 @@ static void PRS(int argc, char *argv[])
"Use -b 4096 if this is an issue for you.\n\n"));
if (inode_size == 0) {
+ inode_size = get_int_from_profile(fs_types, "inode_size", 0);
+ printf("profile inode_size: %d\n", inode_size);
+
profile_get_integer(profile, "defaults", "inode_size", NULL,
0, &inode_size);
profile_get_integer(profile, "fs_types", fs_type,
@@ -1802,5 +2009,6 @@ no_journal:
val = ext2fs_close(fs);
remove_error_table(&et_ext2_error_table);
remove_error_table(&et_prof_error_table);
+ profile_release(profile);
return (retval || val) ? 1 : 0;
}
diff --git a/misc/mke2fs.conf b/misc/mke2fs.conf
index d67593a..a00c4ed 100644
--- a/misc/mke2fs.conf
+++ b/misc/mke2fs.conf
@@ -5,6 +5,13 @@
inode_ratio = 16384
[fs_types]
+ ext3 = {
+ features = has_journal
+ }
+ ext4 = {
+ features = extents,flex_bg
+ inode_size = 256
+ }
small = {
blocksize = 1024
inode_size = 128
Theodore Ts'o wrote:
> The following patch is a work in progress, but I'm sending it out so
> folks can take a look at it and comment on the general approach.
>
> What this does is change how mke2fs -T works so it can take a comma
> separated list, so you can do things like this:
>
> mke2fs -T ext4,small,news
Is there some hierarchy of what these options are and how they fit
together? i.e. small & news might go together (in bizarro world...) but
"small,large" wouldn't make sense - nor would -T ext3,ext4. Or, if
somebody stores mail & news on the same fs, nad they say -T mail,news
but the mail & news types have conflicting directives...
how will you define what an acceptable composite of subtypes will be?
Thanks,
-Eric
On Wed, Feb 20, 2008 at 12:51:21PM -0600, Eric Sandeen wrote:
> Theodore Ts'o wrote:
> > The following patch is a work in progress, but I'm sending it out so
> > folks can take a look at it and comment on the general approach.
> >
> > What this does is change how mke2fs -T works so it can take a comma
> > separated list, so you can do things like this:
> >
> > mke2fs -T ext4,small,news
>
> Is there some hierarchy of what these options are and how they fit
> together? i.e. small & news might go together (in bizarro world...) but
> "small,large" wouldn't make sense - nor would -T ext3,ext4. Or, if
> somebody stores mail & news on the same fs, nad they say -T mail,news
> but the mail & news types have conflicting directives...
>
> how will you define what an acceptable composite of subtypes will be?
There are only three things which mke2fs will do, in my design:
#1) If the first type is not one of "ext2", "ext3", "ext4", or
"ext4dev", mke2fs will determine a suitable default based on either
argv[0] if it is of the form mke2fs.*, or via defaults.fs_type in
/etc/mke2fs.conf. If neither of these is available it will default to "ext2".
#2) If the second type is not one of "small", "floppy" or "default",
mke2fs will create a default type by checking the size of the
to-be-created filesystem. If it is less than 3mb, it is "floppy", if
it is less than 512mb, it is "small", otherwise it is default.
#3) Once it has the list of types, i.e., "ext3,defaults,squid", or
"ext2,floppy,rescue", "ext4,defaults,largefile", etc. it uses this as
a search path when mke2fs needs to look up some parameter, such as
"base_features", or "inode_size", etc.
So suppose we are looking up the inode_size and the fs_types list is
"ext3,defaults,squid". The possible places where the inode_size
parameter cna be found are: defaults.inode_size,
fs_types.ext3.inode_size, fs_types.defaults.inode_size,
fs_types.squid.inode_size. Mke2fs will look in the most specific
place (fs_types.squid.inode_size), and then successively more general,
all the way up to defaults.inode_size for the inode_size parameter in
/etc/mke2fs.conf.
(Oh, looks like I got things backwards for the string lookups; oops,
I'll fix that.)
So the basic idea is that it's as free form as possible, and it's all
based on defaults getting overridden in the config file. So if the
mke2fs.conf file has something like this:
[defaults]
base_features = sparse_super,filetype,resize_inode,dir_index,ext_attr
blocksize = 4096
inode_size = 256
inode_ratio = 16384
[fs_types]
ext3 = {
features = has_journal
}
ext4 = {
features = extents,flex_bg
inode_size = 256
}
small = {
blocksize = 1024
inode_size = 128
inode_ratio = 4096
}
news = {
inode_ratio = 4096
}
And the user runs the command: /sbin/mkfs.ext4 -T news /dev/hda5
(and let's assume for the same of argument that /dev/hda5 is 400
megabytes)
Then mke2fs will expand the fs_types field to be "ext4,small,news".
The user could have specified this explicitly as an argument to the -T
option, but 99% of the time, they won't.
So that means that when we look for the inode_ratio parameter, it will
come form fs_types.news.inode_ratio, and mke2fs will use the value
4096. For the inode_size, the most specific version will be
fs_types.small.inode_size, or 128.
In terms of handling features, things are a bit more complicated. The
design is that we use base_features (first looked in [defaults],
[fs_types].ext4, et. al) to determine the base set of features to
initialize feature set flags. Next, we look for fs_types.ext4.features,
fs_types.small.features, fs_types.news.features, and if any of them
exist, they are used to modify the feature set. Just as with tune2fs,
the ^ character can be used to turn off a feature. Finally, the argument
to -O is used to further modify the feature set.
This are a little bit complicated because I wanted to preserve
backwards compatibility with the existing mke2fs.conf semantics, while
still making it possible to incrementally override portions of the
mke2fs configuration parameters in the simplest but yet most powerful
way possible.
In practice, I suspect most poeple will just say:
mke2fs -T ext4 /dev/hda5
or
mkfs.ext4 /dev/hda5
or
mkfs.ext4 -T news /dev/hda5
Some people might also do:
mke2fs -T ext4,news /dev/hda5
I doubt many people will do something as complicated as:
mke2fs -T ext4,largefiles,squid,extra-resize-space /dev/hda5
and have a complicated mke2fs.conf file site on their system --- but
if they really want to do that, they can.
I could also imageine people using this to make it easier to create
different kinds of filesystems for creating test cases or benchmarks, so
/etc/mke2fs.conf could contain things like
[fs_types]
benchmark_tpc_h = {
raid_stride = 31
raid_stripe_width = 65536
blocksize = 65536
}
benchmark_specweb = {
inode_size = 256
inode_ratio = 16384
blocksize = 4096
}
(Yes, this assumes adding more parameters that can be configured via
mke2fs.conf getting added into mke2fs, but that's part of the point of
this system. We shouldn't be forcing people into needing to type very
long mke2fs command lines if there are ways we can make things more
convenient for them.)
- Ted
Theodore Tso wrote:
> On Wed, Feb 20, 2008 at 12:51:21PM -0600, Eric Sandeen wrote:
>> Theodore Ts'o wrote:
>>> The following patch is a work in progress, but I'm sending it out so
>>> folks can take a look at it and comment on the general approach.
>>>
>>> What this does is change how mke2fs -T works so it can take a comma
>>> separated list, so you can do things like this:
>>>
>>> mke2fs -T ext4,small,news
>> Is there some hierarchy of what these options are and how they fit
>> together? i.e. small & news might go together (in bizarro world...) but
>> "small,large" wouldn't make sense - nor would -T ext3,ext4. Or, if
>> somebody stores mail & news on the same fs, nad they say -T mail,news
>> but the mail & news types have conflicting directives...
>>
>> how will you define what an acceptable composite of subtypes will be?
>
> There are only three things which mke2fs will do, in my design:
I apologize if I was too quick to ask a dumb question rather than
atually read through the patch, but thank you for the text explanation :)
-Eric
On Feb 20, 2008 17:20 -0500, Theodore Ts'o wrote:
> There are only three things which mke2fs will do, in my design:
This should all go into the mke2fs man page...
> [fs_types]
> ext3 = {
> features = has_journal
> }
> ext4 = {
> features = extents,flex_bg
> inode_size = 256
> }
Presumably the ext4 feature should also have features = has_journal?
If this is the default for ext4, why would it need to be given for ext3?
We should also add "dir_nlink,flexbg" while we are in there.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
On Thu, Feb 21, 2008 at 01:52:21AM -0700, Andreas Dilger wrote:
> On Feb 20, 2008 17:20 -0500, Theodore Ts'o wrote:
> > There are only three things which mke2fs will do, in my design:
>
> This should all go into the mke2fs man page...
It will be documented; as I said, this was a request for comments
about the overall design, before I finish polishing it and adding man
page documentation, etc. The patch was very much an interim patch,
including lots and lots of debugging printf's. :-)
>
> > [fs_types]
> > ext3 = {
> > features = has_journal
> > }
> > ext4 = {
> > features = extents,flex_bg
> > inode_size = 256
> > }
>
> Presumably the ext4 feature should also have features = has_journal?
> If this is the default for ext4, why would it need to be given for ext3?
>
> We should also add "dir_nlink,flexbg" while we are in there.
Yes, of course. This was an example, not what I plan to check in.
(And it's flex_bg, not flexbg; we also need to add the uninit_groups
flags, etc.)
The other thing which I've been considering is some what to make the
feature list displayed by dumpe2fs a bit easier to understand. One
thought is to bundle a number of features into things like std_ext2,
std_ext3, std_ext4, etc., with an option to display the full set for
someone who wants a more verbose/explicit description. The one caveat
here is that once a bundle is defined, we don't ever want to change it
so that when someone e-mail's a dumpe2fs output as part of a bug
report, there is no question about what a feature bundle means; it
can't be e2fsprogs version dependent.
- Ted
Theodore Tso wrote:
> There are only three things which mke2fs will do, in my design:
>
> #1) If the first type is not one of "ext2", "ext3", "ext4", or
> "ext4dev", mke2fs will determine a suitable default based on either
> argv[0] if it is of the form mke2fs.*, or via defaults.fs_type in
> /etc/mke2fs.conf. If neither of these is available it will default to "ext2".
>
> #2) If the second type is not one of "small", "floppy" or "default",
> mke2fs will create a default type by checking the size of the
> to-be-created filesystem. If it is less than 3mb, it is "floppy", if
> it is less than 512mb, it is "small", otherwise it is default.
>
> #3) Once it has the list of types, i.e., "ext3,defaults,squid", or
> "ext2,floppy,rescue", "ext4,defaults,largefile", etc. it uses this as
> a search path when mke2fs needs to look up some parameter, such as
> "base_features", or "inode_size", etc.
I started to look into updating the man page for this but it led to
several questions, and a suggestion....
Is this intended to take exactly 1, 2, or 3 arguments? If so is it
always "type," "size," "usepattern?" (ext4,small,news for example)
Can I specify ext4,defaults,news as well as ext4,news,defaults or does
order matter - it seems that it does.
Should it bail out if a stanza is not found (-T foo,bar,baz?) Today it
does not.
>From looking at & playing with the code it seems like it is supposed to
be explicitly type, size, usepattern, although sometimes it doesn't get
it right, for example this:
misc/mke2fs -T ext5,floppy,news fsfile
leads to:
fs_types: 'ext2', 'small', 'ext5', 'floppy', 'news'
hmm...
Also if these 3 positions have special meanings and orders, it's odd to
not have that reflected in the config file stanzas somehow...
It seems to me that perhaps instead of specifying that each
comma-delimited position has a specific meaning, perhaps it should just
be: "comma separated list which indicates profiles from least to most
specific, with values & features from more specific (later) profiles
trumping less specific (earlier) profiles" - would this be more clear &
flexible...?
Oh, and encountering an unknown type should probably toss an error...
-Eric
On Mon, Mar 17, 2008 at 04:29:18PM -0500, Eric Sandeen wrote:
> > #3) Once it has the list of types, i.e., "ext3,defaults,squid", or
> > "ext2,floppy,rescue", "ext4,defaults,largefile", etc. it uses this as
> > a search path when mke2fs needs to look up some parameter, such as
> > "base_features", or "inode_size", etc.
>
> I started to look into updating the man page for this but it led to
> several questions, and a suggestion....
>
> Is this intended to take exactly 1, 2, or 3 arguments? If so is it
> always "type," "size," "usepattern?" (ext4,small,news for example)
No, not necessarily. The user could specify something like
-T tpc-h,tweak1,tweak2
which would then cause mke2fs to look for the stanza's ext4, large,
tpc-h, tweak1, tweak2. What that means would be purely up to the user
who sets up the configuration file.
> Can I specify ext4,defaults,news as well as ext4,news,defaults or does
> order matter - it seems that it does.
Order does matter, because in the example above, tunings in tpc-h
override tunings made the ext4 and large stanza's, while configuration
tunings in tweak1 will overide those in tpc-h and before, and tweak2
will override tweak1 and before, etc.
> Should it bail out if a stanza is not found (-T foo,bar,baz?) Today it
> does not.
Hmm... good question. That would be good if someone were to typo a
type string.
> From looking at & playing with the code it seems like it is supposed to
> be explicitly type, size, usepattern, although sometimes it doesn't get
> it right, for example this:
>
> misc/mke2fs -T ext5,floppy,news fsfile
>
> leads to:
>
> fs_types: 'ext2', 'small', 'ext5', 'floppy', 'news'
>
> hmm...
Well, "ext5" isn't a valid filesystem type.
Also, most of the time I don't expect users to actually specify the
type and size all the time. Normally they won't do that. I would
expect that most of the size will be automated added by mke2fs, based
on the size of the of the partition. The filesystem type will be
added automatically if the user uses /sbin/mkfs.ext3 or
/sbin/mkfs.ext4, or via defaults.fs_type type in the configuration
file.
> Also if these 3 positions have special meanings and orders, it's odd to
> not have that reflected in the config file stanzas somehow...
I'm not sure what you mean by that.
> It seems to me that perhaps instead of specifying that each
> comma-delimited position has a specific meaning, perhaps it should just
> be: "comma separated list which indicates profiles from least to most
> specific, with values & features from more specific (later) profiles
> trumping less specific (earlier) profiles" - would this be more clear &
> flexible...?
I'd probably using "overridding" instead of "trumping", but yes.
> Oh, and encountering an unknown type should probably toss an error...
You mean, a type which doesn't have a profile stanza, as mentioned
above? Or did you mean something else?
- Ted
Theodore Tso wrote:
> On Mon, Mar 17, 2008 at 04:29:18PM -0500, Eric Sandeen wrote:
>>> #3) Once it has the list of types, i.e., "ext3,defaults,squid", or
>>> "ext2,floppy,rescue", "ext4,defaults,largefile", etc. it uses this as
>>> a search path when mke2fs needs to look up some parameter, such as
>>> "base_features", or "inode_size", etc.
>> I started to look into updating the man page for this but it led to
>> several questions, and a suggestion....
>>
>> Is this intended to take exactly 1, 2, or 3 arguments? If so is it
>> always "type," "size," "usepattern?" (ext4,small,news for example)
>
> No, not necessarily. The user could specify something like
>
> -T tpc-h,tweak1,tweak2
>
> which would then cause mke2fs to look for the stanza's ext4, large,
Hm, where did "ext4" and "large" come from? From being called as
mkfs.ext4 on a large block device?
> tpc-h, tweak1, tweak2. What that means would be purely up to the user
> who sets up the configuration file.
ok... but I think *if* you are going to specify fs type and size, then
they must be in the 1st and 2nd positions right? I'm just going by the
stuff like:
if (state == 1) {
if (strcmp(cp, "ext2") &&
strcmp(cp, "ext3") &&
....
which looks like fs type is explicitly checked on the fist pass?
IOW if I do a (silly) -T string like ext3,floppy,largefile I get:
fs_types: 'ext3', 'floppy', 'largefile'
but if I do -T floppy,ext3,largefile I get:
fs_types: 'ext2', 'floppy', 'ext3', 'largefile'
so in this case I guess I have "ext2" overriding defaults before the
others, when in the first case I did not? And depending on how [ext2]
is defined, this might matter.
>> Can I specify ext4,defaults,news as well as ext4,news,defaults or does
>> order matter - it seems that it does.
>
> Order does matter, because in the example above, tunings in tpc-h
> override tunings made the ext4 and large stanza's, while configuration
> tunings in tweak1 will overide those in tpc-h and before, and tweak2
> will override tweak1 and before, etc.
Right, for the named various named stanzas, it does matter in terms of
what is more specific. But, I meant "if I want to specify ext3 does it
have to be first," and I think it does, because otherwise the algorithm
picks ext2 for me...?
>> Should it bail out if a stanza is not found (-T foo,bar,baz?) Today it
>> does not.
>
> Hmm... good question. That would be good if someone were to typo a
> type string.
I think so. Otherwise it's silently dropped/ignored which is probably
not as planned.
>> From looking at & playing with the code it seems like it is supposed to
>> be explicitly type, size, usepattern, although sometimes it doesn't get
>> it right, for example this:
>>
>> misc/mke2fs -T ext5,floppy,news fsfile
>>
>> leads to:
>>
>> fs_types: 'ext2', 'small', 'ext5', 'floppy', 'news'
>>
>> hmm...
>
> Well, "ext5" isn't a valid filesystem type.
but it could be a valid name of a stanza, right. :) My point is that
there are a few things treated specially - type and size, I guess -
which need to be in the right positions, or they will get filled in with
default values, near as I can tell.
> Also, most of the time I don't expect users to actually specify the
> type and size all the time. Normally they won't do that. I would
> expect that most of the size will be automated added by mke2fs, based
> on the size of the of the partition. The filesystem type will be
> added automatically if the user uses /sbin/mkfs.ext3 or
> /sbin/mkfs.ext4, or via defaults.fs_type type in the configuration
> file.
Sure, but if -T is an option to mke2fs it still has to work clearly &
consistently.
>> Also if these 3 positions have special meanings and orders, it's odd to
>> not have that reflected in the config file stanzas somehow...
>
> I'm not sure what you mean by that.
I mean that we have a few "special" stanza names:
[ext2], [ext3], [ext4], [ext4dev]
[floppy], [small], [default]
which have some special meaning, (types and sizes), and will get filled
in for you if you don't specify them, right?
And for all the rest you can make [grandmas_cookies] or whatever you want.
But near as I can tell I have to specify type and size in the first two
slots or I get a different outcome.
>> It seems to me that perhaps instead of specifying that each
>> comma-delimited position has a specific meaning, perhaps it should just
>> be: "comma separated list which indicates profiles from least to most
>> specific, with values & features from more specific (later) profiles
>> trumping less specific (earlier) profiles" - would this be more clear &
>> flexible...?
>
> I'd probably using "overridding" instead of "trumping", but yes.
Details. ;)
I guess what I'm driving at is this: should the "if state == 1" and "if
state == 2" tests go away, and just handle each comma-separated type
completely equally. I'd thinkg that -T news,largefile,ext4 should work
- news should override all defaults, largefile should override anything
thus far, and ext4 should have the final say for anything it specifies;
but do not fill in "ext2, small" for me first as it does today:
mke2fs -T news,largefile,ext4 fsfile
yields:
fs_types: 'ext2', 'small', 'news', 'largefile', 'ext4'
the above is unclear/unexpected behavior, IMHO. I'd expect simply:
fs_types: 'news', 'largefile', 'ext4'
because that is what I specified.
>> Oh, and encountering an unknown type should probably toss an error...
>
> You mean, a type which doesn't have a profile stanza, as mentioned
> above? Or did you mean something else?
right, any comma-delimited type which isn't actually found somewhere
should probably cause mkfs to stop with an error.
Thanks,
-Eric
> - Ted
On Mon, Mar 17, 2008 at 10:23:56PM -0500, Eric Sandeen wrote:
> ok... but I think *if* you are going to specify fs type and size, then
> they must be in the 1st and 2nd positions right? I'm just going by the
> stuff like:
Yes, *if* you want to override the default filesystem type and size
type, then they must be the first and 2nd positions. Or put another
way, what is specified by -T is:
-T [fs_type,][size_type,]user_type[,additional_type...]
Where fs_type may be one of "ext2", "ext3", "ext4", or "ext4dev", and
size_type may be one of "floppy", "small", or "default".
> IOW if I do a (silly) -T string like ext3,floppy,largefile I get:
>
> fs_types: 'ext3', 'floppy', 'largefile'
>
> but if I do -T floppy,ext3,largefile I get:
>
> fs_types: 'ext2', 'floppy', 'ext3', 'largefile'
>
> so in this case I guess I have "ext2" overriding defaults before the
> others, when in the first case I did not? And depending on how [ext2]
> is defined, this might matter.
That's true, but in practice, I think it's highly unlikely most users
will specify more than a single argument to the -T option. Most users
will likely to do:
mke2fs -T news
or
mke2fs -T ext4
or maybe
mke2fs -T news
etc.
I want something sane to happen if they type
mke2fs -T ext4,news
and it should be no different than this:
/sbin/mkfs.ext4 -T news
But truth be told, it's highly unlikely they would do something more
complicated such as:
mke2fs -T tpc-h,tweek1,tweek2
...especially since nearly everything can be specified on the command
line anyway.
> Right, for the named various named stanzas, it does matter in terms of
> what is more specific. But, I meant "if I want to specify ext3 does it
> have to be first," and I think it does, because otherwise the algorithm
> picks ext2 for me...?
In practice, given that the ext3 filesystem stanza overrides
everything that is set by ext2, given the ordering rule which says
that last type wins, in practice
mke2fs -T news,ext3
which will expand to "ext2,default,ext3,news" in practice probably
won't be any different than "default,ext3,news", and if there are no
configuration knobs set by both the "default" and "ext3" stanzas, then
"default,ext3,news" will end up being equivalent to
"ext3,default,news". So in practice I don't expect things to be a
problem.
> > Hmm... good question. That would be good if someone were to typo a
> > type string.
>
> I think so. Otherwise it's silently dropped/ignored which is probably
> not as planned.
The only downside of doing this is if someone overrides
/etc/mke2fs.conf with a custom /home/tytso/.mke2fs.conf by setting the
MKE2FS_CONFIG environment variable, and /home/tytso/.mke2fs.conf
doesn't have all of the system-defined stanzas, you wouldn't want it
to fail. So probably the right way of resolving this is to flag an
error for a non-existent stanza if it was entered by the user on the
command-line, but not if it was automatically added by e2fsck.
> I mean that we have a few "special" stanza names:
>
> [ext2], [ext3], [ext4], [ext4dev]
> [floppy], [small], [default]
>
> which have some special meaning, (types and sizes), and will get filled
> in for you if you don't specify them, right?
Right. Part of the problem was that previously mke2fs supported a
single argument to -T, and sometimes the argument was a pre-defined
size type ("floppy", "small") and sometimes the argument was a
pre-defined usage type (i.e., "news").
So I needed to design a system which was backwards compatible with
existing usage, where -T was used to sometimes specify a size, and
sometimes a usage type (but not both), and make it more general.
That's one of the reasons why I designed it the way I did.
> I guess what I'm driving at is this: should the "if state == 1" and "if
> state == 2" tests go away, and just handle each comma-separated type
> completely equally. I'd thinkg that -T news,largefile,ext4 should work
> - news should override all defaults, largefile should override anything
> thus far, and ext4 should have the final say for anything it specifies;
The problem is that "-T news,largefile,ext4" probably won't do what
the user wants. In general the filesystem type (ext2, ext3, ext4),
etc., are going to define a set of general parameters, which you then
want to get overriden by the size, and then usage type overrides size
parameters. So that's why the only order which makes sense is
"fs_type,size_type,usage_type".
The question then is what to do when the user specifies something
non-sensical, where they put the filesystem type last. One approach
would be to use a two-pass algorithm, where you first scan the list
for any filesystem type or size types, and if so, you suppress
prefixing the list with the fs_type and size_type. I think that's
what you are suggesting.
Then there was my approach, which was admittedly partially influenced
by what was quicker and easier to code, which only checked the
expected position.
Yours probably does make a bit more sense, although it's a bit more of
a pain to code. But either way, it's to deal with a situation that we
really don't want to train the user to use since it will probably
surprise him and not do what he wants.
- Ted
Theodore Tso wrote:
> The question then is what to do when the user specifies something
> non-sensical, where they put the filesystem type last.
Well, I'd say do what they asked, treat the [filesystem type] stanza
just like any other, and anything in it which may override previous
stanzas, does.
> One approach
> would be to use a two-pass algorithm, where you first scan the list
> for any filesystem type or size types, and if so, you suppress
> prefixing the list with the fs_type and size_type. I think that's
> what you are suggesting.
Not exactly...
> Then there was my approach, which was admittedly partially influenced
> by what was quicker and easier to code, which only checked the
> expected position.
Well, I think that what I'm suggesting (but it's late, and I'm tired, so
it's hard for me to tell for sure... ;) is that there is no special
anything. You simply take the specified -T arguments in order, and use
that as the search/override order for any setting found in the stanzas
specified. This would probably be simplest to code, actually, as well
as simplest to document, and even most powerful/flexible for the user.
But, I suppose I can live with
-T [fs_type,][size_type,]user_type[,additional_type...]
which is reasonably clear, though... we just need to document that there
are 7 special stanza names in 2 categories, which the code will put
there *for* you if you don't put it them the right place, yourself.
It's just, IMHO, a more complicated and restricted specification for the
option.
-Eric
On Tue, Mar 18, 2008 at 12:16:53AM -0500, Eric Sandeen wrote:
> Well, I think that what I'm suggesting (but it's late, and I'm tired, so
> it's hard for me to tell for sure... ;) is that there is no special
> anything. You simply take the specified -T arguments in order, and use
> that as the search/override order for any setting found in the stanzas
> specified. This would probably be simplest to code, actually, as well
> as simplest to document, and even most powerful/flexible for the user.
>
> But, I suppose I can live with
>
> -T [fs_type,][size_type,]user_type[,additional_type...]
>
> which is reasonably clear, though... we just need to document that there
> are 7 special stanza names in 2 categories, which the code will put
> there *for* you if you don't put it them the right place, yourself.
> It's just, IMHO, a more complicated and restricted specification for the
> option.
Well, the problem is that we need to be compatible with previous
behavior, where the size type could be set automatically, and where
the filesystem type was intuited from /sbin/mke2fs.ext3. So it was
clear that
/sbin/mkfs.ext4 /dev/sda1
should be equivalent to
/sbin/mke2fs -T ext4 /dev/sda
But in that case, then what should this mean:
/sbin/mkfs.ext4 -T news /dev/sda1
The user very clearly wanted an ext4 filesystem, so we need to
give him the feature flag settings in the ext4 stanza, but she probably
also wanted inode ratio setting implied by -T news.
-----
If this is too confusing, one of the things which we *could* do is
decouple the filesystem size from everything else. "mke2fs -t" is a
deprecated alias for "mke2fs -c". We could make "/sbin/mkfs.ext4"
equivalent to "mke2fs -t ext4". Then we don't have to explain how
the filesystem type handling prepends onto the -T arguments.
If we do that, then we could just say that the size_type (floppy,
small, default) is only used if there is no -T option at all. I would
probably still do the chaining, just because I think it's a cool
feature, but documenting it becomes simpler because it removes a lot
of the special cases.
I rejected this approach originally because it would mean reusing the
-t option right away. But maybe this would be easier for users to
understand, and easier to document in the man pages, and maybe that's
an overriding consideration.
Regards,
- Ted
Theodore Tso wrote:
> I rejected this approach originally because it would mean reusing the
> -t option right away. But maybe this would be easier for users to
> understand, and easier to document in the man pages, and maybe that's
> an overriding consideration.
well, I doubt the man page should override much ;)
I just wanted to air the concerns & my ideas... if, having considered
them, you're still happy with the original approach, that's fine with me.
Thanks,
-Eric
On Tue, Mar 18, 2008 at 08:11:32AM -0500, Eric Sandeen wrote:
> Theodore Tso wrote:
>
> > I rejected this approach originally because it would mean reusing the
> > -t option right away. But maybe this would be easier for users to
> > understand, and easier to document in the man pages, and maybe that's
> > an overriding consideration.
>
> well, I doubt the man page should override much ;)
>
> I just wanted to air the concerns & my ideas... if, having considered
> them, you're still happy with the original approach, that's fine with me.
Sure, but making things easier to understand is important. So we
trade off making "mke2fs -t" doing something different between 1.40
and 1.41, with something that might be easier to understand.
So what do people think about this proposal as an alternative to
what's been discussed?
The filesystem type comes from defaults.fstype in mke2fs.conf, which
can be overridden by /sbin/mkfs.*, which in turn can be overriden by
the -t option. If the the filesystem type does not begin with the
string "ext" it will be rejected with an error. Call this the
"fs_type".
In addition, a list of types can be specified by the user using the -T
flag. Call it "type_list", and it consists of one or more usage types
separated by commas. If a type_list is not specified by -T, mke2fs
use a single type based on the size of the block device, selected from
the list "floppy, small, default".
Mke2fs will search mke2fs.conf for configuration parameters first by
looking at the fs_type, and then successively in the usage types found
in type_list. Parameters found later override earlier parameters.
I can get behind this, as it's compatible with what came before, and
simpler in the general case, and still as expressive for power users
who want to do really complex things in their mke2fs.conf file.
What do people think?
- Ted
On Mar 18, 2008 07:01 -0400, Theodore Ts'o wrote:
> If this is too confusing, one of the things which we *could* do is
> decouple the filesystem size from everything else. "mke2fs -t" is a
> deprecated alias for "mke2fs -c". We could make "/sbin/mkfs.ext4"
> equivalent to "mke2fs -t ext4". Then we don't have to explain how
> the filesystem type handling prepends onto the -T arguments.
Note that "-t" is not listed in the mke2fs man page, but it is
still listed in the mke2fs -h usage line:
Usage: mke2fs [-c|-t|-l filename] [-b block-size] [-f fragment-size]
[-i bytes-per-inode] [-I inode-size] [-j] [-J journal-options]
[-N number-of-inodes] [-m reserved-blocks-percentage] [-o creator-os]
[-g blocks-per-group] [-L volume-label] [-M last-mounted-directory]
[-O feature[,...]] [-r fs-revision] [-E extended-option[,...]] [-qvSV]
device [blocks-count]
> I rejected this approach originally because it would mean reusing the
> -t option right away. But maybe this would be easier for users to
> understand, and easier to document in the man pages, and maybe that's
> an overriding consideration.
How long ago was it deprecated, and are there major distros that still
document/use this feature? I suppose if the old code would fail with
any textual input (as used by new types), and the new code either
fails when no argument is given then it should be fairly clear if the
user has a conflicting usage.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
Theodore Tso wrote:
> So what do people think about this proposal as an alternative to
> what's been discussed?
>
> The filesystem type comes from defaults.fstype in mke2fs.conf, which
> can be overridden by /sbin/mkfs.*, which in turn can be overriden by
> the -t option. If the the filesystem type does not begin with the
> string "ext" it will be rejected with an error. Call this the
> "fs_type".
>
> In addition, a list of types can be specified by the user using the -T
> flag. Call it "type_list", and it consists of one or more usage types
> separated by commas. If a type_list is not specified by -T, mke2fs
> use a single type based on the size of the block device, selected from
> the list "floppy, small, default".
>
> Mke2fs will search mke2fs.conf for configuration parameters first by
> looking at the fs_type, and then successively in the usage types found
> in type_list. Parameters found later override earlier parameters.
>
> I can get behind this, as it's compatible with what came before, and
> simpler in the general case, and still as expressive for power users
> who want to do really complex things in their mke2fs.conf file.
>
> What do people think?
Ted, this sounds good to me.
-Eric
Theodore Tso wrote:
> On Tue, Mar 18, 2008 at 08:11:32AM -0500, Eric Sandeen wrote:
>> Theodore Tso wrote:
>>
>>> I rejected this approach originally because it would mean reusing the
>>> -t option right away. But maybe this would be easier for users to
>>> understand, and easier to document in the man pages, and maybe that's
>>> an overriding consideration.
>> well, I doubt the man page should override much ;)
>>
>> I just wanted to air the concerns & my ideas... if, having considered
>> them, you're still happy with the original approach, that's fine with me.
>
> Sure, but making things easier to understand is important. So we
> trade off making "mke2fs -t" doing something different between 1.40
> and 1.41, with something that might be easier to understand.
>
> So what do people think about this proposal as an alternative to
> what's been discussed?
So I was playing with another option here (sorry, can't stop beating
this dead horse...)
Rather than *looking* for extN and size type in the magic first and
second slots, what if we fill them in there internally, but they will be
overridden by any other -T options specified.
Internally, first an extN type is picked up from mkfs.FOO first, or
defaults fs type if not specified. Seond, also internally, a size type
is chosen next from the default size breakpoints.
So in that case, ext type and size type can be specified anywhere by the
user, and there is no magic; it just follows the precedence from the
order, and the default internal types can be overridden.
So if for example you specify "mkfs.ext3 -T ext4" it's a little
nonsensical, but internally the types are "ext3,ext4" and it's just
normal precedence from then on; anything not specified in ext4 is picked
up from ext3, else defaults.
If we craft the default ext3 and ext4 types properly, then this all
makes good sense, IMHO.
Likewise, if you do "mkfs.ext3 -T floppy" on a filesystem that would
otherwise be "small", internally it's "ext3,small,floppy" but any
size-related types will be found in the last-specified option, "floppy"
and since floppy re-specifies everything found in "small" the small
values are completely overridden.
Does this seem sane?
The man page would be something like:
-T fs-type1,fs-type2,...fs-typeN
Specify filesystem types in a comma-separated list,
with later types taking precedence over earlier types.
The types that can be supported are defined in the
mke2fs configuration file /etc/mke2fs.conf(5), and
are used to set the filesystem geometry and features.
Internally, two fs-types will always be placed ahead
of any types specified on the commandline.
First, an extN fs-type will be placed on the list.
If called as mkfs.ext2, mkfs.ext3, or mkfs.ext4,
the corresponding fs-type (ext2, ext3, or ext4)
will be chosen. Otherwise, an extN fs-type will
be taken from the fs_type parameter in the [defaults]
section of the configuration file. If no extN type
is found anywhere, the default will be ext2.
Second, a default size type of "floppy," "small," or
"default" will be chosen based on the device size
and placed next on the internal list.
These two internally-added fs-types can be overridden
by specifying a -T fs-type which will take precendence
and override those internal types' parameters.
Is this any simpler/clearer?
-Eric
On Thu, Mar 20, 2008 at 02:17:36PM -0500, Eric Sandeen wrote:
> Rather than *looking* for extN and size type in the magic first and
> second slots, what if we fill them in there internally, but they will be
> overridden by any other -T options specified.
>
> Internally, first an extN type is picked up from mkfs.FOO first, or
> defaults fs type if not specified. Seond, also internally, a size type
> is chosen next from the default size breakpoints.
Yes, this works, as long as we make sure every single filesystem and
size stanza in /etc/mke2fs.conf overrides each other completely.
Hmm.... Actually, as it's currently specified, it's not completely
true that a later parameter completely overrides a later one. The one
it's not true for is the "features" profile knob, where each specific
filesystem called e2p_edit_features(), so that one particular features
knob is cumulative.
Given that I think my previous proposal of overriding -t is probably
the better one, and concuptally simpler to understand and document.
- Ted