Re: [PATCH 00/19 v2] mkfs cleaning

Dave Chinner
2016-06-04T00:33:22Z

On Fri, Jun 03, 2016 at 02:09:19PM +0200, Jan Tulak wrote:
> [
> ​snip all, another issue, unrelated to the lsunit one]​
>
> >
> ​I realised one small glitch with conflicts watching in the code in
> combination with flags. It is not regression, I think, because b​efore now,
> it was not possible to do something like -l internal=0,logdev=something,
> but now it should be possible.

I think that overly complicates things. My original intent for this
work was to simplify the interface and code, not make it more
elaborate to support configurations that are theoretically possible
but completely redundant.

i.e. '-l internal=0,logdev=something' is identical in function to
'-l logdev=something', so there is no need to specify internal=0.
AFAICT, therxp eis never a need to specify '-l internal=[0|1]'
because if '-l logdev=<foo>' is specified, it implies an external
log is being configured, and in every other case the log is
internal.

> The code checks only whether an option was seen, not its value, so it
> does't know that we are in fact disabling something. I see two ways how to
> do solve it. One is a custom conflict solving for these cases, putting some
> if (flag1 && option2) somewhere once every argument and option is parsed.
> But this goes against what the patchset did and moves it again out of the
> option table...
>
> The other way is to make a special case for flags in the conflicts-handling
> code. Basically, I would extend the structure with something like:
>
> { .index = L_INTERNAL,
> .conflicts = { L_FILE,
> L_DEV,
> LAST_CONFLICT },
> .conflicts_ignore_on_false=true, // new item, shorter name to be found

What happens when you have two conflicts, on which you want to
ignore on false, the other you want to ignore on true? And where do
you draw the line? L_SU vs L_SUNIT conflict and throw an error only
when the values differ?

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com