Re: [PATCH] xfs_db: properly set inode type

Darrick J. Wong
2017-06-28T23:54:20Z

On Wed, Jun 28, 2017 at 05:45:55PM -0500, Eric Sandeen wrote:
> On 6/27/17 7:42 PM, Darrick J. Wong wrote:
> >> + ino = XFS_AGINO_TO_INO(mp, xfs_daddr_to_agno(mp, b),
> >> + ((b << BBSHIFT) >> mp->m_sb.sb_inodelog) %
> >> + (mp->m_sb.sb_agblocks << mp->m_sb.sb_inopblog));
>
> > XFS_OFFBNO_TO_AGINO(mp, xfs_daddr_to_agbno(mp, b), 0) instead of that
> > long third argument?
>
> Hm, nope:
>
> xfs_db> inode 99
> xfs_db> daddr
> current daddr is 99
>
> xfs_db> daddr 99
> xfs_db> type inode
> xfs_db> inode
> current inode number is 96
>
>
> That ends up taking the first inode in the fsblock (12), not the sector
> (99) I guess.
>
> The macro needs a non-zero offset into the fsblock... found by, um...
> I'm not sure that's going to be much prettier.
>
> How much do you hate how I wrote it first? ;) (I kind of hate it a
> lot but dunno what else we have?)

I guess there's also the problem that if inodesize != 512 then what are
we targeting, anyway? If inodesize = 256 then we can only hit
even-numbered inodes (not so bad) but if inodesize > 512 then do we jump
back to wherever the inode starts? Or just give the user what they
asked for, even if it's garbage?

(FWIW I was fine with xfs_db being dumb and giving you exactly what you
point it at, even if that makes no sense. :P)

--D

>
> -Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html