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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxx>
> 
> 




[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux