On Tue, Feb 18, 2025 at 07:54:09AM -0800, Darrick J. Wong wrote:
> On Tue, Feb 18, 2025 at 12:36:33AM -0800, Christoph Hellwig wrote:
> > On Thu, Feb 06, 2025 at 03:03:32PM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> > >
> > > Aheada of deprecating V4 support in the kernel, let's give people a way
> > > to extract their files from a filesystem without needing to mount.
> >
> > So I've wanted a userspace file access for a while, but if we deprecate
> > the v4 support in the kernel that will propagte to libxfs quickly,
> > and this code won't help you with v4 file systems either. So I don't
> > think the rationale here seems very good.
>
> We aren't removing V4 support from the kernel until September 2030 and
> xfsprogs effectively builds with CONFIG_XFS_SUPPORT_V4=y. That should
> be enough time, right?
>
> > > extern void bmapinflate_init(void);
> > > +extern void rdump_init(void);
> >
> > No need for the extern.
>
> Ok.
>
> > > + /* XXX cannot copy fsxattrs */
> >
> > Should this be fixed first? Or document in a full sentence comment
> > explaining why it can't should not be?
>
> /* XXX cannot copy fsxattrs until setfsxattrat() syscall merge */
>
> > > + [1] = {
> > > + .tv_sec = inode_get_mtime_sec(VFS_I(ip)),
> > > + .tv_nsec = inode_get_mtime_nsec(VFS_I(ip)),
> > > + },
> > > + };
> > > + int ret;
> > > +
> > > + /* XXX cannot copy ctime or btime */
> >
> > Same for this and others.
>
> Is there a way to set ctime or btime? I don't know of any.
Now that I'm past all the morning meetings and have had time to research
things a little more deeply/wake up more: no, there's no way to set the
inode change or birth times.
> /* Cannot set ctime or btime */
So I'll go with ^^ this comment.
> > > + /* Format xattr name */
> > > + if (attr_flags & XFS_ATTR_ROOT)
> > > + nsp = XATTR_TRUSTED_PREFIX;
> > > + else if (attr_flags & XFS_ATTR_SECURE)
> > > + nsp = XATTR_SECURITY_PREFIX;
> > > + else
> > > + nsp = XATTR_USER_PREFIX;
> >
> > Add a self-cotained helper for this? I'm pretty sure we do this
> > translation in a few places.
Actually, xfsprogs doesn't:
$ git grep XATTR_SECURITY_PREFIX
db/rdump.c:293: nsp = XATTR_SECURITY_PREFIX;
mkfs/proto.c:393: } else if (!strncmp(attrname, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN)) {
mkfs/proto.c:394: args.name = (unsigned char *)attrname + XATTR_SECURITY_PREFIX_LEN;
The kernel gets a little closer in xfs_xattr.c:
$ git grep XATTR_SECURITY_PREFIX include/ fs/*.c fs/xfs/
fs/xattr.c:128: if (!strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN) ||
fs/xattr.c:228: int issec = !strncmp(name, XATTR_SECURITY_PREFIX,
fs/xattr.c:229: XATTR_SECURITY_PREFIX_LEN);
fs/xattr.c:249: const char *suffix = name + XATTR_SECURITY_PREFIX_LEN;
fs/xattr.c:442: if (!strncmp(name, XATTR_SECURITY_PREFIX,
fs/xattr.c:443: XATTR_SECURITY_PREFIX_LEN)) {
fs/xattr.c:444: const char *suffix = name + XATTR_SECURITY_PREFIX_LEN;
fs/xfs/xfs_xattr.c:207: .prefix = XATTR_SECURITY_PREFIX,
fs/xfs/xfs_xattr.c:304: prefix = XATTR_SECURITY_PREFIX;
fs/xfs/xfs_xattr.c:305: prefix_len = XATTR_SECURITY_PREFIX_LEN;
But xfs_xattr_put_listent has some custom logic in it:
if (flags & XFS_ATTR_ROOT) {
#ifdef CONFIG_XFS_POSIX_ACL
if (namelen == SGI_ACL_FILE_SIZE &&
strncmp(name, SGI_ACL_FILE,
SGI_ACL_FILE_SIZE) == 0) {
__xfs_xattr_put_listent(
context, XATTR_SYSTEM_PREFIX,
XATTR_SYSTEM_PREFIX_LEN,
XATTR_POSIX_ACL_ACCESS,
strlen(XATTR_POSIX_ACL_ACCESS));
} else if (namelen == SGI_ACL_DEFAULT_SIZE &&
strncmp(name, SGI_ACL_DEFAULT,
SGI_ACL_DEFAULT_SIZE) == 0) {
__xfs_xattr_put_listent(
context, XATTR_SYSTEM_PREFIX,
XATTR_SYSTEM_PREFIX_LEN,
XATTR_POSIX_ACL_DEFAULT,
strlen(XATTR_POSIX_ACL_DEFAULT));
}
#endif
/*
* Only show root namespace entries if we are actually allowed to
* see them.
*/
if (!capable(CAP_SYS_ADMIN))
return;
prefix = XATTR_TRUSTED_PREFIX;
prefix_len = XATTR_TRUSTED_PREFIX_LEN;
But I do see that rdump should translate SGI_ACL_FILE to
XATTR_POSIX_ACL_ACCESS so I'll go do that.
--D
> Ok. I think at least scrub phase5 does this.
>
> > > + if (XFS_IS_REALTIME_INODE(ip))
> > > + btp = ip->i_mount->m_rtdev_targp;
> > > + else
> > > + btp = ip->i_mount->m_ddev_targp;
> >
> > Should be move xfs_inode_buftarg from kernel code to common code?
>
> Hmm. The xfs_inode -> xfs_buftarg translation could be moved to
> libxfs/xfs_inode_util.c, yes. Though that can't happen until 6.15
> because we're well past the merge window. For now I think it's the only
> place in xfsprogs where we do that.
>
> --D
>
