On Wed, Oct 28, 2020 at 12:27:03PM +1100, Dave Chinner wrote:
> On Mon, Oct 26, 2020 at 04:32:41PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Add to xfs_db the ability to list a directory.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > db/namei.c | 380 ++++++++++++++++++++++++++++++++++++++++++++++
> > libxfs/libxfs_api_defs.h | 1
> > man/man8/xfs_db.8 | 14 ++
> > 3 files changed, 395 insertions(+)
> >
> >
> > diff --git a/db/namei.c b/db/namei.c
> > index 3c9889d62338..b2c036e6777a 100644
> > --- a/db/namei.c
> > +++ b/db/namei.c
> > @@ -221,8 +221,388 @@ static const cmdinfo_t path_cmd = {
> > .help = path_help,
> > };
> >
> > +/* List a directory's entries. */
> > +
> > +static const char *filetype_strings[XFS_DIR3_FT_MAX] = {
> > + [XFS_DIR3_FT_UNKNOWN] = N_("unknown"),
> > + [XFS_DIR3_FT_REG_FILE] = N_("regular"),
> > + [XFS_DIR3_FT_DIR] = N_("directory"),
> > + [XFS_DIR3_FT_CHRDEV] = N_("chardev"),
> > + [XFS_DIR3_FT_BLKDEV] = N_("blkdev"),
> > + [XFS_DIR3_FT_FIFO] = N_("fifo"),
> > + [XFS_DIR3_FT_SOCK] = N_("socket"),
> > + [XFS_DIR3_FT_SYMLINK] = N_("symlink"),
> > + [XFS_DIR3_FT_WHT] = N_("whiteout"),
> > +};
>
> What does N_() do that is different to _()?
LOL, it doesn't do anything at all!
Sigh... WTF was the point of commit 97294b227aefd?
> > +static const char *
> > +get_dstr(
> > + struct xfs_mount *mp,
> > + uint8_t filetype)
> > +{
> > + if (!xfs_sb_version_hasftype(&mp->m_sb))
> > + return filetype_strings[XFS_DIR3_FT_UNKNOWN];
> > +
> > + if (filetype >= XFS_DIR3_FT_MAX)
> > + return filetype_strings[XFS_DIR3_FT_UNKNOWN];
> > +
> > + return filetype_strings[filetype];
> > +}
> > +
> > +static void
> > +dir_emit(
> > + struct xfs_mount *mp,
> > + char *name,
> > + ssize_t namelen,
> > + xfs_ino_t ino,
> > + uint8_t dtype)
> > +{
> > + char *display_name;
> > + struct xfs_name xname = { .name = name };
> > + const char *dstr = get_dstr(mp, dtype);
> > + xfs_dahash_t hash;
> > + bool good;
> > +
> > + if (namelen < 0) {
> > + /* Negative length means that name is null-terminated. */
> > + display_name = name;
> > + xname.len = strlen(name);
> > + good = true;
> > + } else {
> > + /*
> > + * Otherwise, name came from a directory entry, so we have to
> > + * copy the string to a buffer so that we can add the null
> > + * terminator.
> > + */
> > + display_name = malloc(namelen + 1);
> > + memcpy(display_name, name, namelen);
> > + display_name[namelen] = 0;
> > + xname.len = namelen;
> > + good = libxfs_dir2_namecheck(name, namelen);
> > + }
> > + hash = libxfs_dir2_hashname(mp, &xname);
> > +
> > + dbprintf("%-18llu %-14s 0x%08llx %3d %s", ino, dstr, hash, xname.len,
> > + display_name);
> > + if (!good)
> > + dbprintf(_(" (corrupt)"));
> > + dbprintf("\n");
>
> Can we get this to emit the directory offset of the entry as well?
Er... I think so. Do you want to report the u32 value that gets loaded
in ctx->pos? Or the actual byte offset within the directory?
> Also, can this be done as a single dbprintf call like this?
>
> dbprintf(%-18llu %-14s 0x%08llx %3d %s %s\n",
> ino, dstr, hash, xname.len, display_name,
> good ? _("(good)") : _("(corrupt)"));
>
> (there will be lots of output on big directories....)
Ok.
> > +static int
> > +list_sfdir(
> > + struct xfs_da_args *args)
> > +{
> > + struct xfs_inode *dp = args->dp;
> > + struct xfs_mount *mp = dp->i_mount;
> > + struct xfs_dir2_sf_entry *sfep;
> > + struct xfs_dir2_sf_hdr *sfp;
> > + xfs_ino_t ino;
> > + unsigned int i;
> > + uint8_t filetype;
> > +
> > + sfp = (struct xfs_dir2_sf_hdr *)dp->i_df.if_u1.if_data;
> > +
> > + /* . and .. entries */
> > + dir_emit(args->dp->i_mount, ".", -1, dp->i_ino, XFS_DIR3_FT_DIR);
> > +
> > + ino = libxfs_dir2_sf_get_parent_ino(sfp);
> > + dir_emit(args->dp->i_mount, "..", -1, ino, XFS_DIR3_FT_DIR);
> > +
> > + /* Walk everything else. */
> > + sfep = xfs_dir2_sf_firstentry(sfp);
> > + for (i = 0; i < sfp->count; i++) {
> > + ino = libxfs_dir2_sf_get_ino(mp, sfp, sfep);
> > + filetype = libxfs_dir2_sf_get_ftype(mp, sfep);
> > +
> > + dir_emit(args->dp->i_mount, (char *)sfep->name, sfep->namelen,
> > + ino, filetype);
> > + sfep = libxfs_dir2_sf_nextentry(mp, sfp, sfep);
> > + }
> > +
> > + return 0;
> > +}
>
> Hmmm - how much of the xfs_readdir() implementation from the kernel
> does this duplicate? It doesn't contain the seek cookie stuff, but
> otherwise it's almost identical, right?
Yep. I think it also omits a fair amount of error handling since we'd
rather just keep going for as long as we can.
> [....]
>
> > +/* If the io cursor points to a directory, list its contents. */
> > +static int
> > +ls_cur(
> > + char *tag,
> > + bool direct)
>
> I find the name "direct" rather confusing here. according to
> the help below, it will be true when we want to "list the directory
> itself, not it's contents"....
>
>
> > +{
> > + struct xfs_inode *dp;
> > + int ret = 0;
> > +
> > + if (iocur_top->typ != &typtab[TYP_INODE]) {
> > + dbprintf(_("current object is not an inode.\n"));
> > + return -1;
> > + }
> > +
> > + ret = -libxfs_iget(mp, NULL, iocur_top->ino, 0, &dp);
> > + if (ret) {
> > + dbprintf(_("failed to iget directory %llu, error %d\n"),
> > + (unsigned long long)iocur_top->ino, ret);
> > + return -1;
> > + }
> > +
> > + if (S_ISDIR(VFS_I(dp)->i_mode) && !direct) {
> > + /* List the contents of a directory. */
> > + if (tag)
> > + dbprintf(_("%s:\n"), tag);
> > +
> > + ret = listdir(dp);
> > + if (ret) {
> > + dbprintf(_("failed to list directory %llu: %s\n"),
> > + (unsigned long long)iocur_top->ino,
> > + strerror(ret));
> > + ret = -1;
> > + goto rele;
> > + }
> > + } else if (direct || !S_ISDIR(VFS_I(dp)->i_mode)) {
> > + /* List the directory entry associated with a single file. */
> > + char inum[32];
> > +
> > + if (!tag) {
> > + snprintf(inum, sizeof(inum), "<%llu>",
> > + (unsigned long long)iocur_top->ino);
> > + tag = inum;
> > + } else {
> > + char *p = strrchr(tag, '/');
> > +
> > + if (p)
> > + tag = p + 1;
> > + }
> > +
> > + dir_emit(mp, tag, -1, iocur_top->ino,
> > + libxfs_mode_to_ftype(VFS_I(dp)->i_mode));
>
> I'm not sure what this is supposed to do - we turn the current inode
> if it's not a directory into a -directory entry- without actually
> know it's name? And we can pass in an inode that isn't a directory
> and do the same? This doesn't make a huge amount of sense to me - it
> tries to display the inode number as a dirent?
I added this (somewhat confusing) ability so that fstests could resolve
a path to an inode number without having to dig any farther into the
disk format.
IOWs, you can do:
ino=$(_scratch_xfs_db -c 'ls -d /usr/bin/bash')
to get the inode number directly. Without this, you'd have to do
something horrible like this...
ino=$(_scratch_xfs_db -c 'path /usr/bin/bash' -c 'print' -c 'stack' /dev/sda | \
tr ',' ' ' | \
awk '{if ($1 ~ /inumber/) {print $3; exit(0); } else if ($1 == "inode") {print $2; exit(0);}}')
To map a path to an inode number. I thought it made a lot more sense to
do that in C (even if it makes the xfs_db CLI a little weird) than
implement a bunch of string parsing after the fact.
Maybe I should just simplify it to "display the inode number of whatever
the path resolves to" instead of constructing an artificial directory
entry.
> > + } else {
> > + dbprintf(_("current inode %llu is not a directory.\n"),
> > + (unsigned long long)iocur_top->ino);
> > + ret = -1;
> > + goto rele;
> > + }
>
> I don't think we can get to this else branch. If we don't take the
> first branch (dir && !direct), the either we are not a dir or direct
> is set. The second branch will then be taken if we are not a dir or
> direct is set....
Yes, I /will/ do that.
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
