| Message ID | 159770524187.3958786.13522554876108493954.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfsprogs: widen timestamps to deal with y2038 | expand |
On Tue, Aug 18, 2020 at 2:23 AM Darrick J. Wong <darrick.wong@oracle.com> wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > Check the bigtime iflag in relation to the fs feature set. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Amir Goldstein <amir73il@gmail.com> and some questions below... > --- > repair/dinode.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > > diff --git a/repair/dinode.c b/repair/dinode.c > index ad2f672d8703..3507cd06075d 100644 > --- a/repair/dinode.c > +++ b/repair/dinode.c > @@ -2173,7 +2173,8 @@ check_nsec( > union xfs_timestamp *t, > int *dirty) > { > - if ((dip->di_flags2 & be64_to_cpu(XFS_DIFLAG2_BIGTIME)) || > + if ((dip->di_version >= 3 && It seems a bit strange that di_version check was added by this commit. > + (dip->di_flags2 & cpu_to_be64(XFS_DIFLAG2_BIGTIME))) || > be32_to_cpu(t->t_nsec) < NSEC_PER_SEC) > return; > > @@ -2601,6 +2602,16 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"), > flags2 &= ~XFS_DIFLAG2_COWEXTSIZE; > } > > + if ((flags2 & XFS_DIFLAG2_BIGTIME) && > + !xfs_sb_version_hasbigtime(&mp->m_sb)) { > + if (!uncertain) { > + do_warn( > + _("inode %" PRIu64 " is marked bigtime but file system does not support large timestamps\n"), > + lino); > + } > + flags2 &= ~XFS_DIFLAG2_BIGTIME; Should we maybe also reset the timestamps to epoc in this case? Thanks, Amir.
On Tue, Aug 18, 2020 at 05:58:11PM +0300, Amir Goldstein wrote: > On Tue, Aug 18, 2020 at 2:23 AM Darrick J. Wong <darrick.wong@oracle.com> wrote: > > > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Check the bigtime iflag in relation to the fs feature set. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > Reviewed-by: Amir Goldstein <amir73il@gmail.com> > > and some questions below... > > > --- > > repair/dinode.c | 13 ++++++++++++- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > > > diff --git a/repair/dinode.c b/repair/dinode.c > > index ad2f672d8703..3507cd06075d 100644 > > --- a/repair/dinode.c > > +++ b/repair/dinode.c > > @@ -2173,7 +2173,8 @@ check_nsec( > > union xfs_timestamp *t, > > int *dirty) > > { > > - if ((dip->di_flags2 & be64_to_cpu(XFS_DIFLAG2_BIGTIME)) || > > + if ((dip->di_version >= 3 && > > It seems a bit strange that di_version check was added by this commit. Ooh, yeah. That was added a few patches back, though it shouldn't have been. > > + (dip->di_flags2 & cpu_to_be64(XFS_DIFLAG2_BIGTIME))) || > > be32_to_cpu(t->t_nsec) < NSEC_PER_SEC) > > return; > > > > @@ -2601,6 +2602,16 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"), > > flags2 &= ~XFS_DIFLAG2_COWEXTSIZE; > > } > > > > + if ((flags2 & XFS_DIFLAG2_BIGTIME) && > > + !xfs_sb_version_hasbigtime(&mp->m_sb)) { > > + if (!uncertain) { > > + do_warn( > > + _("inode %" PRIu64 " is marked bigtime but file system does not support large timestamps\n"), > > + lino); > > + } > > + flags2 &= ~XFS_DIFLAG2_BIGTIME; > > Should we maybe also reset the timestamps to epoc in this case? Yeah, since it's possible that the timestamp would then have an invalid nsec field once the bigtime field is cleared. This also needs to log something about logging "...would zero timestamps and clear flag". Thanks for catching this. :) --D > > Thanks, > Amir.
diff --git a/repair/dinode.c b/repair/dinode.c index ad2f672d8703..3507cd06075d 100644 --- a/repair/dinode.c +++ b/repair/dinode.c @@ -2173,7 +2173,8 @@ check_nsec( union xfs_timestamp *t, int *dirty) { - if ((dip->di_flags2 & be64_to_cpu(XFS_DIFLAG2_BIGTIME)) || + if ((dip->di_version >= 3 && + (dip->di_flags2 & cpu_to_be64(XFS_DIFLAG2_BIGTIME))) || be32_to_cpu(t->t_nsec) < NSEC_PER_SEC) return; @@ -2601,6 +2602,16 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"), flags2 &= ~XFS_DIFLAG2_COWEXTSIZE; } + if ((flags2 & XFS_DIFLAG2_BIGTIME) && + !xfs_sb_version_hasbigtime(&mp->m_sb)) { + if (!uncertain) { + do_warn( + _("inode %" PRIu64 " is marked bigtime but file system does not support large timestamps\n"), + lino); + } + flags2 &= ~XFS_DIFLAG2_BIGTIME; + } + if (!verify_mode && flags2 != be64_to_cpu(dino->di_flags2)) { if (!no_modify) { do_warn(_("fixing bad flags2.\n"));