On Wed, 2021-10-20 at 18:04 +0530, Venky Shankar wrote:
> On Tue, Oct 19, 2021 at 2:02 PM Ilya Dryomov <idryomov@xxxxxxxxx> wrote:
> >
> > On Fri, Oct 1, 2021 at 7:05 AM Venky Shankar <vshankar@xxxxxxxxxx> wrote:
> > >
> > > v4:
> > > - use mount_syntax_v1,.. as file names
> > >
> > > [This is based on top of new mount syntax series]
> > >
> > > Patrick proposed the idea of having debugfs entries to signify if
> > > kernel supports the new (v2) mount syntax. The primary use of this
> > > information is to catch any bugs in the new syntax implementation.
> > >
> > > This would be done as follows::
> > >
> > > The userspace mount helper tries to mount using the new mount syntax
> > > and fallsback to using old syntax if the mount using new syntax fails.
> > > However, a bug in the new mount syntax implementation can silently
> > > result in the mount helper switching to old syntax.
> > >
> > > So, the debugfs entries can be relied upon by the mount helper to
> > > check if the kernel supports the new mount syntax. Cases when the
> > > mount using the new syntax fails, but the kernel does support the
> > > new mount syntax, the mount helper could probably log before switching
> > > to the old syntax (or fail the mount altogether when run in test mode).
> > >
> > > Debugfs entries are as follows::
> > >
> > > /sys/kernel/debug/ceph/
> > > ....
> > > ....
> > > /sys/kernel/debug/ceph/meta
> > > /sys/kernel/debug/ceph/meta/client_features
> > > /sys/kernel/debug/ceph/meta/client_features/mount_syntax_v2
> > > /sys/kernel/debug/ceph/meta/client_features/mount_syntax_v1
> > > ....
> > > ....
> >
> > Hi Venky, Jeff,
> >
> > If this is supposed to be used in the wild and not just in teuthology,
> > I would be wary of going with debugfs. debugfs isn't always available
> > (it is actually compiled out in some configurations, it may or may not
> > be mounted, etc). With the new mount syntax feature it is not a big
> > deal because the mount helper should do just fine without it but with
> > other features we may find ourselves in a situation where the mount
> > helper (or something else) just *has* to know whether the feature is
> > supported or not and falling back to "no" if debugfs is not available
> > is undesirable or too much work.
> >
> > I don't have a great suggestion though. When I needed to do this in
> > the past for RADOS feature bits, I went with a read-only kernel module
> > parameter [1]. They are exported via sysfs which is guaranteed to be
> > available. Perhaps we should do the same for mount_syntax -- have it
> > be either 1 or 2, allowing it to be revved in the future?
>
> I'm ok with exporting via sysfs (since it's guaranteed). My only ask
> here would be to have the mount support information present itself as
> files rather than file contents to avoid writing parsing stuff in
> userspace, which is ok, however, relying on stat() is nicer.
>
You should be able to do that by just making a read-only parameter
called "mount_syntax_v2", and then you can test for it by doing
something like:
# stat /sys/module/ceph/parameters/mount_syntax_v2
The contents of the file can be blank (or just return Y or something).
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d6a3408a77807037872892c2a2034180fcc08d12
> >
> > Thanks,
> >
> > Ilya
> >
>
>
--
Jeff Layton <jlayton@xxxxxxxxxx>
