All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josef Bacik <josef@toxicpanda.com>
To: Christian Brauner <brauner@kernel.org>
Cc: linux-fsdevel@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 0/4] Add the ability to query mount options in statmount
Date: Tue, 25 Jun 2024 09:00:08 -0400	[thread overview]
Message-ID: <20240625130008.GA2945924@perftesting> (raw)
In-Reply-To: <20240625-tragbar-sitzgelegenheit-48f310320058@brauner>

On Tue, Jun 25, 2024 at 12:42:14PM +0200, Christian Brauner wrote:
> On Mon, Jun 24, 2024 at 03:40:49PM GMT, Josef Bacik wrote:
> > Hello,
> > 
> > Currently if you want to get mount options for a mount and you're using
> > statmount(), you still have to open /proc/mounts to parse the mount options.
> > statmount() does have the ability to store an arbitrary string however,
> > additionally the way we do that is with a seq_file, which is also how we use
> > ->show_options for the individual file systems.
> > 
> > Extent statmount() to have a flag for fetching the mount options of a mount.
> > This allows users to not have to parse /proc mount for anything related to a
> > mount.  I've extended the existing statmount() test to validate this feature
> > works as expected.  As you can tell from the ridiculous amount of silly string
> > parsing, this is a huge win for users and climate change as we will no longer
> > have to waste several cycles parsing strings anymore.
> > 
> > This is based on my branch that extends listmount/statmount to walk into foreign
> > namespaces.  Below are links to that posting, that branch, and this branch to
> > make it easier to review.
> 
> So I was very hesitant to do it this way because I feel this is pretty
> ugly dumping mount options like that but Karel and others have the same
> use-case that they want to retrieve it all via statmount() (or another
> ID-based system call) so I guess I'll live with this. But note that this
> will be a fairly ugly interface at times. For example, mounting overlayfs with
> 500 lower layers then what one gets is:
> 

Yeah this isn't awesome, but neither is the string parsing code I have in the
selftest to validate this feature.

I chose this approach because 1) it's simple and I'm lazy, and 2) I think
anything else becomes a lot more complicated and more work than what people
actually want.

I imagine what would be ideal would be some sort of mount option iterator
mechanism.  Either we shoe-horn it into statmount() or we add a
listmountoptions() syscall, and we then get back a list of mount options
individually laid out.  This means we now have to keep track of where we are in
our mount option traversal, and change all the ->show_options callbacks to
handle this new iter thing.

We could go this way I suppose, but again this is a lot of work, and honestly I
just want to log mount options into some database so I can go looking for people
doing weird shit on my giant fleet of machines/containers.  Would the iter thing
make the overlayfs thing better?  Yeah for sure.  Do we care?  I don't think so,
we just want all the options, and we can all strsep/strtok with a comma.
Thanks,

Josef

  reply	other threads:[~2024-06-25 13:00 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-24 19:40 [PATCH 0/4] Add the ability to query mount options in statmount Josef Bacik
2024-06-24 19:40 ` [PATCH 1/4] fs: rename show_mnt_opts -> show_vfsmnt_opts Josef Bacik
2024-06-24 19:40 ` [PATCH 2/4] fs: add a helper to show all the options for a mount Josef Bacik
2024-06-25 14:16   ` Christian Brauner
2024-06-26  7:47     ` Karel Zak
2024-06-24 19:40 ` [PATCH 3/4] fs: export mount options via statmount() Josef Bacik
2024-06-24 19:40 ` [PATCH 4/4] sefltests: extend the statmount test for mount options Josef Bacik
2024-06-24 19:53 ` [PATCH 0/4] Add the ability to query mount options in statmount Jeff Layton
2024-06-25 10:42 ` Christian Brauner
2024-06-25 13:00   ` Josef Bacik [this message]
2024-06-25 13:04     ` Miklos Szeredi
2024-06-25 13:35       ` Christian Brauner
2024-06-25 13:52         ` Karel Zak
2024-06-25 13:55           ` Christian Brauner
2024-06-25 14:17           ` Josef Bacik
2024-06-26  7:34             ` Karel Zak
2024-06-26 12:23             ` Miklos Szeredi
2024-11-11 13:12               ` Miklos Szeredi
2024-11-11 13:29                 ` Christian Brauner
2024-11-11 13:47                   ` Miklos Szeredi
2024-11-11 15:28                 ` Josef Bacik
2024-11-11 16:02                   ` Miklos Szeredi
2024-11-12  8:54                     ` Karel Zak
2024-06-26 12:03 ` (subset) " Christian Brauner

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=20240625130008.GA2945924@perftesting \
    --to=josef@toxicpanda.com \
    --cc=brauner@kernel.org \
    --cc=kernel-team@fb.com \
    --cc=linux-fsdevel@vger.kernel.org \
    /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.