On Mon, Jun 27, 2016 at 10:34:39AM -0500, Eric Sandeen wrote:
>
>
> On 6/27/16 4:48 AM, Carlos Maiolino wrote:
> > On Fri, Jun 24, 2016 at 04:24:58PM -0500, Eric Sandeen wrote:
> >> kernel commit 5ef828c4
> >> xfs: avoid false quotacheck after unclean shutdown
> >>
> >> made xfs_sb_from_disk() also call xfs_sb_quota_from_disk
> >> by default.
> >>
> >> However, when this was merged to libxfs, existing separate
> >> calls to libxfs_sb_quota_from_disk remained, and calling it
> >> twice in a row on a V4 superblock leads to issues, because:
> >>
> >>
> >> if (sbp->sb_qflags & XFS_PQUOTA_ACCT) {
> >> ...
> >> sbp->sb_pquotino = sbp->sb_gquotino;
> >> sbp->sb_gquotino = NULLFSINO;
> >>
> >> and after the second call, we have set both pquotino and gquotino
> >> to NULLFSINO.
> >>
> >> Fix this by making it safe to call twice, and also remove the extra
> >> calls to libxfs_sb_quota_from_disk.
> >>
> >> This is only spotted when running xfstests with "-m crc=0" because
> >> the sb_from_disk change came about after V5 became default, and
> >> the above behavior only exists on a V4 superblock.
> >>
> >> Reported-by: Eryu Guan <eguan@xxxxxxxxxx>
> >> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> >> ---
> >>
> >>
> >> diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
> >> index 45db6ae..44f3e3e 100644
> >> --- a/libxfs/xfs_sb.c
> >> +++ b/libxfs/xfs_sb.c
> >> @@ -316,13 +316,16 @@ xfs_sb_quota_from_disk(struct xfs_sb *sbp)
> >> XFS_PQUOTA_CHKD : XFS_GQUOTA_CHKD;
> >> sbp->sb_qflags &= ~(XFS_OQUOTA_ENFD | XFS_OQUOTA_CHKD);
> >>
> >> - if (sbp->sb_qflags & XFS_PQUOTA_ACCT) {
> >> + if (sbp->sb_qflags & XFS_PQUOTA_ACCT &&
> >> + sbp->sb_gquotino != NULLFSINO) {
> >
> > Although I agree with this check, shouldn't we report some sort of error when it
> > happens? Once, it's not supposed to happen, and, might be a sign of corruption?
>
> I dunno, it would also happen if it gets called twice, which is intentionally
> made harmless by this change. We don't warn on free(NULL) for example...
>
Well, I don't 100% agree with not having a warning here, but it doesn't make the
patch less valuable.
Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> I don't think it needs a warning.
>
> -Eric
>
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
--
Carlos
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
