On Wed, Apr 26, 2017 at 5:10 PM, Darrick J. Wong
<darrick.wong@xxxxxxxxxx> wrote:
> On Wed, Apr 26, 2017 at 09:02:44AM -0500, Eric Sandeen wrote:
>> The "defaultval" field in the options structure was a bit confusing,
>> so when the rmapbt & reflink options got added, the desire was
>> to keep them off by default, and "defaultval = 0" got set.
>>
>> However, the purpose of this field is to define the default value
>> when the flag is specified with no associated value, i.e.
>>
>> -m rmapbt vs. -m rmapbt=0 or -m rmapbt=1
>>
>> Today, the resulting behavior is unexpected, and different from any
>> other mkfs flags; specifying "-m rmapbt,reflink" results in a
>> filesystem /without/ those features.
>>
>> Fix these to be consistent with every other boolean flag in the
>> mkfs options, so that specifying the flag with no value will
>> enable the feature.
>>
>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>
> Someone should document that 'defaultval' is not the way to say
> 'disabled by default' in whatever the mkfs option processing code
> turns into.
>
> (Granted maybe it does and I just can't read :P)
The current documentation is confusing about this too, which is my
fault. I already have a patch in my current set that renames it to
flagval and makes it clear what it does. And maybe I will remove it
altogether in exchange for some way how to explicitly say "This is a
flag, it has only 0 and 1. Thus, no value means 'enable it.'" in
another patchset.
Cheers,
Jan
>
> Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> --D
>
>> ---
>>
>> I want to get this fixed up for 4.11, so just sending it separately
>> ahead of any other change Jan may make in this area.
>>
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index affa405..10858e4 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -697,13 +697,13 @@ struct opt_params mopts = {
>> .conflicts = { LAST_CONFLICT },
>> .minval = 0,
>> .maxval = 1,
>> - .defaultval = 0,
>> + .defaultval = 1,
>> },
>> { .index = M_REFLINK,
>> .conflicts = { LAST_CONFLICT },
>> .minval = 0,
>> .maxval = 1,
>> - .defaultval = 0,
>> + .defaultval = 1,
>> },
>> },
>> };
>>
--
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
