Re: [PATCH 2/2] xfs_db: add an ls command

Dave Chinner
2020-10-28T23:21:03Z

On Wed, Oct 28, 2020 at 03:50:46PM -0700, Darrick J. Wong wrote:
> 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:
> > > + 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?

I'd suggest that it should be the same as the telldir cookie that is
returned by the kernel for the given entry.

> > > + } 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...

You mean:

$ ls -i /bin/bash | cut -f 1 -d " "
175492
$

i.e. if you want to provide the inode number rather than just the
path, then let's use the same names as a real ls implementation :)

> 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.

I also suspect it would be simpler to separate it out into two
functions rather than the way it is implemented now....

> Maybe I should just simplify it to "display the inode number of whatever
> the path resolves to" instead of constructing an artificial directory
> entry.

*nod*

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com