Re: [PATCH] xfs_db: properly set inode type

Eric Sandeen
2017-06-28T00:47:41Z

On 6/27/17 7:42 PM, Darrick J. Wong wrote:
> On Tue, Jun 27, 2017 at 07:16:39PM -0500, Eric Sandeen wrote:
>> When we set the type to "inode" the verifier validates multiple
>> inodes in the current fs block, so setting the buffer size to
>> that of just one inode is not sufficient and it'll emit spurious
>> verifier errors for all but the first, as we read off the end:
>>
>> xfs_db> daddr 99
>> xfs_db> type inode
>> Metadata corruption detected at xfs_inode block 0x63/0x200
>> Metadata corruption detected at xfs_inode block 0x63/0x200
>> Metadata corruption detected at xfs_inode block 0x63/0x200
>> Metadata corruption detected at xfs_inode block 0x63/0x200
>> Metadata corruption detected at xfs_inode block 0x63/0x200
>> Metadata corruption detected at xfs_inode block 0x63/0x200
>> Metadata corruption detected at xfs_inode block 0x63/0x200
>>
>> Use the special set_cur_inode() function for this purpose
>> as is done in inode_f().
>
> Do you need a similar thing for dquots? I sort of worry that we're
> going down the rabbit hole of special casing all over xfs_db, but...
> I'll defer to you. :)

I thought we did, but a quick test worked...

the dquot verifier only verifies one (indeed printing the dquot
only shows the first one in the block) so for now, I think not.

I did find it interesting/curious that i.e. "agi_f" hard-codes
the size and doesn't use the type's size function, but for
now I'm just ignoring that...

>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/db/io.c b/db/io.c
>> index b97b710..655a978 100644
>> --- a/db/io.c
>> +++ b/db/io.c
>> @@ -618,6 +618,18 @@ set_iocur_type(
>> struct xfs_buf *bp = iocur_top->bp;
>> int bb_count;
>>
>> + /* Inodes are special; verifier checks all inodes in the buffer */
>> + if (t->typnm == TYP_INODE) {
>> + xfs_daddr_t b = iocur_top->bb;
>> + xfs_ino_t ino;
>> +
>> + 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?

Macros, macros everywhere. Ok, sounds good.

-Eric

> --D
>
>> + set_cur_inode(ino);
>> + return;
>> + }
>> +
>> /* adjust cursor for types that contain fields */
>> if (t->fields) {
>> bb_count = BTOBB(byteize(fsize(t->fields, iocur_top->data, 0, 0)));
>>
>> --
>> 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