Re: [PATCH v4 00/17] xfs: mount API patch series

[Ian Kent <raven@themaw.net>]

On Fri, 2019-10-04 at 16:25 +0800, Ian Kent wrote:
> On Fri, 2019-10-04 at 14:57 +0800, Ian Kent wrote:
> > On Thu, 2019-10-03 at 18:30 -0500, Eric Sandeen wrote:
> > > On 10/3/19 5:25 AM, Ian Kent wrote:
> > > > This patch series add support to xfs for the new kernel mount
> > > > API
> > > > as described in the LWN article at 
> > > > https://lwn.net/Articles/780267/
> > > > .
> > > > 
> > > > In the article there's a lengthy description of the reasons for
> > > > adopting the API and problems expected to be resolved by using
> > > > it.
> > > > 
> > > > The series has been applied to the repository located at
> > > > git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git, and built
> > > > and
> > > > some simple tests run on it along with the generic xfstests.
> > > > 
> > > > Other things that continue to cause me concern:
> > > > 
> > > > - Message logging.
> > > 
> > > ...
> > > 
> > > Haven't actually reviewed yet, but just playing with it, I
> > > noticed
> > > an
> > > oddity;
> > > 
> > > # mount -o loop,allocsize=abc fsfile mnt
> > > 
> > > fails as expected, but with no dmesg to be found.  Is that a
> > > known
> > > behavior?
> > 
> > That's interesting.
> 
> But it's not actually.
> 
> > I'll see if I can work out what path that is taking though the
> > kernel, don't think it's getting to the xfs options handling.
> 
> In the original xfs code, if there's a failure in xfs_parseargs()
> such as in this case when suffix_kstrtoint() fails, probably at
> kstrtoint(), the -EINVAL is returned to xfs_fs_fill_super() which
> subsequently returns that to the VFS.
> 
> The VFS itself doesn't log a failure message, it just returns the
> error to user space.
> 
> With the patch series applied, xfs_parse_param() does essentially
> the same thing and because it returns other than -ENOPARAM to the
> VFS the error is returned, without the VFS logging an error, to
> user space.
> 
> There are a few cases in xfs options handling where this happens.
> The series hasn't tried to change this.

While the description above is accurate in terms of xfs options
handling behaviour, in this case it's not the invalid allocsize
that's causing the failure, it fails with a valid size as well.

Certainly mount(8) is calling mount(2) trying a bunch of alternate
fs types, xfs being one, and is getting -EINVAL back from them all.

Not sure of the value of tracking down an explanation of this since
it's fairly well outside the scope of the series but I will if you
want me to.

> 
> > > -Eric