On Tue, 1 Feb 2011, Andreas Dilger wrote:
> On 2011-02-01, at 10:14, Lukas Czerner wrote:
> > This commit adds extended options '-E' to the resize2fs code along with
> > the first extended option lazy_itable_init=n.
>
> Here it says the option is "=n", is it also possible to force this with "=y"?
Oh, no not really. It should be just =0 or =1, it is the same as we have in
mke2fs.
>
> > +static void parse_extended_opts(int *flags, const char *opts)
> > +{
> > + if (!strcmp(token, "lazy_itable_init")) {
> > + int lazy;
> > + if (arg)
> > + lazy = strtoul(arg, &p, 0);
> > + else
> > + lazy = 1;
> > + if (lazy)
> > + *flags |= RESIZE_LAZY_ITABLE_INIT;
>
> Here it parses the option as "=0" or "=1", not "=n" or "=y". It looks like "=n" will accidentally return 0 from strtoul(), so that works as expected, but the "=y" option would fail (it would also return 0).
As I mentioned it is not supposed to work with =y or =n, but only with
=0 or =1. So I think it is expected, isn't it ?
>
> > + if (r_usage) {
> > + fprintf(stderr, _("\nBad option(s) specified: %s\n\n"
> > + "\tlazy_itable_init=<0 to disable, 1 to enable>\n\n"),
>
> It looks a bit confusing "=<0 to disable", I thought initially that meant any value <= 0 would disable it, though I later see that there is a closing '>'. I think it is more standard to use "{}" braces for "one of these options must be given".
:) I did not noticed that before, but you're right, I'll change that to
use "{}".
>
> > @@ -232,6 +290,12 @@ int main (int argc, char ** argv)
> > + if (access("/sys/fs/ext4/features/lazy_itable_init", R_OK) == 0)
> > + flags |= RESIZE_LAZY_ITABLE_INIT;
> > +
> > + if (extended_opts)
> > + parse_extended_opts(&flags, extended_opts);
>
> Good that the command-line options override the default value.
Yes, it suppose to be that way.
>
> > +.B lazy_itable_init\fR[\fB= \fI<0 to disable, 1 to enable>\fR]
>
> Similarly, this should use "{}" around the options instead of "<>".
>
> > +If enabled and the uninit_bg feature is enabled, the inode table will
> > +not be fully initialized by
>
> I would write "not be zeroed out on disk", since it is otherwise unclear
> if "not fully initialized" means that there will be less inodes available
> or some other issues if the inode table is not "fully" initialized.
Right.
>
> Cheers, Andreas
>
Thanks!
-Lukas
--
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
