Re: [PATCH 02/26] xfs: refactor loading quota inodes in the regular case

Darrick J. Wong
2024-08-23T17:51:55Z

On Thu, Aug 22, 2024 at 09:31:58PM -0700, Christoph Hellwig wrote:
> > + xfs_ino_t ino = NULLFSINO;
> > +
> > + switch (type) {
> > + case XFS_DQTYPE_USER:
> > + ino = mp->m_sb.sb_uquotino;
> > + break;
> > + case XFS_DQTYPE_GROUP:
> > + ino = mp->m_sb.sb_gquotino;
> > + break;
> > + case XFS_DQTYPE_PROJ:
> > + ino = mp->m_sb.sb_pquotino;
> > + break;
> > + default:
> > + ASSERT(0);
> > + return -EFSCORRUPTED;
> > + }
>
> I'd probably split this type to ino lookup into a separate helper,
> but that doesn't really matter.

I tried that, but left it embedded here because I didn't want to write a
helper function that then had to return a magic value for "some
programmer f*cked up, let's just bail out" that also couldn't have been
read in from disk. In theory 0 should work because
xfs_sb_quota_from_disk should have converted that to NULLFSINO for us,
but that felt like a good way to introduce a subtle bug that will blow
up later.

I suppose 0 wouldn't be the worst magic value, since xfs_iget would just
blow up noisily for us. OTOH all this gets deleted at the other end of
the metadir series anyway so I'd preferentially fix xfs_dqinode_load.

Thanks for the review!

--D

> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
>