Re: [PATCH] xfs_db: use correct inode to set inode type

[Gao Xiang <hsiangkao@redhat.com>]

On Thu, Aug 13, 2020 at 11:29:05PM +0800, Zorro Lang wrote:

...

> > >  		ino = XFS_AGINO_TO_INO(mp, xfs_daddr_to_agno(mp, b),
> > > -			((b << BBSHIFT) >> mp->m_sb.sb_inodelog) %
> > > +			(((b << BBSHIFT) + bo) >> mp->m_sb.sb_inodelog) %
> > >  			XFS_AGB_TO_AGINO(mp, mp->m_sb.sb_agblocks));
> > >  		set_cur_inode(ino);
> > >  		return;
> > 
> > Not familar with such code, but after looking into a bit, (my premature
> > thought is that) I'm wondering if we need to reverify original buffer in
> > 
> > if (type->fields) {
> > 	...
> > 	set_cur()
> > }
> > 
> > iocur_top->typ = type;
> > 
> > /* verify the buffer if the type has one. */
> > ...
> > 
> > since set_cur() already verified the buffer in
> > set_cur->libxfs_buf_read->...->libxfs_readbuf_verify?
> > 
> > Not related to this patchset but I'm a bit curious about it now...
> 
> I'm not the one who learn about xfsprogs best:) But by looking into
> set_cur_inode() -> set_cur(), I think the set_cur() does the xfs_buf
> verification, so I don't think we need to do that again at the end
> of set_iocur_type().

Nope, I wasn't saying we need to add anything after set_cur_inode().
but commit 55f224baf83d ("xfs_db: update buffer size when new type is set")

In detail, I think it might be

if (type->fields) {
	...
	set_cur()
	return;		<- here
}

iocur_top->typ = type;

/* verify the buffer if the type has one. */
...

Thanks,
Gao Xiang

> 
> Thanks,
> Zorro
> 
> > 
> > Thanks,
> > Gao Xiang
> > 
> > > -- 
> > > 2.20.1
> > > 
> > 
>