Re: [PATCH 26/31] xfs: add parent pointer ioctls

Christoph Hellwig
2024-04-16T18:08:31Z

On Tue, Apr 16, 2024 at 10:59:08AM -0700, Darrick J. Wong wrote:
> On Mon, Apr 15, 2024 at 10:21:03PM -0700, Christoph Hellwig wrote:
> > > + if (memcmp(&handle->ha_fsid, mp->m_fixedfsid, sizeof(struct xfs_fsid)))
> > > + return -ESTALE;
> > > +
> > > + if (handle->ha_fid.fid_len != xfs_filehandle_fid_len())
> > > + return -EINVAL;
> >
> > Maybe we should just stash the fid without the fsid? The check for a
> > match fsid when userspace already had to resolve it to even call the
> > ioctl is a bit silly.
>
> Remember a few revisions ago when this ioctl only returned the raw fid
> information? At the time, I didn't want to bloat struct parent_rec with
> the full handle information, but then I changed it to export a full
> handle. When I went to update libfrog/getparents.c, I realized that
> giving full handles to userspace is a much better interface because any
> code that's trying to walk up the directory tree to compute the file
> path can simply call getparents again with the handle it has been given.
>
> It's much more ergonomic if userspace programs don't need to know how to
> construct handles from partial information they has been given. If a
> calling program wants to open the parent directory, it can call
> open_by_fshandle directly:

Yeah. I think I actually suggested that :) So I'll take that back.
That just leaves us with the question if we want to validate the
fsid here and diverge from the mormal handle ops or not. I think
I'd prefer behaving the same as the other handle ops just to avoid
confusion.