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

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, Oct 29, 2020 at 10:20:56AM +1100, Dave Chinner wrote:
> 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.

Done.

> > > > +	} 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 :)

Done.  The option is now -i instead of -d.

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

Done.

--D

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