All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] xfsprogs: xfs_logprint misc log decoding issues
@ 2021-01-28  7:37 Donald Douwsma
  2021-01-28  7:37 ` [PATCH 1/2] xfs_logprint: print misc buffers when using -o Donald Douwsma
  2021-01-28  7:37 ` [PATCH 2/2] xfs_logprint: decode superblock updates correctly Donald Douwsma
  0 siblings, 2 replies; 7+ messages in thread
From: Donald Douwsma @ 2021-01-28  7:37 UTC (permalink / raw)
  To: linux-xfs; +Cc: Donald Douwsma

I've been seeing confusing superblock transactions in logprint, this is
an attempt to resolve them and make debugging misc buffer updates easier.

Before

$ xfs_logprint -o xfs.image
----------------------------------------------------------------------------
Oper (2): tid: c32589f8  len: 24  clientid: TRANS  flags: none
BUF:  #regs: 2   start blkno: 0 (0x0)  len: 1  bmap size: 1  flags: 0x9000
Oper (3): tid: c32589f8  len: 384  clientid: TRANS  flags: none
SUPER BLOCK Buffer:
icount: 6360863066640355328  ifree: 262144  fdblks: 0  frext: 0
----------------------------------------------------------------------------

After

$ xfs_logprint -o xfs.image
----------------------------------------------------------------------------
Oper (2): tid: c32589f8  len: 24  clientid: TRANS  flags: none
BUF:  #regs: 2   start blkno: 0 (0x0)  len: 1  bmap size: 1  flags: 0x9000
Oper (3): tid: c32589f8  len: 384  clientid: TRANS  flags: none
SUPER BLOCK Buffer:
icount: 64  ifree: 61  fdblks: 259564  frext: 0
 0 42534658   100000        0      400        0        0        0        0
 8 84a2ced5 974f8da4 8fd4b0b3 9a870a19        0  4000200        0 80000000
10        0 81000000        0 82000000  1000000      100  4000000        0
18    a0000    2a4b4 10000001        0        0        0  408090c 19000010
20        0 40000000        0 3d000000        0 ecf50300        0        0
28 ffffffff ffffffff ffffffff ffffffff        0  2000000        0        0
30        0  1000000 8a020000 8a020000        0        0        0        0
38        0        0        0        0        0        0        0        0
40        0        0        0        0        0        0        0        0
48        0        0        0        0        0        0        0        0
50        0        0        0        0        0        0        0        0
58        0        0        0        0        0        0        0        0

----------------------------------------------------------------------------

Donald Douwsma (2):
  xfs_logprint: print misc buffers when using -o
  xfs_logprint: decode superblock updates correctly

 logprint/log_misc.c      | 41 ++++++++++++----------------------------
 logprint/log_print_all.c | 25 +++++++++++-------------
 2 files changed, 23 insertions(+), 43 deletions(-)

-- 
2.27.0


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

* [PATCH 1/2] xfs_logprint: print misc buffers when using -o
  2021-01-28  7:37 [PATCH 0/2] xfsprogs: xfs_logprint misc log decoding issues Donald Douwsma
@ 2021-01-28  7:37 ` Donald Douwsma
  2021-01-28 17:35   ` Darrick J. Wong
  2021-01-28  7:37 ` [PATCH 2/2] xfs_logprint: decode superblock updates correctly Donald Douwsma
  1 sibling, 1 reply; 7+ messages in thread
From: Donald Douwsma @ 2021-01-28  7:37 UTC (permalink / raw)
  To: linux-xfs; +Cc: Donald Douwsma

Logprint only dumps raw buffers for unhandled misc buffer types, but
this information is generally useful when debugging logprint issues so
allow it to print whenever -o is used.

Switch to using the common xlog_print_data function to dump the buffer.

Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
---
 logprint/log_misc.c      | 19 +++----------------
 logprint/log_print_all.c |  2 +-
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index c325f046..d44e9ff7 100644
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -392,23 +392,10 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
 		}
 	} else {
 		printf(_("BUF DATA\n"));
-		if (print_data) {
-			uint *dp  = (uint *)*ptr;
-			int  nums = be32_to_cpu(head->oh_len) >> 2;
-			int  byte = 0;
-
-			while (byte < nums) {
-				if ((byte % 8) == 0)
-					printf("%2x ", byte);
-				printf("%8x ", *dp);
-				dp++;
-				byte++;
-				if ((byte % 8) == 0)
-					printf("\n");
-			}
-			printf("\n");
-		}
 	}
+
+	xlog_recover_print_data(*ptr, be32_to_cpu(head->oh_len));
+
 	*ptr += be32_to_cpu(head->oh_len);
     }
     if (head && head->oh_flags & XLOG_CONTINUE_TRANS)
diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
index eafffe28..2b9e810d 100644
--- a/logprint/log_print_all.c
+++ b/logprint/log_print_all.c
@@ -176,8 +176,8 @@ xlog_recover_print_buffer(
 		} else {
 			printf(_("	BUF DATA\n"));
 			if (!print_buffer) continue;
-			xlog_recover_print_data(p, len);
 		}
+		xlog_recover_print_data(p, len);
 	}
 }
 
-- 
2.27.0


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

* [PATCH 2/2] xfs_logprint: decode superblock updates correctly
  2021-01-28  7:37 [PATCH 0/2] xfsprogs: xfs_logprint misc log decoding issues Donald Douwsma
  2021-01-28  7:37 ` [PATCH 1/2] xfs_logprint: print misc buffers when using -o Donald Douwsma
@ 2021-01-28  7:37 ` Donald Douwsma
  2021-01-28 17:28   ` Darrick J. Wong
  1 sibling, 1 reply; 7+ messages in thread
From: Donald Douwsma @ 2021-01-28  7:37 UTC (permalink / raw)
  To: linux-xfs; +Cc: Donald Douwsma

Back when the way superblocks are logged changed, logprint wasnt
updated updated. Currently logprint displays incorrect accounting
information.

 SUPER BLOCK Buffer:
 icount: 6360863066640355328  ifree: 262144  fdblks: 0  frext: 0

 $ printf "0x%x\n" 6360863066640355328
 0x5846534200001000

Part of this decodes as 'XFSB', the xfs superblock magic number and not
the free space accounting.

Fix this by looking at the entire superblock buffer and using the format
structure as is done for the other allocation group headers.

Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
---
 logprint/log_misc.c      | 22 +++++++++-------------
 logprint/log_print_all.c | 23 ++++++++++-------------
 2 files changed, 19 insertions(+), 26 deletions(-)

diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index d44e9ff7..929842d0 100644
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -243,25 +243,21 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
 	xlog_print_op_header(head, *i, ptr);
 	if (super_block) {
 		printf(_("SUPER BLOCK Buffer: "));
-		if (be32_to_cpu(head->oh_len) < 4*8) {
+		if (be32_to_cpu(head->oh_len) < sizeof(struct xfs_sb)) {
 			printf(_("Out of space\n"));
 		} else {
-			__be64		 a, b;
+			struct xfs_sb *sb, sb_s;
 
 			printf("\n");
-			/*
-			 * memmove because *ptr may not be 8-byte aligned
-			 */
-			memmove(&a, *ptr, sizeof(__be64));
-			memmove(&b, *ptr+8, sizeof(__be64));
+			/* memmove because *ptr may not be 8-byte aligned */
+			sb = &sb_s;
+			memmove(sb, *ptr, sizeof(struct xfs_sb));
 			printf(_("icount: %llu  ifree: %llu  "),
-			       (unsigned long long) be64_to_cpu(a),
-			       (unsigned long long) be64_to_cpu(b));
-			memmove(&a, *ptr+16, sizeof(__be64));
-			memmove(&b, *ptr+24, sizeof(__be64));
+				be64_to_cpu(sb->sb_icount),
+				be64_to_cpu(sb->sb_ifree) );
 			printf(_("fdblks: %llu  frext: %llu\n"),
-			       (unsigned long long) be64_to_cpu(a),
-			       (unsigned long long) be64_to_cpu(b));
+				be64_to_cpu(sb->sb_fdblocks),
+				be64_to_cpu(sb->sb_frextents));
 		}
 		super_block = 0;
 	} else if (be32_to_cpu(*(__be32 *)(*ptr)) == XFS_AGI_MAGIC) {
diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
index 2b9e810d..8ff87068 100644
--- a/logprint/log_print_all.c
+++ b/logprint/log_print_all.c
@@ -91,22 +91,19 @@ xlog_recover_print_buffer(
 		len = item->ri_buf[i].i_len;
 		i++;
 		if (blkno == 0) { /* super block */
-			printf(_("	SUPER Block Buffer:\n"));
+                        struct xfs_sb *sb = (struct xfs_sb *)p;
+			printf(_("	Super Block Buffer: (XFSB)\n"));
 			if (!print_buffer)
 				continue;
-		       printf(_("              icount:%llu ifree:%llu  "),
-			       (unsigned long long)
-				       be64_to_cpu(*(__be64 *)(p)),
-			       (unsigned long long)
-				       be64_to_cpu(*(__be64 *)(p+8)));
-		       printf(_("fdblks:%llu  frext:%llu\n"),
-			       (unsigned long long)
-				       be64_to_cpu(*(__be64 *)(p+16)),
-			       (unsigned long long)
-				       be64_to_cpu(*(__be64 *)(p+24)));
+			printf(_("		icount:%llu  ifree:%llu  "),
+					be64_to_cpu(sb->sb_icount),
+					be64_to_cpu(sb->sb_ifree));
+			printf(_("fdblks:%llu  frext:%llu\n"),
+					be64_to_cpu(sb->sb_fdblocks),
+					be64_to_cpu(sb->sb_frextents));
 			printf(_("		sunit:%u  swidth:%u\n"),
-			       be32_to_cpu(*(__be32 *)(p+56)),
-			       be32_to_cpu(*(__be32 *)(p+60)));
+			       be32_to_cpu(sb->sb_unit),
+			       be32_to_cpu(sb->sb_width));
 		} else if (be32_to_cpu(*(__be32 *)p) == XFS_AGI_MAGIC) {
 			int bucket, buckets;
 			agi = (xfs_agi_t *)p;
-- 
2.27.0


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

* Re: [PATCH 2/2] xfs_logprint: decode superblock updates correctly
  2021-01-28  7:37 ` [PATCH 2/2] xfs_logprint: decode superblock updates correctly Donald Douwsma
@ 2021-01-28 17:28   ` Darrick J. Wong
  2021-01-28 21:59     ` Donald Douwsma
  0 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2021-01-28 17:28 UTC (permalink / raw)
  To: Donald Douwsma; +Cc: linux-xfs

On Thu, Jan 28, 2021 at 06:37:08PM +1100, Donald Douwsma wrote:
> Back when the way superblocks are logged changed, logprint wasnt
> updated updated. Currently logprint displays incorrect accounting

Double 'updated'.

> information.
> 
>  SUPER BLOCK Buffer:
>  icount: 6360863066640355328  ifree: 262144  fdblks: 0  frext: 0
> 
>  $ printf "0x%x\n" 6360863066640355328
>  0x5846534200001000
> 
> Part of this decodes as 'XFSB', the xfs superblock magic number and not
> the free space accounting.
> 
> Fix this by looking at the entire superblock buffer and using the format
> structure as is done for the other allocation group headers.
> 
> Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
> ---
>  logprint/log_misc.c      | 22 +++++++++-------------
>  logprint/log_print_all.c | 23 ++++++++++-------------
>  2 files changed, 19 insertions(+), 26 deletions(-)
> 
> diff --git a/logprint/log_misc.c b/logprint/log_misc.c
> index d44e9ff7..929842d0 100644
> --- a/logprint/log_misc.c
> +++ b/logprint/log_misc.c
> @@ -243,25 +243,21 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
>  	xlog_print_op_header(head, *i, ptr);
>  	if (super_block) {
>  		printf(_("SUPER BLOCK Buffer: "));
> -		if (be32_to_cpu(head->oh_len) < 4*8) {
> +		if (be32_to_cpu(head->oh_len) < sizeof(struct xfs_sb)) {
>  			printf(_("Out of space\n"));
>  		} else {
> -			__be64		 a, b;
> +			struct xfs_sb *sb, sb_s;
>  
>  			printf("\n");
> -			/*
> -			 * memmove because *ptr may not be 8-byte aligned
> -			 */
> -			memmove(&a, *ptr, sizeof(__be64));
> -			memmove(&b, *ptr+8, sizeof(__be64));
> +			/* memmove because *ptr may not be 8-byte aligned */
> +			sb = &sb_s;
> +			memmove(sb, *ptr, sizeof(struct xfs_sb));
>  			printf(_("icount: %llu  ifree: %llu  "),
> -			       (unsigned long long) be64_to_cpu(a),
> -			       (unsigned long long) be64_to_cpu(b));
> -			memmove(&a, *ptr+16, sizeof(__be64));
> -			memmove(&b, *ptr+24, sizeof(__be64));
> +				be64_to_cpu(sb->sb_icount),
> +				be64_to_cpu(sb->sb_ifree) );

Extra space at the end ..................................^

>  			printf(_("fdblks: %llu  frext: %llu\n"),
> -			       (unsigned long long) be64_to_cpu(a),
> -			       (unsigned long long) be64_to_cpu(b));
> +				be64_to_cpu(sb->sb_fdblocks),
> +				be64_to_cpu(sb->sb_frextents));
>  		}
>  		super_block = 0;
>  	} else if (be32_to_cpu(*(__be32 *)(*ptr)) == XFS_AGI_MAGIC) {
> diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
> index 2b9e810d..8ff87068 100644
> --- a/logprint/log_print_all.c
> +++ b/logprint/log_print_all.c
> @@ -91,22 +91,19 @@ xlog_recover_print_buffer(
>  		len = item->ri_buf[i].i_len;
>  		i++;
>  		if (blkno == 0) { /* super block */
> -			printf(_("	SUPER Block Buffer:\n"));
> +                        struct xfs_sb *sb = (struct xfs_sb *)p;
> +			printf(_("	Super Block Buffer: (XFSB)\n"));
>  			if (!print_buffer)
>  				continue;
> -		       printf(_("              icount:%llu ifree:%llu  "),
> -			       (unsigned long long)
> -				       be64_to_cpu(*(__be64 *)(p)),
> -			       (unsigned long long)
> -				       be64_to_cpu(*(__be64 *)(p+8)));
> -		       printf(_("fdblks:%llu  frext:%llu\n"),
> -			       (unsigned long long)
> -				       be64_to_cpu(*(__be64 *)(p+16)),
> -			       (unsigned long long)
> -				       be64_to_cpu(*(__be64 *)(p+24)));
> +			printf(_("		icount:%llu  ifree:%llu  "),
> +					be64_to_cpu(sb->sb_icount),
> +					be64_to_cpu(sb->sb_ifree));
> +			printf(_("fdblks:%llu  frext:%llu\n"),
> +					be64_to_cpu(sb->sb_fdblocks),
> +					be64_to_cpu(sb->sb_frextents));
>  			printf(_("		sunit:%u  swidth:%u\n"),
> -			       be32_to_cpu(*(__be32 *)(p+56)),
> -			       be32_to_cpu(*(__be32 *)(p+60)));
> +			       be32_to_cpu(sb->sb_unit),
> +			       be32_to_cpu(sb->sb_width));

/me wonders if these nearly identical decoder routines ought to be
refactored into a common helper?

Also, what happens if logprint encounters a log from before whenever we
changed superblock encoding in the log?  When was that, anyway?

--D

>  		} else if (be32_to_cpu(*(__be32 *)p) == XFS_AGI_MAGIC) {
>  			int bucket, buckets;
>  			agi = (xfs_agi_t *)p;
> -- 
> 2.27.0
> 

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

* Re: [PATCH 1/2] xfs_logprint: print misc buffers when using -o
  2021-01-28  7:37 ` [PATCH 1/2] xfs_logprint: print misc buffers when using -o Donald Douwsma
@ 2021-01-28 17:35   ` Darrick J. Wong
  2021-01-28 21:51     ` Donald Douwsma
  0 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2021-01-28 17:35 UTC (permalink / raw)
  To: Donald Douwsma; +Cc: linux-xfs

On Thu, Jan 28, 2021 at 06:37:07PM +1100, Donald Douwsma wrote:
> Logprint only dumps raw buffers for unhandled misc buffer types, but
> this information is generally useful when debugging logprint issues so
> allow it to print whenever -o is used.
> 
> Switch to using the common xlog_print_data function to dump the buffer.
> 
> Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
> ---
>  logprint/log_misc.c      | 19 +++----------------
>  logprint/log_print_all.c |  2 +-
>  2 files changed, 4 insertions(+), 17 deletions(-)
> 
> diff --git a/logprint/log_misc.c b/logprint/log_misc.c
> index c325f046..d44e9ff7 100644
> --- a/logprint/log_misc.c
> +++ b/logprint/log_misc.c
> @@ -392,23 +392,10 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
>  		}
>  	} else {
>  		printf(_("BUF DATA\n"));
> -		if (print_data) {
> -			uint *dp  = (uint *)*ptr;
> -			int  nums = be32_to_cpu(head->oh_len) >> 2;
> -			int  byte = 0;
> -
> -			while (byte < nums) {
> -				if ((byte % 8) == 0)
> -					printf("%2x ", byte);
> -				printf("%8x ", *dp);
> -				dp++;
> -				byte++;
> -				if ((byte % 8) == 0)
> -					printf("\n");
> -			}
> -			printf("\n");
> -		}

Nitpicking: One patch to collapse this into a xlog_recover_print_data
call as a no-functional-changes cleanup, then a second patch to make the
buffer dumps happen any time -D or -o are specified.

TBH the sb/agheader decoders probably need some serious updating to
handle newer fields.  It's also unfortunate that xfs_db doesn't know how
to decode log buffers; adding such a thing would be a neat way to enable
targetted fuzzing of log recovery.

--D

>  	}
> +
> +	xlog_recover_print_data(*ptr, be32_to_cpu(head->oh_len));
> +
>  	*ptr += be32_to_cpu(head->oh_len);
>      }
>      if (head && head->oh_flags & XLOG_CONTINUE_TRANS)
> diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
> index eafffe28..2b9e810d 100644
> --- a/logprint/log_print_all.c
> +++ b/logprint/log_print_all.c
> @@ -176,8 +176,8 @@ xlog_recover_print_buffer(
>  		} else {
>  			printf(_("	BUF DATA\n"));
>  			if (!print_buffer) continue;
> -			xlog_recover_print_data(p, len);
>  		}
> +		xlog_recover_print_data(p, len);
>  	}
>  }
>  
> -- 
> 2.27.0
> 

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

* Re: [PATCH 1/2] xfs_logprint: print misc buffers when using -o
  2021-01-28 17:35   ` Darrick J. Wong
@ 2021-01-28 21:51     ` Donald Douwsma
  0 siblings, 0 replies; 7+ messages in thread
From: Donald Douwsma @ 2021-01-28 21:51 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs



On 29/01/2021 04:35, Darrick J. Wong wrote:
> On Thu, Jan 28, 2021 at 06:37:07PM +1100, Donald Douwsma wrote:
>> Logprint only dumps raw buffers for unhandled misc buffer types, but
>> this information is generally useful when debugging logprint issues so
>> allow it to print whenever -o is used.
>>
>> Switch to using the common xlog_print_data function to dump the buffer.
>>
>> Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
>> ---
>>  logprint/log_misc.c      | 19 +++----------------
>>  logprint/log_print_all.c |  2 +-
>>  2 files changed, 4 insertions(+), 17 deletions(-)
>>
>> diff --git a/logprint/log_misc.c b/logprint/log_misc.c
>> index c325f046..d44e9ff7 100644
>> --- a/logprint/log_misc.c
>> +++ b/logprint/log_misc.c
>> @@ -392,23 +392,10 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
>>  		}
>>  	} else {
>>  		printf(_("BUF DATA\n"));
>> -		if (print_data) {
>> -			uint *dp  = (uint *)*ptr;
>> -			int  nums = be32_to_cpu(head->oh_len) >> 2;
>> -			int  byte = 0;
>> -
>> -			while (byte < nums) {
>> -				if ((byte % 8) == 0)
>> -					printf("%2x ", byte);
>> -				printf("%8x ", *dp);
>> -				dp++;
>> -				byte++;
>> -				if ((byte % 8) == 0)
>> -					printf("\n");
>> -			}
>> -			printf("\n");
>> -		}
> 
> Nitpicking: One patch to collapse this into a xlog_recover_print_data
> call as a no-functional-changes cleanup, then a second patch to make the
> buffer dumps happen any time -D or -o are specified.
> 

ok

> TBH the sb/agheader decoders probably need some serious updating to
> handle newer fields.  It's also unfortunate that xfs_db doesn't know how
> to decode log buffers; adding such a thing would be a neat way to enable
> targetted fuzzing of log recovery.
> 

The free space accounting probably isn't the most useful thing to be dumping
because of the way they're re-calculated from the AG headers during recovery,
but I'd been looking into a sb free space issue and this was confusing me.

It could dump all the fields like xfs_db does, but that would be very verbose. 

By decode log buffers do you mean more of the raw log buffer?

> --D
> 
>>  	}
>> +
>> +	xlog_recover_print_data(*ptr, be32_to_cpu(head->oh_len));
>> +
>>  	*ptr += be32_to_cpu(head->oh_len);
>>      }
>>      if (head && head->oh_flags & XLOG_CONTINUE_TRANS)
>> diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
>> index eafffe28..2b9e810d 100644
>> --- a/logprint/log_print_all.c
>> +++ b/logprint/log_print_all.c
>> @@ -176,8 +176,8 @@ xlog_recover_print_buffer(
>>  		} else {
>>  			printf(_("	BUF DATA\n"));
>>  			if (!print_buffer) continue;
>> -			xlog_recover_print_data(p, len);
>>  		}
>> +		xlog_recover_print_data(p, len);
>>  	}
>>  }
>>  
>> -- 
>> 2.27.0
>>
> 


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

* Re: [PATCH 2/2] xfs_logprint: decode superblock updates correctly
  2021-01-28 17:28   ` Darrick J. Wong
@ 2021-01-28 21:59     ` Donald Douwsma
  0 siblings, 0 replies; 7+ messages in thread
From: Donald Douwsma @ 2021-01-28 21:59 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs



On 29/01/2021 04:28, Darrick J. Wong wrote:
> On Thu, Jan 28, 2021 at 06:37:08PM +1100, Donald Douwsma wrote:
>> Back when the way superblocks are logged changed, logprint wasnt
>> updated updated. Currently logprint displays incorrect accounting
> 
> Double 'updated'.

Oops

> 
>> information.
>>
>>  SUPER BLOCK Buffer:
>>  icount: 6360863066640355328  ifree: 262144  fdblks: 0  frext: 0
>>
>>  $ printf "0x%x\n" 6360863066640355328
>>  0x5846534200001000
>>
>> Part of this decodes as 'XFSB', the xfs superblock magic number and not
>> the free space accounting.
>>
>> Fix this by looking at the entire superblock buffer and using the format
>> structure as is done for the other allocation group headers.
>>
>> Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
>> ---
>>  logprint/log_misc.c      | 22 +++++++++-------------
>>  logprint/log_print_all.c | 23 ++++++++++-------------
>>  2 files changed, 19 insertions(+), 26 deletions(-)
>>
>> diff --git a/logprint/log_misc.c b/logprint/log_misc.c
>> index d44e9ff7..929842d0 100644
>> --- a/logprint/log_misc.c
>> +++ b/logprint/log_misc.c
>> @@ -243,25 +243,21 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
>>  	xlog_print_op_header(head, *i, ptr);
>>  	if (super_block) {
>>  		printf(_("SUPER BLOCK Buffer: "));
>> -		if (be32_to_cpu(head->oh_len) < 4*8) {
>> +		if (be32_to_cpu(head->oh_len) < sizeof(struct xfs_sb)) {
>>  			printf(_("Out of space\n"));
>>  		} else {
>> -			__be64		 a, b;
>> +			struct xfs_sb *sb, sb_s;
>>  
>>  			printf("\n");
>> -			/*
>> -			 * memmove because *ptr may not be 8-byte aligned
>> -			 */
>> -			memmove(&a, *ptr, sizeof(__be64));
>> -			memmove(&b, *ptr+8, sizeof(__be64));
>> +			/* memmove because *ptr may not be 8-byte aligned */
>> +			sb = &sb_s;
>> +			memmove(sb, *ptr, sizeof(struct xfs_sb));
>>  			printf(_("icount: %llu  ifree: %llu  "),
>> -			       (unsigned long long) be64_to_cpu(a),
>> -			       (unsigned long long) be64_to_cpu(b));
>> -			memmove(&a, *ptr+16, sizeof(__be64));
>> -			memmove(&b, *ptr+24, sizeof(__be64));
>> +				be64_to_cpu(sb->sb_icount),
>> +				be64_to_cpu(sb->sb_ifree) );
> 
> Extra space at the end ..................................^

ack

> 
>>  			printf(_("fdblks: %llu  frext: %llu\n"),
>> -			       (unsigned long long) be64_to_cpu(a),
>> -			       (unsigned long long) be64_to_cpu(b));
>> +				be64_to_cpu(sb->sb_fdblocks),
>> +				be64_to_cpu(sb->sb_frextents));
>>  		}
>>  		super_block = 0;
>>  	} else if (be32_to_cpu(*(__be32 *)(*ptr)) == XFS_AGI_MAGIC) {
>> diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
>> index 2b9e810d..8ff87068 100644
>> --- a/logprint/log_print_all.c
>> +++ b/logprint/log_print_all.c
>> @@ -91,22 +91,19 @@ xlog_recover_print_buffer(
>>  		len = item->ri_buf[i].i_len;
>>  		i++;
>>  		if (blkno == 0) { /* super block */
>> -			printf(_("	SUPER Block Buffer:\n"));
>> +                        struct xfs_sb *sb = (struct xfs_sb *)p;
>> +			printf(_("	Super Block Buffer: (XFSB)\n"));
>>  			if (!print_buffer)
>>  				continue;
>> -		       printf(_("              icount:%llu ifree:%llu  "),
>> -			       (unsigned long long)
>> -				       be64_to_cpu(*(__be64 *)(p)),
>> -			       (unsigned long long)
>> -				       be64_to_cpu(*(__be64 *)(p+8)));
>> -		       printf(_("fdblks:%llu  frext:%llu\n"),
>> -			       (unsigned long long)
>> -				       be64_to_cpu(*(__be64 *)(p+16)),
>> -			       (unsigned long long)
>> -				       be64_to_cpu(*(__be64 *)(p+24)));
>> +			printf(_("		icount:%llu  ifree:%llu  "),
>> +					be64_to_cpu(sb->sb_icount),
>> +					be64_to_cpu(sb->sb_ifree));
>> +			printf(_("fdblks:%llu  frext:%llu\n"),
>> +					be64_to_cpu(sb->sb_fdblocks),
>> +					be64_to_cpu(sb->sb_frextents));
>>  			printf(_("		sunit:%u  swidth:%u\n"),
>> -			       be32_to_cpu(*(__be32 *)(p+56)),
>> -			       be32_to_cpu(*(__be32 *)(p+60)));
>> +			       be32_to_cpu(sb->sb_unit),
>> +			       be32_to_cpu(sb->sb_width));
> 
> /me wonders if these nearly identical decoder routines ought to be
> refactored into a common helper?

log_print_all.c seems to be coupled to the log recovery code in libxfs when
-t is used, but xfs_log_misc.c does its own thing to decode the records 
outside of recovery. Its a bit messy.

> 
> Also, what happens if logprint encounters a log from before whenever we
> changed superblock encoding in the log?  When was that, anyway?

... um bad things, it will dump information from further along in the log, 
possibly beyond the end of the buffer and crash. 

I _think_ it was when lazy superblock updates went in, tracking that down
would be a better way to work out how to handle this, I'll do some research.

> 
> --D
> 
>>  		} else if (be32_to_cpu(*(__be32 *)p) == XFS_AGI_MAGIC) {
>>  			int bucket, buckets;
>>  			agi = (xfs_agi_t *)p;
>> -- 
>> 2.27.0
>>
> 


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

end of thread, other threads:[~2021-01-28 22:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-28  7:37 [PATCH 0/2] xfsprogs: xfs_logprint misc log decoding issues Donald Douwsma
2021-01-28  7:37 ` [PATCH 1/2] xfs_logprint: print misc buffers when using -o Donald Douwsma
2021-01-28 17:35   ` Darrick J. Wong
2021-01-28 21:51     ` Donald Douwsma
2021-01-28  7:37 ` [PATCH 2/2] xfs_logprint: decode superblock updates correctly Donald Douwsma
2021-01-28 17:28   ` Darrick J. Wong
2021-01-28 21:59     ` Donald Douwsma

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.