From: Dave Chinner <david@fromorbit.com>
To: Jan Tulak <jtulak@redhat.com>
Cc: Eric Sandeen <sandeen@sandeen.net>, xfs-oss <xfs@oss.sgi.com>
Subject: Re: [PATCH 00/19 v2] mkfs cleaning
Date: Sat, 4 Jun 2016 10:32:59 +1000 [thread overview]
Message-ID: <20160604003259.GV26977@dastard> (raw)
In-Reply-To: <CACj3i734=8ZJcdJztA76ru93SpdNNUgVtF_tUv2Jy_8U0V9HzQ@mail.gmail.com>
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 before 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
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2016-06-04 0:33 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-21 9:39 [PATCH 00/19 v2] mkfs cleaning Jan Tulak
2016-04-21 9:39 ` [PATCH 01/19] xfsprogs: use common code for multi-disk detection Jan Tulak
2016-04-21 9:39 ` [PATCH 02/19] mkfs: sanitise ftype parameter values Jan Tulak
2016-04-21 9:39 ` [PATCH 03/19] mkfs: Sanitise the superblock feature macros Jan Tulak
2016-05-02 23:06 ` Eric Sandeen
2016-05-04 0:48 ` Eric Sandeen
2016-04-21 9:39 ` [PATCH 04/19] mkfs: validate all input values Jan Tulak
2016-04-21 9:39 ` [PATCH 05/19] mkfs: factor boolean option parsing Jan Tulak
2016-04-21 9:39 ` [PATCH 06/19] mkfs: validate logarithmic parameters sanely Jan Tulak
2016-04-21 9:39 ` [PATCH 07/19] mkfs: structify input parameter passing Jan Tulak
2016-04-21 9:39 ` [PATCH 08/19] mkfs: getbool is redundant Jan Tulak
2016-05-02 23:08 ` Eric Sandeen
2016-04-21 9:39 ` [PATCH 09/19] mkfs: use getnum_checked for all ranged parameters Jan Tulak
2016-04-21 9:39 ` [PATCH 10/19] mkfs: add respecification detection to generic parsing Jan Tulak
2016-04-21 9:39 ` [PATCH 11/19] mkfs: table based parsing for converted parameters Jan Tulak
2016-05-02 23:09 ` Eric Sandeen
2016-04-21 9:39 ` [PATCH 12/19] mkfs: merge getnum Jan Tulak
2016-04-21 9:39 ` [PATCH 13/19] mkfs: encode conflicts into parsing table Jan Tulak
2016-05-02 23:11 ` Eric Sandeen
2016-05-03 23:39 ` Eric Sandeen
2016-05-04 0:47 ` Eric Sandeen
2016-04-21 9:39 ` [PATCH 14/19] mkfs: add string options to generic parsing Jan Tulak
2016-05-02 23:11 ` Eric Sandeen
2016-04-21 9:39 ` [PATCH 15/19] mkfs: don't treat files as though they are block devices Jan Tulak
2016-04-21 12:43 ` Jan Tulak
2016-04-21 20:13 ` Eric Sandeen
2016-04-22 7:46 ` Jan Tulak
2016-04-22 7:49 ` [PATCH 15/19 v2] " Jan Tulak
2016-04-29 14:47 ` [PATCH 15/19 v3] " Jan Tulak
2016-04-29 19:11 ` Eric Sandeen
2016-05-03 9:59 ` Jan Tulak
2016-05-02 23:13 ` [PATCH 15/19] " Eric Sandeen
2016-04-21 9:39 ` [PATCH 16/19] mkfs: move spinodes crc check Jan Tulak
2016-04-21 9:39 ` [PATCH 17/19] mkfs: unit conversions are case insensitive Jan Tulak
2016-04-21 9:39 ` [PATCH 18/19] mkfs: add optional 'reason' for illegal_option Jan Tulak
2016-04-21 9:39 ` [PATCH 19/19] mkfs: conflicting values with disabled crc should fail Jan Tulak
2016-04-28 8:29 ` [RFC PATCH] xfstests: Add mkfs input validation tests Jan Tulak
2016-04-29 1:59 ` Dave Chinner
2016-04-29 14:42 ` Jan Tulak
2016-05-02 23:05 ` [PATCH 00/19 v2] mkfs cleaning Eric Sandeen
2016-05-10 6:10 ` Dave Chinner
2016-06-01 13:19 ` Jan Tulak
2016-06-03 0:53 ` Dave Chinner
2016-06-03 9:20 ` Jan Tulak
2016-06-03 12:09 ` Jan Tulak
2016-06-04 0:32 ` Dave Chinner [this message]
2016-06-06 7:42 ` Jan Tulak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160604003259.GV26977@dastard \
--to=david@fromorbit.com \
--cc=jtulak@redhat.com \
--cc=sandeen@sandeen.net \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox