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

Darrick J. Wong
2020-10-29T18:42:04Z

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