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

Darrick J. Wong
2024-04-16T17:59:09Z

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:

struct parent_rec *rec = ...;

fd = open_by_fshandle(&rec->gpr_handle, sizeof(rec->gpr_handle),
O_RDWR);

vs:

struct parent_rec *rec = ...;
struct xfs_handle *hanp;
size_t hlen;

fd_to_handle(child_fd, &hanp, &hlen);
hanp->ha_fid.fid_ino = rec->gpr_ino;
hanp->ha_fid.fid_gen = rec->gpr_gen;

fd = open_by_fshandle(hanp, hlen, O_RDWR);

--D