All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Lukas Czerner <lczerner@redhat.com>
Cc: linux-ext4@vger.kernel.org, Theodore Ts'o <tytso@mit.edu>,
	David Howells <dhowells@redhat.com>
Subject: Re: [PATCH 02/17] ext4: Add fs parameter description
Date: Tue, 24 Dec 2019 17:18:18 +0000	[thread overview]
Message-ID: <20191224171818.GO4203@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20191217163432.diborehdrfkmfqxp@work>

On Tue, Dec 17, 2019 at 05:34:32PM +0100, Lukas Czerner wrote:

> > 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...
> 
> That would be great, thanks.

It took longer than I hoped, sorry ;-/  The current patchset is in
#untested.fs_parse; the really interesting part is up to
"turn fs_param_is_... into functions".

One surprising source of PITA around your patchset is ext4_show_options().
It pretty much forces you into keeping "no..." forms separate, even though
normally you could just say
	fsparam_flag_no("quota",               Opt_quota),
and get rid of Opt_noquota, etc.

If you keep that dependency, it'll need to be documented - right in
fs/ext4/super.c, to make sure we don't get "optimizing" followups breaking
the hell out of things.

Said that, I really doubt that token2str() is a good idea.  It might make
more sense to start with separating _ext4_show_options() from that
machinery.

Another thing is that all fsparam_bool() users are modifying user-visible
ABI; use fparam_flag_no() + fsparam_u32() with the same name and same
opt - that'll give you the existing behaviour.

  reply	other threads:[~2019-12-24 17:18 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-06 10:14 ext4: new mount API conversion Lukas Czerner
2019-11-06 10:14 ` [PATCH 01/17] vfs: Handle fs_param_neg_with_empty Lukas Czerner
2019-11-06 10:14 ` [PATCH 02/17] ext4: Add fs parameter description Lukas Czerner
2019-12-17  0:44   ` Al Viro
2019-12-17 12:19     ` Lukas Czerner
2019-12-17 15:20       ` Al Viro
2019-12-17 16:34         ` Lukas Czerner
2019-12-24 17:18           ` Al Viro [this message]
2019-11-06 10:14 ` [PATCH 03/17] ext4: move option validation to a separate function Lukas Czerner
2019-11-06 10:14 ` [PATCH 04/17] ext4: Change handle_mount_opt() to use fs_parameter Lukas Czerner
2019-11-06 10:14 ` [PATCH 05/17] ext4: Allow sb to be NULL in ext4_msg() Lukas Czerner
2019-11-06 10:14 ` [PATCH 06/17] ext4: move quota configuration out of handle_mount_opt() Lukas Czerner
2019-11-06 10:14 ` [PATCH 07/17] ext4: check ext2/3 compatibility outside handle_mount_opt() Lukas Czerner
2019-11-06 10:14 ` [PATCH 08/17] ext4: get rid of super block and sbi from handle_mount_ops() Lukas Czerner
2019-11-06 10:14 ` [PATCH 09/17] ext4: parse Opt_sb in handle_mount_opt() Lukas Czerner
2019-11-06 10:14 ` [PATCH 10/17] ext4: clean up return values " Lukas Czerner
2019-11-06 10:14 ` [PATCH 11/17] ext4: add ext4_get_tree for the new mount API Lukas Czerner
2019-11-06 10:14 ` [PATCH 12/17] ext4: refactor ext4_remount() Lukas Czerner
2019-11-06 10:14 ` [PATCH 13/17] ext4: add ext4_reconfigure for the new mount API Lukas Czerner
2019-11-06 10:14 ` [PATCH 14/17] ext4: add ext4_fc_free " Lukas Czerner
2019-11-06 10:14 ` [PATCH 15/17] ext4: switch to " Lukas Czerner
2019-11-06 10:14 ` [PATCH 16/17] ext4: change token2str() to use ext4_param_specs Lukas Czerner
2019-11-06 10:14 ` [PATCH 17/17] ext4: Remove unused code from old mount api Lukas Czerner

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=20191224171818.GO4203@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=dhowells@redhat.com \
    --cc=lczerner@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.