Re: [PATCH 00/19 v2] mkfs cleaning

Jan Tulak
2016-06-06T07:43:26Z

[-- Attachment #1.1: Type: text/plain, Size: 2236 bytes --]

On Sat, Jun 4, 2016 at 2:32 AM, Dave Chinner <david@fromorbit.com> wrote:

> 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?
>
>
​Yeah, that's true... let's call it feature then.

Cheers,
Jan​

--
Jan Tulak
jtulak@redhat.com / jan@tulak.me

[-- Attachment #1.2: Type: text/html, Size: 3769 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]