Re: [PATCH 11/11] ext4: add cross rename support

[J. Bruce Fields]

On Sat, Jan 18, 2014 at 07:49:29AM +0100, Miklos Szeredi wrote:
> On Fri, Jan 17, 2014 at 11:08 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
> > On Fri, Jan 17, 2014 at 11:53:07PM +1300, Michael Kerrisk (man-pages) wrote:
> >> >        The following additional errors are defined for renameat2():
> >> >
> >> >        EOPNOTSUPP
> >> >               The filesystem does not support a flag in flags
> >>
> >> This is not the usual error for an invalid bit flag. Please make it EINVAL.
> >
> > I agree that EINVAL makes sense for an invalid bit flag.
> >
> > But renameat2() can also fail when the caller passes a perfectly valid
> > flags field but the paths resolve to a filesystem that doesn't support
> > the RENAME_EXCHANGE operation.  EOPNOTSUPP looks more appropriate in
> > that case.
> 
> OTOH, from the app's perspective, it makes little difference whether a
> particular kernel doesn't support the reanameat2 syscall, or it
> doesn't support RENAME_FOO flag or if it does support RENAME_FOO but
> not in all filesystems.  In all those cases it has to just fall back
> to something supported and it doesn't matter *why* it wasn't
> supported.

Well, in theory it could allow an optimization:

	if (kernel_has_foo) {
		ret = rename(.,.,.,.,RENAME_FOO);
		if (ret && errno == EINVAL)
			kernel_has_foo = 0;
	}
	if (!kernel_has_foo)
		fallback...

or maybe even:

	if (kernel_has_foo && fs_has_foo[fsid])
		ret = rename(.,.,.,.,RENAME_FOO);
		if (ret && errno == EINVAL)
			kernel_has_foo = 0;
		if (ret && errno == EOPNOTSUPP)
			fs_has_foo[fsid] = 0;
	}
	if (!kernel_has_foo || !fs_has_foo[fsid])
		fallback...

which may both be of dubious value--unless, say, you're implementing a
network protocol and making this distinction to your client allows it to
save server round trips.

That may not be *totally* farfetched--if for example we added rename2 to
the nfs protocol then servers probably will be required to make this
sort of distinction per filesystem, exactly to allow client logic like
the above.

And as long as we can, I'd just rather give the caller more information
than less.

As for precedent for EOPNOTSUPP: grepping through man-pages the one
documented use of EOPNOTSUPP I see for filesystems is fallocate, for a
similar "filesystem doesn't support this operation" case.  "git grep
EOPNOTSUPP fs/" in the kernel repo suggests there are many more such,
but I haven't tried to figure out what any of them are.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/