On Tue, Dec 17, 2019 at 01:19:56PM +0100, Lukas Czerner wrote:
> On Tue, Dec 17, 2019 at 12:44:19AM +0000, Al Viro wrote:
> > On Wed, Nov 06, 2019 at 11:14:42AM +0100, Lukas Czerner wrote:
> > > + fsparam_string_empty
> > > + ("usrjquota", Opt_usrjquota),
> > > + fsparam_string_empty
> > > + ("grpjquota", Opt_grpjquota),
> >
> > Umm... That makes ...,usrjquota,... equivalent to ...,usrjquota=,...
> > unless I'm misreading the series. Different from mainline, right?
>
> Unfortunatelly yes, I do not think this is a problem, but if you have a
> solution within the new mount api framework I am happy to use it.
Er... Dump the fsparam_string_empty() use and instead of your
+ if (token == Opt_usrjquota) {
+ if (result.negated)
+ return clear_qf_name(sb, USRQUOTA);
+ else
+ return set_qf_name(sb, USRQUOTA, param);
do
+ if (token == Opt_usrjquota) {
+ if (!param->string[0])
+ return clear_qf_name(sb, USRQUOTA);
+ else
+ return set_qf_name(sb, USRQUOTA, param);
with the same for grpjquota?
> > > + fsparam_bool ("barrier", Opt_barrier),
> > > + fsparam_flag ("nobarrier", Opt_nobarrier),
> >
> > That's even more interesting. Current mainline:
> > barrier OK, sets EXT4_MOUNT_BARRIER
> > barrier=0 OK, sets EXT4_MOUNT_BARRIER
> > barrier=42 OK, sets EXT4_MOUNT_BARRIER
> > barrier=yes error
> > barrier=no error
> > nobarrier OK, clears EXT4_MOUNT_BARRIER
> > Unless I'm misreading your series, you get
> > barrier error
>
> Not really, this seems to be working as expected. Assuming that this
> didn't change since 5.4.0-rc6. I does make sense to me that specifying
> bool type parameter without any options would express "true".
>
>
> > barrier=0 OK, sets EXT4_MOUNT_BARRIER
>
>
> > barrier=42 error
> > barrier=yes OK, sets EXT4_MOUNT_BARRIER
> > barrier=no OK, sets EXT4_MOUNT_BARRIER
>
> Those three are different, just because of how param_book() work. I do
> not really see a problem with it, but if we want to keep it exactly the
> same as current mainline it would be difficult with how the current api
> works. Any suggestions ?
If fsparam_bool() doesn't do the right thing, perhaps it shouldn't be
used in the first place? Or changed, for that matter - it's not as if
we had many users of that thing and the only in-tree one is definitely
breaking userland ABI.
In case of ext4, after rereading that code (and getting some sleep) the
current behaviour is, AFAICS to accept barrier | nobarrier | barrier=<number>
with the last case being equialent to nobarrier when number is 0 and barrier
in all other cases. Is that an accurate description?
If so, I would prefer
fsparam_flag_no("barrier", Opt_barrier), // barrier | nobarrier
fsparam_u32("barrier", Opt_barrier), // barrier=<number>
as the solution, with fs_parse() having been taught to allow argument-bearing
and argument-less options with the same name, picking the right one. That
way Opt_nobarrier gets removed as well...
I'll push a branch with that stuff later today; will post when it's out...
