On Mon, Apr 24, 2017 at 7:00 AM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote: > On Fri, Mar 10, 2017 at 09:34:35AM +1100, Dave Chinner wrote: >> On Thu, Mar 09, 2017 at 06:57:51PM +0100, Luis R. Rodriguez wrote: >> > I used reset_opt() and went with "last entry specified wins". From my >> > review the goal of the respecification was to ensure each opt param >> > parsed would not reset a prior set param, a paranoid measure, however >> > this clearly does not work well if we want to allow for "last entry >> > specified wins", or re-use the validators for a config file parsing >> > for a first shot a parsing entries. >> >> Which is essentially broken, because doing something like: >> >> -m crc =1 -m reflink=1 -m crc=0 >> >> leaves you with an /invalid config/ because of the respecification >> of -m crc=0 and the order in which options are parsed and verified. >> >> Indeed, things like block and sector sizes are particularly nasty in >> this respect, because other options can be specified in block or >> sector units. SO things like: >> >> -s size=4k -b size=1s -s size=512 -d size=1000000s >> >> were considered valid. respecification of options like this is just >> borken, and even if we take "last specification wins" it still means >> that the block size specification is ambiguous and potentially >> incorrect depending on other options. Hence respecification of >> options is simply not allowed and post-processing of the options >> doesn't change that. > > We have to pick an approach and stick with, the above seems sensible. > >> i.e., the biggest issue with reusing the existing parsing code for >> the "default config" is that is doesn't just set default values - it >> prevents other options from being used. > > Right as per original design. > >> IOWs, the config file should >> set the default values in the option table, not set the options >> directly as happens on the command line.... > > As I respin my patches addressing concerns an issue I see with this is current > semantics for "defaultval" is not that they will be the defaults, but rather > they will be the defaults *iff* the user did specify the option on the command > line but did not provide an explicit value. This for example would not allow Yes, a confusing name. I'm aware of it and have a fix to rename it to .flagval. It was part of my big set before and now I moved it into the smaller set I submitted on this Sunday. The same set adds a new field .value, which can be used to specify default as in "if the user does not specify this option at all". Jan > distributions to disable a feature which perhaps is enabled by default in > future versions xfsprogs, or enable one which perhaps is by default disabled. > Even if the config file would have values specified, they will be ignored > unless the user passed a user input value with no value for it. > > If this is fine by everyone then great. > > Luis -- Jan Tulak jtulak@xxxxxxxxxx / jan@xxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
