All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] xfsprogs: 4.15 rollup pt. 3
@ 2018-01-25 23:48 Darrick J. Wong
  2018-01-25 23:48 ` [PATCH 1/2] xfs_db: print transaction reservation type information Darrick J. Wong
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-01-25 23:48 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

Hi all,

Here are two more patches (outside of the xfs_scrub series) to enhance
xfs_db.  The first patch adds a command to print the current transaction
reservation sizes so that we can more easily debug minimum log size
problems.  The second patch changes xfs_db's interpretation of the t_sec
field in inodes to be signed, since the kernel will feed us signed int32
values -- otherwise, you utimensat() a file with a date of 1 Jan 1950
and xfs_db displays that as April 2086.

--D

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] xfs_db: print transaction reservation type information
  2018-01-25 23:48 [PATCH 0/2] xfsprogs: 4.15 rollup pt. 3 Darrick J. Wong
@ 2018-01-25 23:48 ` Darrick J. Wong
  2018-01-25 23:48 ` [PATCH 2/2] xfs_db: interpret inode timestamps as signed integers Darrick J. Wong
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-01-25 23:48 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a new xfs_db command to print the transaction reservation info for
a given filesystem.  This will make it easier to compare the calculations
made by the kernel and xfsprogs in case there is a discrepancy.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/command.c             |    1 +
 db/logformat.c           |   59 ++++++++++++++++++++++++++++++++++++++++++++++
 db/logformat.h           |    1 +
 libxfs/libxfs_api_defs.h |    2 +-
 4 files changed, 62 insertions(+), 1 deletion(-)


diff --git a/db/command.c b/db/command.c
index 5ff3c4f..12ae5b7 100644
--- a/db/command.c
+++ b/db/command.c
@@ -138,6 +138,7 @@ init_commands(void)
 	hash_init();
 	inode_init();
 	input_init();
+	logres_init();
 	logformat_init();
 	io_init();
 	metadump_init();
diff --git a/db/logformat.c b/db/logformat.c
index 70097bc..b290bd3 100644
--- a/db/logformat.c
+++ b/db/logformat.c
@@ -147,3 +147,62 @@ logformat_init(void)
 
 	add_command(&logformat_cmd);
 }
+
+static void
+print_logres(
+	int			i,
+	struct xfs_trans_res	*res)
+{
+	dbprintf(_("type %d logres %u logcount %d flags 0x%x\n"),
+		i, res->tr_logres, res->tr_logcount, res->tr_logflags);
+}
+
+int
+logres_f(
+	int			argc,
+	char			**argv)
+{
+	struct xfs_trans_res	resv;
+	struct xfs_trans_res	*res;
+	struct xfs_trans_res	*end_res;
+	int			i;
+
+	res = (struct xfs_trans_res *)M_RES(mp);
+	end_res = (struct xfs_trans_res *)(M_RES(mp) + 1);
+	for (i = 0; res < end_res; i++, res++)
+		print_logres(i, res);
+	libxfs_log_get_max_trans_res(mp, &resv);
+	print_logres(-1, &resv);
+
+	return 0;
+}
+
+static void
+logres_help(void)
+{
+	dbprintf(_(
+"\n"
+" The 'logres' command prints information about all log reservation types.\n"
+" This includes the reservation space, the intended transaction roll count,\n"
+" and the reservation flags, if any.\n"
+"\n"
+	));
+}
+
+static const struct cmdinfo logres_cmd = {
+	.name =		"logres",
+	.altname =	NULL,
+	.cfunc =	logres_f,
+	.argmin =	0,
+	.argmax =	0,
+	.canpush =	0,
+	.args =		NULL,
+	.oneline =	N_("dump log reservations"),
+	.help =		logres_help,
+};
+
+void
+logres_init(void)
+{
+	add_command(&logres_cmd);
+}
diff --git a/db/logformat.h b/db/logformat.h
index f9763ee..60396c0 100644
--- a/db/logformat.h
+++ b/db/logformat.h
@@ -17,3 +17,4 @@
  */
 
 void logformat_init(void);
+void logres_init(void);
diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index d2ab02a..5d56340 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -60,7 +60,7 @@
 #define xfs_trans_roll			libxfs_trans_roll
 #define xfs_trans_get_buf_map		libxfs_trans_get_buf_map
 #define xfs_trans_resv_calc		libxfs_trans_resv_calc
-
+#define xfs_log_get_max_trans_res	libxfs_log_get_max_trans_res
 #define xfs_attr_get			libxfs_attr_get
 #define xfs_attr_set			libxfs_attr_set
 #define xfs_attr_remove			libxfs_attr_remove


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] xfs_db: interpret inode timestamps as signed integers
  2018-01-25 23:48 [PATCH 0/2] xfsprogs: 4.15 rollup pt. 3 Darrick J. Wong
  2018-01-25 23:48 ` [PATCH 1/2] xfs_db: print transaction reservation type information Darrick J. Wong
@ 2018-01-25 23:48 ` Darrick J. Wong
  2018-01-30 16:46   ` Eric Sandeen
  2018-01-26 18:14 ` [PATCH 3/2] misc: ubsan fixes Darrick J. Wong
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2018-01-25 23:48 UTC (permalink / raw)
  To: sandeen, darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

The Linux kernel treats core.*time.sec as a signed integer value, so
xfs_db should do likewise, or else files will have inconsistent times
if the seconds count is negative.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/fprint.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


diff --git a/db/fprint.c b/db/fprint.c
index 261a31e..e8eb510 100644
--- a/db/fprint.c
+++ b/db/fprint.c
@@ -147,7 +147,8 @@ fp_time(
 	     i++, bitpos += size) {
 		if (array)
 			dbprintf("%d:", i + base);
-		t=(time_t)getbitval((char *)obj + byteize(bitpos), 0, sizeof(int32_t)*8, 0);
+		t = (time_t)getbitval((char *)obj + byteize(bitpos), 0,
+				sizeof(int32_t) * 8, BVSIGNED);
 		c = ctime(&t);
 		dbprintf("%24.24s", c);
 		if (i < count - 1)


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/2] misc: ubsan fixes
  2018-01-25 23:48 [PATCH 0/2] xfsprogs: 4.15 rollup pt. 3 Darrick J. Wong
  2018-01-25 23:48 ` [PATCH 1/2] xfs_db: print transaction reservation type information Darrick J. Wong
  2018-01-25 23:48 ` [PATCH 2/2] xfs_db: interpret inode timestamps as signed integers Darrick J. Wong
@ 2018-01-26 18:14 ` Darrick J. Wong
  2018-01-30 16:47   ` Eric Sandeen
  2018-01-30  3:38 ` [PATCH 4/2] mkfs: don't crash on dswidth overflow Darrick J. Wong
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2018-01-26 18:14 UTC (permalink / raw)
  To: sandeen; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Fix a few things the undefined behavior sanitizer complained about.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/bit.c    |    4 ++--
 repair/sb.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/db/bit.c b/db/bit.c
index a20b6ba..bf8d80e 100644
--- a/db/bit.c
+++ b/db/bit.c
@@ -112,11 +112,11 @@ getbitval(
 #if __BYTE_ORDER == LITTLE_ENDIAN
 			if (i == 0 && signext && nbits < 64)
 				rval = -1LL << nbits;
-			rval |= 1LL << (nbits - i - 1);
+			rval |= 1ULL << (nbits - i - 1);
 #else
 			if ((i == (nbits - 1)) && signext && nbits < 64)
 				rval |= (-1LL << nbits);
-			rval |= 1LL << (nbits - i - 1);
+			rval |= 1ULL << (nbits - i - 1);
 #endif
 		}
 	}
diff --git a/repair/sb.c b/repair/sb.c
index f40cdea..3dc6538 100644
--- a/repair/sb.c
+++ b/repair/sb.c
@@ -89,11 +89,11 @@ verify_sb_blocksize(xfs_sb_t *sb)
 	/* check to make sure blocksize is legal 2^N, 9 <= N <= 16 */
 	if (sb->sb_blocksize == 0)
 		return XR_BAD_BLOCKSIZE;
-	if (sb->sb_blocksize != (1 << sb->sb_blocklog))
-		return XR_BAD_BLOCKLOG;
 	if (sb->sb_blocklog < XFS_MIN_BLOCKSIZE_LOG ||
 	    sb->sb_blocklog > XFS_MAX_BLOCKSIZE_LOG)
 		return XR_BAD_BLOCKLOG;
+	if (sb->sb_blocksize != (1 << sb->sb_blocklog))
+		return XR_BAD_BLOCKLOG;
 
 	return 0;
 }

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/2] mkfs: don't crash on dswidth overflow
  2018-01-25 23:48 [PATCH 0/2] xfsprogs: 4.15 rollup pt. 3 Darrick J. Wong
                   ` (2 preceding siblings ...)
  2018-01-26 18:14 ` [PATCH 3/2] misc: ubsan fixes Darrick J. Wong
@ 2018-01-30  3:38 ` Darrick J. Wong
  2018-01-30  3:54   ` Eric Sandeen
  2018-01-30  3:38 ` [PATCH 5/2] mkfs: don't call values 'illegal', they're invalid Darrick J. Wong
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2018-01-30  3:38 UTC (permalink / raw)
  To: sandeen; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

I ran mkfs.xfs -d su=1048576,sw=$((18 * 1048576)), forgetting that sw
takes a multiple of su (unlike swidth which takes any space unit).  I
was surprised when we hit a floating point exception, which I traced
back to an integer overflow when we calculate swidth from dsw.

So, do the 64-bit multiplication so we can detect the overflow and
complain about it.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 mkfs/xfs_mkfs.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 5f1ac9f..7c9d148 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -2211,6 +2211,7 @@ calc_stripe_factors(
 	struct cli_params	*cli,
 	struct fs_topology	*ft)
 {
+	long long int	big_dswidth;
 	int		dsunit = 0;
 	int		dswidth = 0;
 	int		lsunit = 0;
@@ -2251,7 +2252,14 @@ _("data su must be a multiple of the sector size (%d)\n"), cfg->sectorsize);
 		}
 
 		dsunit  = (int)BTOBBT(dsu);
-		dswidth = dsunit * dsw;
+		big_dswidth = (long long int)dsunit * dsw;
+		if (big_dswidth > INT_MAX) {
+			fprintf(stderr,
+_("data stripe width (%lld) is too large of a multiple of the data stripe unit (%d)\n"),
+				big_dswidth, dsunit);
+			usage();
+		}
+		dswidth = big_dswidth;
 	}
 
 	if (dsunit && (dswidth % dsunit != 0)) {

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/2] mkfs: don't call values 'illegal', they're invalid
  2018-01-25 23:48 [PATCH 0/2] xfsprogs: 4.15 rollup pt. 3 Darrick J. Wong
                   ` (3 preceding siblings ...)
  2018-01-30  3:38 ` [PATCH 4/2] mkfs: don't crash on dswidth overflow Darrick J. Wong
@ 2018-01-30  3:38 ` Darrick J. Wong
  2018-01-30  3:55   ` Eric Sandeen
  2018-01-30  3:39 ` [PATCH 6/2] mkfs: always explain why numeric inputs are invalid Darrick J. Wong
  2018-01-30  4:13 ` [PATCH 7/2] mkfs: more sunit/swidth sanity checking Eric Sandeen
  6 siblings, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2018-01-30  3:38 UTC (permalink / raw)
  To: sandeen; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Specifying invalid inputs to mkfs does not break any (reasonable) laws,
so the error message should complain about invalid inputs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 mkfs/xfs_mkfs.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 7c9d148..6285b24 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -911,7 +911,7 @@ illegal(
 	const char	*value,
 	const char	*opt)
 {
-	fprintf(stderr, _("Illegal value %s for -%s option\n"), value, opt);
+	fprintf(stderr, _("Invalid value %s for -%s option\n"), value, opt);
 	usage();
 }
 
@@ -1267,7 +1267,7 @@ illegal_option(
 	const char		*reason)
 {
 	fprintf(stderr,
-		_("Illegal value %s for -%c %s option. %s\n"),
+		_("Invalid value %s for -%c %s option. %s\n"),
 		value, opts->name, opts->subopts[index],
 		reason ? reason : "");
 	usage();

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/2] mkfs: always explain why numeric inputs are invalid
  2018-01-25 23:48 [PATCH 0/2] xfsprogs: 4.15 rollup pt. 3 Darrick J. Wong
                   ` (4 preceding siblings ...)
  2018-01-30  3:38 ` [PATCH 5/2] mkfs: don't call values 'illegal', they're invalid Darrick J. Wong
@ 2018-01-30  3:39 ` Darrick J. Wong
  2018-01-30  3:57   ` Eric Sandeen
  2018-01-30  4:13 ` [PATCH 7/2] mkfs: more sunit/swidth sanity checking Eric Sandeen
  6 siblings, 1 reply; 14+ messages in thread
From: Darrick J. Wong @ 2018-01-30  3:39 UTC (permalink / raw)
  To: sandeen; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Always explain why invalid numeric inputs are not valid, and in a
complete sentence since that's what illegal_optio() sets us up for.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 mkfs/xfs_mkfs.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 6285b24..f527476 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -1269,7 +1269,7 @@ illegal_option(
 	fprintf(stderr,
 		_("Invalid value %s for -%c %s option. %s\n"),
 		value, opts->name, opts->subopts[index],
-		reason ? reason : "");
+		reason);
 	usage();
 }
 
@@ -1360,18 +1360,20 @@ getnum(
 
 		c = strtoll(str, &str_end, 0);
 		if (c == 0 && str_end == str)
-			illegal_option(str, opts, index, NULL);
+			illegal_option(str, opts, index,
+					_("Value not recognized as number."));
 		if (*str_end != '\0')
-			illegal_option(str, opts, index, NULL);
+			illegal_option(str, opts, index,
+					_("Unit suffixes are not allowed."));
 	}
 
 	/* Validity check the result. */
 	if (c < sp->minval)
-		illegal_option(str, opts, index, _("value is too small"));
+		illegal_option(str, opts, index, _("Value is too small."));
 	else if (c > sp->maxval)
-		illegal_option(str, opts, index, _("value is too large"));
+		illegal_option(str, opts, index, _("Value is too large."));
 	if (sp->is_power_2 && !ispow2(c))
-		illegal_option(str, opts, index, _("value must be a power of 2"));
+		illegal_option(str, opts, index, _("Value must be a power of 2."));
 	return c;
 }
 

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/2] mkfs: don't crash on dswidth overflow
  2018-01-30  3:38 ` [PATCH 4/2] mkfs: don't crash on dswidth overflow Darrick J. Wong
@ 2018-01-30  3:54   ` Eric Sandeen
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Sandeen @ 2018-01-30  3:54 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On 1/29/18 9:38 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> I ran mkfs.xfs -d su=1048576,sw=$((18 * 1048576)), forgetting that sw
> takes a multiple of su (unlike swidth which takes any space unit).  I
> was surprised when we hit a floating point exception, which I traced
> back to an integer overflow when we calculate swidth from dsw.
> 
> So, do the 64-bit multiplication so we can detect the overflow and
> complain about it.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks fine (if not a bit of a mouthful) ;)

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  mkfs/xfs_mkfs.c |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 5f1ac9f..7c9d148 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -2211,6 +2211,7 @@ calc_stripe_factors(
>  	struct cli_params	*cli,
>  	struct fs_topology	*ft)
>  {
> +	long long int	big_dswidth;
>  	int		dsunit = 0;
>  	int		dswidth = 0;
>  	int		lsunit = 0;
> @@ -2251,7 +2252,14 @@ _("data su must be a multiple of the sector size (%d)\n"), cfg->sectorsize);
>  		}
>  
>  		dsunit  = (int)BTOBBT(dsu);
> -		dswidth = dsunit * dsw;
> +		big_dswidth = (long long int)dsunit * dsw;
> +		if (big_dswidth > INT_MAX) {
> +			fprintf(stderr,
> +_("data stripe width (%lld) is too large of a multiple of the data stripe unit (%d)\n"),
> +				big_dswidth, dsunit);
> +			usage();
> +		}
> +		dswidth = big_dswidth;
>  	}
>  
>  	if (dsunit && (dswidth % dsunit != 0)) {
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/2] mkfs: don't call values 'illegal', they're invalid
  2018-01-30  3:38 ` [PATCH 5/2] mkfs: don't call values 'illegal', they're invalid Darrick J. Wong
@ 2018-01-30  3:55   ` Eric Sandeen
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Sandeen @ 2018-01-30  3:55 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On 1/29/18 9:38 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Specifying invalid inputs to mkfs does not break any (reasonable) laws,
> so the error message should complain about invalid inputs.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Maybe not in your state ...

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  mkfs/xfs_mkfs.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 7c9d148..6285b24 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -911,7 +911,7 @@ illegal(
>  	const char	*value,
>  	const char	*opt)
>  {
> -	fprintf(stderr, _("Illegal value %s for -%s option\n"), value, opt);
> +	fprintf(stderr, _("Invalid value %s for -%s option\n"), value, opt);
>  	usage();
>  }
>  
> @@ -1267,7 +1267,7 @@ illegal_option(
>  	const char		*reason)
>  {
>  	fprintf(stderr,
> -		_("Illegal value %s for -%c %s option. %s\n"),
> +		_("Invalid value %s for -%c %s option. %s\n"),
>  		value, opts->name, opts->subopts[index],
>  		reason ? reason : "");
>  	usage();
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/2] mkfs: always explain why numeric inputs are invalid
  2018-01-30  3:39 ` [PATCH 6/2] mkfs: always explain why numeric inputs are invalid Darrick J. Wong
@ 2018-01-30  3:57   ` Eric Sandeen
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Sandeen @ 2018-01-30  3:57 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On 1/29/18 9:39 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Always explain why invalid numeric inputs are not valid, and in a
> complete sentence since that's what illegal_optio() sets us up for.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 7/2] mkfs: more sunit/swidth sanity checking
  2018-01-25 23:48 [PATCH 0/2] xfsprogs: 4.15 rollup pt. 3 Darrick J. Wong
                   ` (5 preceding siblings ...)
  2018-01-30  3:39 ` [PATCH 6/2] mkfs: always explain why numeric inputs are invalid Darrick J. Wong
@ 2018-01-30  4:13 ` Eric Sandeen
  2018-01-30 16:48   ` Darrick J. Wong
  6 siblings, 1 reply; 14+ messages in thread
From: Eric Sandeen @ 2018-01-30  4:13 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs

This fixes 2 issues with stripe geometry validation.

# mkfs.xfs -d sunit=64,swidth=0 ...
both data sunit and data swidth options must be specified

But I did specify it, I specified 0!

So use cli_opt_set() to detect that it was specified.

But we can't allow the above configuration (in fact it causes
a % 0 later in mkfs), so catch it in the "swidth must be a
multiple of sunit" test a bit further down.

(sunit=0,swidth=0 /is/ valid, it's used to override disk
geometry if desired.)

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

don't mind my mkfs patch series hijacking! ;)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index f527476..219b209 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -2233,7 +2233,7 @@ calc_stripe_factors(
 		dsw = cli->dsw;
 
 	/* data sunit/swidth options */
-	if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
+	if (cli_opt_set(&dopts, D_SUNIT) != cli_opt_set(&dopts, D_SWIDTH)) {
 		fprintf(stderr,
 _("both data sunit and data swidth options must be specified\n"));
 		usage();
@@ -2241,7 +2241,7 @@ _("both data sunit and data swidth options must be specified\n"));
 
 	/* convert dsu/dsw to dsunit/dswidth and use them from now on */
 	if (dsu || dsw) {
-		if ((dsu && !dsw) || (!dsu && dsw)) {
+		if (cli_opt_set(&dopts, D_SU) != cli_opt_set(&dopts, D_SW)) {
 			fprintf(stderr,
 _("both data su and data sw options must be specified\n"));
 			usage();
@@ -2264,7 +2264,7 @@ _("data stripe width (%lld) is too large of a multiple of the data stripe unit (
 		dswidth = big_dswidth;
 	}
 
-	if (dsunit && (dswidth % dsunit != 0)) {
+	if (dsunit && (!dswidth || (dswidth % dsunit != 0))) {
 		fprintf(stderr,
 _("data stripe width (%d) must be a multiple of the data stripe unit (%d)\n"),
 			dswidth, dsunit);


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] xfs_db: interpret inode timestamps as signed integers
  2018-01-25 23:48 ` [PATCH 2/2] xfs_db: interpret inode timestamps as signed integers Darrick J. Wong
@ 2018-01-30 16:46   ` Eric Sandeen
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Sandeen @ 2018-01-30 16:46 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs

On 1/25/18 5:48 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The Linux kernel treats core.*time.sec as a signed integer value, so
> xfs_db should do likewise, or else files will have inconsistent times
> if the seconds count is negative.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  db/fprint.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/db/fprint.c b/db/fprint.c
> index 261a31e..e8eb510 100644
> --- a/db/fprint.c
> +++ b/db/fprint.c
> @@ -147,7 +147,8 @@ fp_time(
>  	     i++, bitpos += size) {
>  		if (array)
>  			dbprintf("%d:", i + base);
> -		t=(time_t)getbitval((char *)obj + byteize(bitpos), 0, sizeof(int32_t)*8, 0);
> +		t = (time_t)getbitval((char *)obj + byteize(bitpos), 0,
> +				sizeof(int32_t) * 8, BVSIGNED);
>  		c = ctime(&t);
>  		dbprintf("%24.24s", c);
>  		if (i < count - 1)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/2] misc: ubsan fixes
  2018-01-26 18:14 ` [PATCH 3/2] misc: ubsan fixes Darrick J. Wong
@ 2018-01-30 16:47   ` Eric Sandeen
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Sandeen @ 2018-01-30 16:47 UTC (permalink / raw)
  To: Darrick J. Wong, sandeen; +Cc: linux-xfs



On 1/26/18 12:14 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Fix a few things the undefined behavior sanitizer complained about.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>


Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---

>  db/bit.c    |    4 ++--
>  repair/sb.c |    4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/db/bit.c b/db/bit.c
> index a20b6ba..bf8d80e 100644
> --- a/db/bit.c
> +++ b/db/bit.c
> @@ -112,11 +112,11 @@ getbitval(
>  #if __BYTE_ORDER == LITTLE_ENDIAN
>  			if (i == 0 && signext && nbits < 64)
>  				rval = -1LL << nbits;
> -			rval |= 1LL << (nbits - i - 1);
> +			rval |= 1ULL << (nbits - i - 1);
>  #else
>  			if ((i == (nbits - 1)) && signext && nbits < 64)
>  				rval |= (-1LL << nbits);
> -			rval |= 1LL << (nbits - i - 1);
> +			rval |= 1ULL << (nbits - i - 1);
>  #endif
>  		}
>  	}
> diff --git a/repair/sb.c b/repair/sb.c
> index f40cdea..3dc6538 100644
> --- a/repair/sb.c
> +++ b/repair/sb.c
> @@ -89,11 +89,11 @@ verify_sb_blocksize(xfs_sb_t *sb)
>  	/* check to make sure blocksize is legal 2^N, 9 <= N <= 16 */
>  	if (sb->sb_blocksize == 0)
>  		return XR_BAD_BLOCKSIZE;
> -	if (sb->sb_blocksize != (1 << sb->sb_blocklog))
> -		return XR_BAD_BLOCKLOG;
>  	if (sb->sb_blocklog < XFS_MIN_BLOCKSIZE_LOG ||
>  	    sb->sb_blocklog > XFS_MAX_BLOCKSIZE_LOG)
>  		return XR_BAD_BLOCKLOG;
> +	if (sb->sb_blocksize != (1 << sb->sb_blocklog))
> +		return XR_BAD_BLOCKLOG;
>  
>  	return 0;
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 7/2] mkfs: more sunit/swidth sanity checking
  2018-01-30  4:13 ` [PATCH 7/2] mkfs: more sunit/swidth sanity checking Eric Sandeen
@ 2018-01-30 16:48   ` Darrick J. Wong
  0 siblings, 0 replies; 14+ messages in thread
From: Darrick J. Wong @ 2018-01-30 16:48 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: sandeen, linux-xfs

On Mon, Jan 29, 2018 at 10:13:58PM -0600, Eric Sandeen wrote:
> This fixes 2 issues with stripe geometry validation.
> 
> # mkfs.xfs -d sunit=64,swidth=0 ...
> both data sunit and data swidth options must be specified
> 
> But I did specify it, I specified 0!
> 
> So use cli_opt_set() to detect that it was specified.
> 
> But we can't allow the above configuration (in fact it causes
> a % 0 later in mkfs), so catch it in the "swidth must be a
> multiple of sunit" test a bit further down.
> 
> (sunit=0,swidth=0 /is/ valid, it's used to override disk
> geometry if desired.)
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
> 
> don't mind my mkfs patch series hijacking! ;)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index f527476..219b209 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -2233,7 +2233,7 @@ calc_stripe_factors(
>  		dsw = cli->dsw;
>  
>  	/* data sunit/swidth options */
> -	if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
> +	if (cli_opt_set(&dopts, D_SUNIT) != cli_opt_set(&dopts, D_SWIDTH)) {
>  		fprintf(stderr,
>  _("both data sunit and data swidth options must be specified\n"));
>  		usage();
> @@ -2241,7 +2241,7 @@ _("both data sunit and data swidth options must be specified\n"));
>  
>  	/* convert dsu/dsw to dsunit/dswidth and use them from now on */
>  	if (dsu || dsw) {
> -		if ((dsu && !dsw) || (!dsu && dsw)) {
> +		if (cli_opt_set(&dopts, D_SU) != cli_opt_set(&dopts, D_SW)) {
>  			fprintf(stderr,
>  _("both data su and data sw options must be specified\n"));
>  			usage();
> @@ -2264,7 +2264,7 @@ _("data stripe width (%lld) is too large of a multiple of the data stripe unit (
>  		dswidth = big_dswidth;
>  	}
>  
> -	if (dsunit && (dswidth % dsunit != 0)) {
> +	if (dsunit && (!dswidth || (dswidth % dsunit != 0))) {
>  		fprintf(stderr,
>  _("data stripe width (%d) must be a multiple of the data stripe unit (%d)\n"),
>  			dswidth, dsunit);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-01-30 16:48 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-25 23:48 [PATCH 0/2] xfsprogs: 4.15 rollup pt. 3 Darrick J. Wong
2018-01-25 23:48 ` [PATCH 1/2] xfs_db: print transaction reservation type information Darrick J. Wong
2018-01-25 23:48 ` [PATCH 2/2] xfs_db: interpret inode timestamps as signed integers Darrick J. Wong
2018-01-30 16:46   ` Eric Sandeen
2018-01-26 18:14 ` [PATCH 3/2] misc: ubsan fixes Darrick J. Wong
2018-01-30 16:47   ` Eric Sandeen
2018-01-30  3:38 ` [PATCH 4/2] mkfs: don't crash on dswidth overflow Darrick J. Wong
2018-01-30  3:54   ` Eric Sandeen
2018-01-30  3:38 ` [PATCH 5/2] mkfs: don't call values 'illegal', they're invalid Darrick J. Wong
2018-01-30  3:55   ` Eric Sandeen
2018-01-30  3:39 ` [PATCH 6/2] mkfs: always explain why numeric inputs are invalid Darrick J. Wong
2018-01-30  3:57   ` Eric Sandeen
2018-01-30  4:13 ` [PATCH 7/2] mkfs: more sunit/swidth sanity checking Eric Sandeen
2018-01-30 16:48   ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.