diff mbox series

[20/26] xfs_db: report bigtime format timestamps

Message ID 160375537615.881414.8162037930017365466.stgit@magnolia (mailing list archive)
State Superseded
Headers show
Series xfsprogs: widen timestamps to deal with y2038 | expand

Commit Message

Darrick J. Wong Oct. 26, 2020, 11:36 p.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

Report the large format timestamps.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/fprint.c              |   66 ++++++++++++++++++++++++++++++++--------------
 db/inode.c               |    4 ++-
 db/sb.c                  |    2 +
 libxfs/libxfs_api_defs.h |    1 +
 4 files changed, 52 insertions(+), 21 deletions(-)

Comments

Christoph Hellwig Oct. 29, 2020, 9:50 a.m. UTC | #1 
> +static void
> +fp_time64(
> +	time64_t		sec)
>  {
> +	time_t			tt = sec;
>  	char			*c;
> +
> +	BUILD_BUG_ON(sizeof(long) != sizeof(time_t));

Why?
Darrick J. Wong Oct. 29, 2020, 5:45 p.m. UTC | #2 
On Thu, Oct 29, 2020 at 09:50:10AM +0000, Christoph Hellwig wrote:
> > +static void
> > +fp_time64(
> > +	time64_t		sec)
> >  {
> > +	time_t			tt = sec;
> >  	char			*c;
> > +
> > +	BUILD_BUG_ON(sizeof(long) != sizeof(time_t));
> 
> Why?

Trying to make the best of a braindead situation.  IIRC C99/11/18 don't
provide a specific definition of what time_t is supposed to be.  POSIX
2017 seems to hint that it should be an integer seconds counter, but
doesn't provide any further clarity.  (And then says it defers to ISO C,
having made that allusion to integerness.)

Since I'd rather print a raw s64 value than risk truncating a time and
printing a totally garbage prettyprinted timestamp, I added the
LONG_{MIN,MAX} checks, but that assumes that time_t is a long.

Hence adding a trap so that if xfsprogs ever does encounter C library
where time_t isn't a long int, we'd get to hear about it.  Granted that
further assumes that time_t isn't a float, but ... ugh.

I guess this could have assigned sec to a time_t value and then compared
it back to the original value to see if we ripped off any upper bits.

--D
Christoph Hellwig Oct. 30, 2020, 8:23 a.m. UTC | #3 
On Thu, Oct 29, 2020 at 10:45:44AM -0700, Darrick J. Wong wrote:
> On Thu, Oct 29, 2020 at 09:50:10AM +0000, Christoph Hellwig wrote:
> > > +static void
> > > +fp_time64(
> > > +	time64_t		sec)
> > >  {
> > > +	time_t			tt = sec;
> > >  	char			*c;
> > > +
> > > +	BUILD_BUG_ON(sizeof(long) != sizeof(time_t));
> > 
> > Why?
> 
> Trying to make the best of a braindead situation.  IIRC C99/11/18 don't
> provide a specific definition of what time_t is supposed to be.  POSIX
> 2017 seems to hint that it should be an integer seconds counter, but
> doesn't provide any further clarity.  (And then says it defers to ISO C,
> having made that allusion to integerness.)

I'm pretty sure the x32 ABI has a time_t that is bigger than long,
which broken POSIX semantics.

> Hence adding a trap so that if xfsprogs ever does encounter C library
> where time_t isn't a long int, we'd get to hear about it.  Granted that
> further assumes that time_t isn't a float, but ... ugh.
> 
> I guess this could have assigned sec to a time_t value and then compared
> it back to the original value to see if we ripped off any upper bits.

I think the standard way would be an intmax_t local variable that the
value is assigned to.
Eric Sandeen Nov. 16, 2020, 9:16 p.m. UTC | #4 
On 10/26/20 6:36 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Report the large format timestamps.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

hch had some concerns about this, but we didn't seem to reach a
resolution... I'd like to get the bigtime stuff done this release
and not navel-gaze too much about weird abis etc, so I'm inclined
to just take this and we can fix it later in the release and/or
when somebody hits that BUG_ON...

Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Darrick J. Wong Nov. 16, 2020, 10:41 p.m. UTC | #5 
On Mon, Nov 16, 2020 at 03:16:17PM -0600, Eric Sandeen wrote:
> On 10/26/20 6:36 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Report the large format timestamps.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> hch had some concerns about this, but we didn't seem to reach a
> resolution... I'd like to get the bigtime stuff done this release
> and not navel-gaze too much about weird abis etc, so I'm inclined
> to just take this and we can fix it later in the release and/or
> when somebody hits that BUG_ON...
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>

...and while I wasn't looking/these patches were all sitting in my dev
tree, this[1] happened.  So now that musl unconditionally defines time_t
to be an int64_t even on 32-bit platforms, I guess this is broken and
I'll try to go whip up a fix.

--D

[1] https://musl.libc.org/time64.html
diff mbox series

Patch

diff --git a/db/fprint.c b/db/fprint.c
index 7ceab29cc608..48562f0c9518 100644
--- a/db/fprint.c
+++ b/db/fprint.c
@@ -112,23 +112,43 @@  fp_sarray(
 	return 1;
 }
 
-int
-fp_time(
-	void			*obj,
-	int			bit,
-	int			count,
-	char			*fmtstr,
-	int			size,
-	int			arg,
-	int			base,
-	int			array)
+static void
+fp_time64(
+	time64_t		sec)
 {
-	struct timespec64	tv;
-	xfs_timestamp_t		*ts;
-	int			bitpos;
+	time_t			tt = sec;
 	char			*c;
+
+	BUILD_BUG_ON(sizeof(long) != sizeof(time_t));
+
+	if (sec > LONG_MAX || sec < LONG_MIN)
+		goto raw;
+
+	c = ctime(&tt);
+	if (!c)
+		goto raw;
+
+	dbprintf("%24.24s", c);
+	return;
+raw:
+	dbprintf("%lld", sec);
+}
+
+int
+fp_time(
+	void			*obj,
+	int			bit,
+	int			count,
+	char			*fmtstr,
+	int			size,
+	int			arg,
+	int			base,
+	int			array)
+{
+	struct timespec64	tv;
+	xfs_timestamp_t		*ts;
+	int			bitpos;
 	int			i;
-	time_t			t;
 
 	ASSERT(bitoffs(bit) == 0);
 	for (i = 0, bitpos = bit;
@@ -139,10 +159,8 @@  fp_time(
 
 		ts = obj + byteize(bitpos);
 		tv = libxfs_inode_from_disk_ts(obj, *ts);
-		t = tv.tv_sec;
 
-		c = ctime(&t);
-		dbprintf("%24.24s", c);
+		fp_time64(tv.tv_sec);
 
 		if (i < count - 1)
 			dbprintf(" ");
@@ -195,7 +213,8 @@  fp_qtimer(
 	int			base,
 	int			array)
 {
-	uint32_t		sec;
+	struct xfs_disk_dquot	*ddq = obj;
+	time64_t		sec;
 	__be32			*t;
 	int			bitpos;
 	int			i;
@@ -208,9 +227,16 @@  fp_qtimer(
 			dbprintf("%d:", i + base);
 
 		t = obj + byteize(bitpos);
-		sec = be32_to_cpu(*t);
+		sec = libxfs_dquot_from_disk_ts(ddq, *t);
 
-		dbprintf("%u", sec);
+		/*
+		 * Display the raw value if it's the default grace expiration
+		 * period (root dquot) or if the quota has not expired.
+		 */
+		if (ddq->d_id == 0 || sec == 0)
+			dbprintf("%lld", sec);
+		else
+			fp_time64(sec);
 
 		if (i < count - 1)
 			dbprintf(" ");
diff --git a/db/inode.c b/db/inode.c
index cc0e680aadea..f0e08ebf5ad9 100644
--- a/db/inode.c
+++ b/db/inode.c
@@ -175,10 +175,12 @@  const field_t	inode_v3_flds[] = {
 	{ "dax", FLDT_UINT1,
 	  OI(COFF(flags2) + bitsz(uint64_t) - XFS_DIFLAG2_DAX_BIT - 1), C1,
 	  0, TYP_NONE },
+	{ "bigtime", FLDT_UINT1,
+	  OI(COFF(flags2) + bitsz(uint64_t) - XFS_DIFLAG2_BIGTIME_BIT - 1), C1,
+	  0, TYP_NONE },
 	{ NULL }
 };
 
-
 const field_t	timestamp_flds[] = {
 	{ "sec", FLDT_TIME, OI(0), C1, 0, TYP_NONE },
 	{ "nsec", FLDT_NSEC, OI(0), C1, 0, TYP_NONE },
diff --git a/db/sb.c b/db/sb.c
index b1033e5ef7f0..a04f36c73255 100644
--- a/db/sb.c
+++ b/db/sb.c
@@ -727,6 +727,8 @@  version_string(
 		strcat(s, ",REFLINK");
 	if (xfs_sb_version_hasinobtcounts(sbp))
 		strcat(s, ",INOBTCNT");
+	if (xfs_sb_version_hasbigtime(sbp))
+		strcat(s, ",BIGTIME");
 	return s;
 }
 
diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index 40da71ab3163..419e6d9888cf 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -99,6 +99,7 @@ 
 #define xfs_dir_replace			libxfs_dir_replace
 
 #define xfs_dqblk_repair		libxfs_dqblk_repair
+#define xfs_dquot_from_disk_ts		libxfs_dquot_from_disk_ts
 #define xfs_dquot_verify		libxfs_dquot_verify
 
 #define xfs_finobt_calc_reserves	libxfs_finobt_calc_reserves