diff mbox series

[17/18] xfs_repair: support bigtime

Message ID 159770524187.3958786.13522554876108493954.stgit@magnolia
State New
Headers show
Series xfsprogs: widen timestamps to deal with y2038 | expand

Commit Message

Darrick J. Wong Aug. 17, 2020, 11 p.m. UTC
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>
---
 repair/dinode.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Amir Goldstein Aug. 18, 2020, 2:58 p.m. UTC | #1
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.
Darrick J. Wong Aug. 18, 2020, 3:32 p.m. UTC | #2
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 mbox series

Patch

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"));