On Thu, Nov 14, 2024 at 11:51 PM Erin Shepherd <erin.shepherd@xxxxxx> wrote:
>
> On 14/11/2024 15:13, Christian Brauner wrote:
>
> > On Thu, Nov 14, 2024 at 02:13:06PM +0100, Erin Shepherd wrote:
> >> These two concerns combined with the special flag make me wonder if pidfs
> >> is so much of a special snowflake we should just special case it up front
> >> and skip all of the shared handle decode logic?
> > Care to try a patch and see what it looks like?
>
> The following is a completely untested sketch on top of the existing patch series.
> Some notes:
>
> - I made heavy use of the cleanup macros. I'm happy to convert things back to
> goto out_xx style if preferred - writing things this way just made bashing out
> the code without dropping resources on the floor easier
Your cleanup is very welcome, just please! not in the same patch as refactoring
and logic changes. Please do these 3 different things in different commits.
This patch is unreviewable as far as I am concerned.
> - If you don't implement fh_to_dentry then name_to_handle_at will just return an error
> unless called with AT_HANDLE_FID. We need to decide what to do about that
What's to decide? I did not understand the problem.
> - The GET_PATH_FD_IS_NORMAL/etc constants don't match (what I see as) usual kernel style
> but I'm not sure how to conventionally express something like that
I believe the conventional way to express a custom operation is an
optional method.
For example:
static int exportfs_get_name(struct vfsmount *mnt, struct dentry *dir,
char *name, struct dentry *child)
{
const struct export_operations *nop = dir->d_sb->s_export_op;
struct path path = {.mnt = mnt, .dentry = dir};
if (nop->get_name)
return nop->get_name(dir, name, child);
else
return get_name(&path, name, child);
}
There are plenty of optional custom inode, file, sb, dentry
operations with default fallback. some examples:
if (dir_inode->i_op->atomic_open) {
dentry = atomic_open(nd, dentry, file, open_flag, mode);
if (!splice && file_out->f_op->copy_file_range) {
ret = file_out->f_op->copy_file_range(file_in, pos_in,
file_out, pos_out,
len, flags);
} else if (!splice && file_in->f_op->remap_file_range && samesb) {
ret = file_in->f_op->remap_file_range(file_in, pos_in,
So I think the right model for you to follow is a custom optional
s_export_op->open_by_handle() operation.
Thanks,
Amir.
