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

[Dave Chinner <david@fromorbit.com>]

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