Re: [PATCH 4/4] xfs_db: add command to copy directory trees out of filesystems

Darrick J. Wong
2025-02-18T15:54:09Z

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@kernel.org>
> >
> > 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.

/* Cannot set ctime or btime */

> > + /* 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.

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