On Tue, 2024-11-19 at 14:02 +0100, Max Kellermann wrote:
> On Tue, Nov 19, 2024 at 1:51 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > -ENAMETOOLONG could be problematic there. This function is often called
> > when we have a dentry and need to build a path to it to send to the MDS
> > in a call. The system call that caused us to generate this path
> > probably doesn't involve a pathname itself, so the caller may be
> > confused by an -ENAMETOOLONG return.
>
> It is unfortunate that the Ceph-MDS protocol requires having to
> convert a file descriptor back to a path name - but do you really
> believe EIO would cause less confusion? ENAMETOOLONG is exactly what
> happens, even if it's an internal error. But there are many error
> codes that describe internal errors, so there's some prior art.
>
> EIO just doesn't fit, returning EIO would be confusing - even more so
> because EIO isn't a documented error code for open().
>
Fair enough. EIO is just my goto when I don't have a better idea.
Probably you want some weird error code that will make people stand up
and take notice. Maybe?
#define ENOSR 63 /* Out of streams resources */
> If this is about building path names for sending to the MDS, and not
> for the userspace ABI, maybe the PATH_MAX limitation is wrong here. If
> Ceph doesn't have such a limitation, the Ceph code shouldn't use the
> userspace ABI limit for protocol use.
>
It is possible to build a dir hierarchy that is deeper than can be
represented by a PATH_MAX-length string. No reason you can't allocate a
bigger buffer there. Does the MDS have a limit on the size of a path
string that it'll accept?
> > You may want to go with a more generic error code here -- -EIO or
> > something. It might also be worthwhile to leave in a pr_warn_once or
> > something since there may be users confused by this error return.
>
> Users cannot read the kernel log, and this allows users to flood the
> kernel log. So we get all the disadvantages of the kernel log while
> our users get none of the advantages.
Sure. I'd make this a pr_warn_once or heavily ratelimit it or
something. No need for a lot of these messages, but eventually users
are going to wonder why they're getting these weird errors and bug the
admins. Giving them this hint might be helpful.
Alternately you could add a conditional tracepoint or something, but
those are more obscure.
--
Jeff Layton <jlayton@xxxxxxxxxx>
