All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs_db: allow write -d to inodes
@ 2017-04-11 22:53 Darrick J. Wong
  2017-04-12  2:09 ` Eric Sandeen
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Darrick J. Wong @ 2017-04-11 22:53 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: xfs

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

Add a helper function to xfs_db so that we can recalculate the CRC of an
inode whose field we just wrote.  This enables us to write arbitrary
values with a good CRC for the purpose of checking the read verifiers on
a v5 filesystem.  Mention this newfound ability in the manpage.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/io.c           |   11 +++++++++++
 db/io.h           |    1 +
 db/write.c        |    7 ++++++-
 man/man8/xfs_db.8 |    7 ++++++-
 4 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/db/io.c b/db/io.c
index f398195..67ed5f9 100644
--- a/db/io.c
+++ b/db/io.c
@@ -465,6 +465,17 @@ xfs_dummy_verify(
 }
 
 void
+xfs_verify_recalc_inode_crc(
+	struct xfs_buf *bp)
+{
+	ASSERT(iocur_top->ino_buf);
+	ASSERT(iocur_top->bp == bp);
+
+	libxfs_dinode_calc_crc(mp, iocur_top->data);
+	iocur_top->ino_crc_ok = 1;
+}
+
+void
 xfs_verify_recalc_crc(
 	struct xfs_buf *bp)
 {
diff --git a/db/io.h b/db/io.h
index c69e9ce..12d96c2 100644
--- a/db/io.h
+++ b/db/io.h
@@ -64,6 +64,7 @@ extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
 extern void     ring_add(void);
 extern void	set_iocur_type(const struct typ *t);
 extern void	xfs_dummy_verify(struct xfs_buf *bp);
+extern void	xfs_verify_recalc_inode_crc(struct xfs_buf *bp);
 extern void	xfs_verify_recalc_crc(struct xfs_buf *bp);
 
 /*
diff --git a/db/write.c b/db/write.c
index 5c83874..70c9865 100644
--- a/db/write.c
+++ b/db/write.c
@@ -137,7 +137,9 @@ write_f(
 		return 0;
 	}
 
-	if (invalid_data && iocur_top->typ->crc_off == TYP_F_NO_CRC_OFF) {
+	if (invalid_data &&
+	    iocur_top->typ->crc_off == TYP_F_NO_CRC_OFF &&
+	    !iocur_top->ino_buf) {
 		dbprintf(_("Cannot recalculate CRCs on this type of object\n"));
 		return 0;
 	}
@@ -164,6 +166,9 @@ write_f(
 	if (corrupt) {
 		local_ops.verify_write = xfs_dummy_verify;
 		dbprintf(_("Allowing write of corrupted data and bad CRC\n"));
+	} else if (iocur_top->ino_buf) {
+		local_ops.verify_write = xfs_verify_recalc_inode_crc;
+		dbprintf(_("Allowing write of corrupted inode with good CRC\n"));
 	} else { /* invalid data */
 		local_ops.verify_write = xfs_verify_recalc_crc;
 		dbprintf(_("Allowing write of corrupted data with good CRC\n"));
diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8
index 460d89d..b1c341d 100644
--- a/man/man8/xfs_db.8
+++ b/man/man8/xfs_db.8
@@ -755,7 +755,7 @@ and
 bits respectively, and their string equivalent reported
 (but no modifications are made).
 .TP
-.BI "write [\-c] [" "field value" "] ..."
+.BI "write [\-c] [\-d] [" "field value" "] ..."
 Write a value to disk.
 Specific fields can be set in structures (struct mode),
 or a block can be set to data values (data mode),
@@ -778,6 +778,11 @@ with no arguments gives more information on the allowed commands.
 .B \-c
 Skip write verifiers and CRC recalculation; allows invalid data to be written
 to disk.
+.TP 0.4i
+.B \-d
+Skip write verifiers but perform CRC recalculation.
+This allows invalid data, including inodes, to be written to disk to
+test detection of invalid data.
 .RE
 .SH TYPES
 This section gives the fields in each structure type and their meanings.

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

* Re: [PATCH] xfs_db: allow write -d to inodes
  2017-04-11 22:53 [PATCH] xfs_db: allow write -d to inodes Darrick J. Wong
@ 2017-04-12  2:09 ` Eric Sandeen
  2017-04-12  2:58   ` Eric Sandeen
  2017-04-12  3:45 ` [PATCH] xfs_db: allow write -d to dqblks Eric Sandeen
  2017-04-12  3:54 ` [PATCH] xfs_db: set buf flag for inode or dqblk with "type" Eric Sandeen
  2 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2017-04-12  2:09 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: xfs

On 4/11/17 5:53 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add a helper function to xfs_db so that we can recalculate the CRC of an
> inode whose field we just wrote.  This enables us to write arbitrary
> values with a good CRC for the purpose of checking the read verifiers on
> a v5 filesystem.  Mention this newfound ability in the manpage.

-d isn't new, so I'd really rather have a patch to document the existing
behavior, followed by a patch to extend it to inodes.

There are actually several structures for which -d still won't work, so
it would be good if that fact were documented - reading "including
inodes" may leave some heads scratching.

Let me ruminate on the inode crc writing, I have this nagging feeling
that there's an easier generic way to handle these structures,
...but I'm probably wrong.

Thanks,
-Eric

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  db/io.c           |   11 +++++++++++
>  db/io.h           |    1 +
>  db/write.c        |    7 ++++++-
>  man/man8/xfs_db.8 |    7 ++++++-
>  4 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/db/io.c b/db/io.c
> index f398195..67ed5f9 100644
> --- a/db/io.c
> +++ b/db/io.c
> @@ -465,6 +465,17 @@ xfs_dummy_verify(
>  }
>  
>  void
> +xfs_verify_recalc_inode_crc(
> +	struct xfs_buf *bp)
> +{
> +	ASSERT(iocur_top->ino_buf);
> +	ASSERT(iocur_top->bp == bp);
> +
> +	libxfs_dinode_calc_crc(mp, iocur_top->data);
> +	iocur_top->ino_crc_ok = 1;
> +}
> +
> +void
>  xfs_verify_recalc_crc(
>  	struct xfs_buf *bp)
>  {
> diff --git a/db/io.h b/db/io.h
> index c69e9ce..12d96c2 100644
> --- a/db/io.h
> +++ b/db/io.h
> @@ -64,6 +64,7 @@ extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
>  extern void     ring_add(void);
>  extern void	set_iocur_type(const struct typ *t);
>  extern void	xfs_dummy_verify(struct xfs_buf *bp);
> +extern void	xfs_verify_recalc_inode_crc(struct xfs_buf *bp);
>  extern void	xfs_verify_recalc_crc(struct xfs_buf *bp);
>  
>  /*
> diff --git a/db/write.c b/db/write.c
> index 5c83874..70c9865 100644
> --- a/db/write.c
> +++ b/db/write.c
> @@ -137,7 +137,9 @@ write_f(
>  		return 0;
>  	}
>  
> -	if (invalid_data && iocur_top->typ->crc_off == TYP_F_NO_CRC_OFF) {
> +	if (invalid_data &&
> +	    iocur_top->typ->crc_off == TYP_F_NO_CRC_OFF &&
> +	    !iocur_top->ino_buf) {
>  		dbprintf(_("Cannot recalculate CRCs on this type of object\n"));
>  		return 0;
>  	}
> @@ -164,6 +166,9 @@ write_f(
>  	if (corrupt) {
>  		local_ops.verify_write = xfs_dummy_verify;
>  		dbprintf(_("Allowing write of corrupted data and bad CRC\n"));
> +	} else if (iocur_top->ino_buf) {
> +		local_ops.verify_write = xfs_verify_recalc_inode_crc;
> +		dbprintf(_("Allowing write of corrupted inode with good CRC\n"));
>  	} else { /* invalid data */
>  		local_ops.verify_write = xfs_verify_recalc_crc;
>  		dbprintf(_("Allowing write of corrupted data with good CRC\n"));
> diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8
> index 460d89d..b1c341d 100644
> --- a/man/man8/xfs_db.8
> +++ b/man/man8/xfs_db.8
> @@ -755,7 +755,7 @@ and
>  bits respectively, and their string equivalent reported
>  (but no modifications are made).
>  .TP
> -.BI "write [\-c] [" "field value" "] ..."
> +.BI "write [\-c] [\-d] [" "field value" "] ..."
>  Write a value to disk.
>  Specific fields can be set in structures (struct mode),
>  or a block can be set to data values (data mode),
> @@ -778,6 +778,11 @@ with no arguments gives more information on the allowed commands.
>  .B \-c
>  Skip write verifiers and CRC recalculation; allows invalid data to be written
>  to disk.
> +.TP 0.4i
> +.B \-d
> +Skip write verifiers but perform CRC recalculation.
> +This allows invalid data, including inodes, to be written to disk to
> +test detection of invalid data.
>  .RE
>  .SH TYPES
>  This section gives the fields in each structure type and their meanings.
> 


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

* Re: [PATCH] xfs_db: allow write -d to inodes
  2017-04-12  2:09 ` Eric Sandeen
@ 2017-04-12  2:58   ` Eric Sandeen
  2017-04-12  6:11     ` Darrick J. Wong
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2017-04-12  2:58 UTC (permalink / raw)
  To: Eric Sandeen, Darrick J. Wong; +Cc: xfs

On 4/11/17 9:09 PM, Eric Sandeen wrote:
> On 4/11/17 5:53 PM, Darrick J. Wong wrote:
>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>
>> Add a helper function to xfs_db so that we can recalculate the CRC of an
>> inode whose field we just wrote.  This enables us to write arbitrary
>> values with a good CRC for the purpose of checking the read verifiers on
>> a v5 filesystem.  Mention this newfound ability in the manpage.
> 
> -d isn't new, so I'd really rather have a patch to document the existing
> behavior, followed by a patch to extend it to inodes.
> 
> There are actually several structures for which -d still won't work, so
> it would be good if that fact were documented - reading "including
> inodes" may leave some heads scratching.
> 
> Let me ruminate on the inode crc writing, I have this nagging feeling
> that there's an easier generic way to handle these structures,
> ...but I'm probably wrong.

I don't know where I was going with that idea.  :/

I can split this up into a manpage patch and an inode crc patch on
the way in if you'd like.

For the content as-is, 

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

-Eric

> Thanks,
> -Eric
> 
>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>> ---
>>  db/io.c           |   11 +++++++++++
>>  db/io.h           |    1 +
>>  db/write.c        |    7 ++++++-
>>  man/man8/xfs_db.8 |    7 ++++++-
>>  4 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/db/io.c b/db/io.c
>> index f398195..67ed5f9 100644
>> --- a/db/io.c
>> +++ b/db/io.c
>> @@ -465,6 +465,17 @@ xfs_dummy_verify(
>>  }
>>  
>>  void
>> +xfs_verify_recalc_inode_crc(
>> +	struct xfs_buf *bp)
>> +{
>> +	ASSERT(iocur_top->ino_buf);
>> +	ASSERT(iocur_top->bp == bp);
>> +
>> +	libxfs_dinode_calc_crc(mp, iocur_top->data);
>> +	iocur_top->ino_crc_ok = 1;
>> +}
>> +
>> +void
>>  xfs_verify_recalc_crc(
>>  	struct xfs_buf *bp)
>>  {
>> diff --git a/db/io.h b/db/io.h
>> index c69e9ce..12d96c2 100644
>> --- a/db/io.h
>> +++ b/db/io.h
>> @@ -64,6 +64,7 @@ extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
>>  extern void     ring_add(void);
>>  extern void	set_iocur_type(const struct typ *t);
>>  extern void	xfs_dummy_verify(struct xfs_buf *bp);
>> +extern void	xfs_verify_recalc_inode_crc(struct xfs_buf *bp);
>>  extern void	xfs_verify_recalc_crc(struct xfs_buf *bp);
>>  
>>  /*
>> diff --git a/db/write.c b/db/write.c
>> index 5c83874..70c9865 100644
>> --- a/db/write.c
>> +++ b/db/write.c
>> @@ -137,7 +137,9 @@ write_f(
>>  		return 0;
>>  	}
>>  
>> -	if (invalid_data && iocur_top->typ->crc_off == TYP_F_NO_CRC_OFF) {
>> +	if (invalid_data &&
>> +	    iocur_top->typ->crc_off == TYP_F_NO_CRC_OFF &&
>> +	    !iocur_top->ino_buf) {
>>  		dbprintf(_("Cannot recalculate CRCs on this type of object\n"));
>>  		return 0;
>>  	}
>> @@ -164,6 +166,9 @@ write_f(
>>  	if (corrupt) {
>>  		local_ops.verify_write = xfs_dummy_verify;
>>  		dbprintf(_("Allowing write of corrupted data and bad CRC\n"));
>> +	} else if (iocur_top->ino_buf) {
>> +		local_ops.verify_write = xfs_verify_recalc_inode_crc;
>> +		dbprintf(_("Allowing write of corrupted inode with good CRC\n"));
>>  	} else { /* invalid data */
>>  		local_ops.verify_write = xfs_verify_recalc_crc;
>>  		dbprintf(_("Allowing write of corrupted data with good CRC\n"));
>> diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8
>> index 460d89d..b1c341d 100644
>> --- a/man/man8/xfs_db.8
>> +++ b/man/man8/xfs_db.8
>> @@ -755,7 +755,7 @@ and
>>  bits respectively, and their string equivalent reported
>>  (but no modifications are made).
>>  .TP
>> -.BI "write [\-c] [" "field value" "] ..."
>> +.BI "write [\-c] [\-d] [" "field value" "] ..."
>>  Write a value to disk.
>>  Specific fields can be set in structures (struct mode),
>>  or a block can be set to data values (data mode),
>> @@ -778,6 +778,11 @@ with no arguments gives more information on the allowed commands.
>>  .B \-c
>>  Skip write verifiers and CRC recalculation; allows invalid data to be written
>>  to disk.
>> +.TP 0.4i
>> +.B \-d
>> +Skip write verifiers but perform CRC recalculation.
>> +This allows invalid data, including inodes, to be written to disk to
>> +test detection of invalid data.
>>  .RE
>>  .SH TYPES
>>  This section gives the fields in each structure type and their meanings.
>>
> 
> --
> 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] 9+ messages in thread

* [PATCH] xfs_db: allow write -d to dqblks
  2017-04-11 22:53 [PATCH] xfs_db: allow write -d to inodes Darrick J. Wong
  2017-04-12  2:09 ` Eric Sandeen
@ 2017-04-12  3:45 ` Eric Sandeen
  2017-04-12  6:11   ` Darrick J. Wong
  2017-04-12  3:54 ` [PATCH] xfs_db: set buf flag for inode or dqblk with "type" Eric Sandeen
  2 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2017-04-12  3:45 UTC (permalink / raw)
  To: Darrick J. Wong, Eric Sandeen; +Cc: xfs

Allow write -d to write bad data and recalculate CRC
for dqblks.

Inspired-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/db/io.c b/db/io.c
index 67ed5f9..9918a51 100644
--- a/db/io.c
+++ b/db/io.c
@@ -476,6 +476,17 @@ xfs_verify_recalc_inode_crc(
 }
 
 void
+xfs_verify_recalc_dquot_crc(
+	struct xfs_buf *bp)
+{
+	ASSERT((iocur_top->dquot_buf));
+	ASSERT(iocur_top->bp == bp);
+
+	xfs_update_cksum(iocur_top->data, sizeof(struct xfs_dqblk),
+			 XFS_DQUOT_CRC_OFF);
+}
+
+void
 xfs_verify_recalc_crc(
 	struct xfs_buf *bp)
 {
diff --git a/db/io.h b/db/io.h
index 12d96c2..b415b82 100644
--- a/db/io.h
+++ b/db/io.h
@@ -65,6 +65,7 @@ extern void     ring_add(void);
 extern void	set_iocur_type(const struct typ *t);
 extern void	xfs_dummy_verify(struct xfs_buf *bp);
 extern void	xfs_verify_recalc_inode_crc(struct xfs_buf *bp);
+extern void	xfs_verify_recalc_dquot_crc(struct xfs_buf *bp);
 extern void	xfs_verify_recalc_crc(struct xfs_buf *bp);
 
 /*
diff --git a/db/write.c b/db/write.c
index 70c9865..d24ea05 100644
--- a/db/write.c
+++ b/db/write.c
@@ -139,7 +139,8 @@ write_f(
 
 	if (invalid_data &&
 	    iocur_top->typ->crc_off == TYP_F_NO_CRC_OFF &&
-	    !iocur_top->ino_buf) {
+	    !iocur_top->ino_buf &&
+	    !iocur_top->dquot_buf) {
 		dbprintf(_("Cannot recalculate CRCs on this type of object\n"));
 		return 0;
 	}
@@ -169,6 +170,9 @@ write_f(
 	} else if (iocur_top->ino_buf) {
 		local_ops.verify_write = xfs_verify_recalc_inode_crc;
 		dbprintf(_("Allowing write of corrupted inode with good CRC\n"));
+	} else if (iocur_top->dquot_buf) {
+		local_ops.verify_write = xfs_verify_recalc_dquot_crc;
+		dbprintf(_("Allowing write of corrupted dquot with good CRC\n"));
 	} else { /* invalid data */
 		local_ops.verify_write = xfs_verify_recalc_crc;
 		dbprintf(_("Allowing write of corrupted data with good CRC\n"));


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

* [PATCH] xfs_db: set buf flag for inode or dqblk with "type"
  2017-04-11 22:53 [PATCH] xfs_db: allow write -d to inodes Darrick J. Wong
  2017-04-12  2:09 ` Eric Sandeen
  2017-04-12  3:45 ` [PATCH] xfs_db: allow write -d to dqblks Eric Sandeen
@ 2017-04-12  3:54 ` Eric Sandeen
  2017-04-12  6:07   ` Darrick J. Wong
  2 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2017-04-12  3:54 UTC (permalink / raw)
  To: Darrick J. Wong, Eric Sandeen; +Cc: xfs

Currently, setting "type" to inode or dqblk does not
appropriately set the buffer type flags for these
types.  i.e.:

xfs_db> inode 128

sets ino_buf, but

xfs_db> convert inode 128 fsblock
0x10 (16)
xfs_db> fsblock 16
xfs_db> type inode

does not.  This is apparent if we try to use "type" to
switch to an inode or dqblk, then try to use "write -d"
which tests these flags.

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

diff --git a/db/io.c b/db/io.c
index 9918a51..d78fc66 100644
--- a/db/io.c
+++ b/db/io.c
@@ -617,6 +617,13 @@ set_iocur_type(
 	struct xfs_buf	*bp = iocur_top->bp;
 
 	iocur_top->typ = t;
+	iocur_top->ino_buf = 0;
+	iocur_top->dquot_buf  = 0;
+
+	if (t->typnm == TYP_INODE)
+		iocur_top->ino_buf = 1;
+	else if (t->typnm == TYP_DQBLK)
+		iocur_top->dquot_buf = 1;
 
 	/* verify the buffer if the type has one. */
 	if (!bp)



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

* Re: [PATCH] xfs_db: set buf flag for inode or dqblk with "type"
  2017-04-12  3:54 ` [PATCH] xfs_db: set buf flag for inode or dqblk with "type" Eric Sandeen
@ 2017-04-12  6:07   ` Darrick J. Wong
  2017-04-12 12:35     ` Eric Sandeen
  0 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2017-04-12  6:07 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eric Sandeen, xfs

On Tue, Apr 11, 2017 at 10:54:15PM -0500, Eric Sandeen wrote:
> Currently, setting "type" to inode or dqblk does not
> appropriately set the buffer type flags for these
> types.  i.e.:
> 
> xfs_db> inode 128
> 
> sets ino_buf, but
> 
> xfs_db> convert inode 128 fsblock
> 0x10 (16)
> xfs_db> fsblock 16
> xfs_db> type inode
> 
> does not.  This is apparent if we try to use "type" to
> switch to an inode or dqblk, then try to use "write -d"
> which tests these flags.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/db/io.c b/db/io.c
> index 9918a51..d78fc66 100644
> --- a/db/io.c
> +++ b/db/io.c
> @@ -617,6 +617,13 @@ set_iocur_type(
>  	struct xfs_buf	*bp = iocur_top->bp;
>  
>  	iocur_top->typ = t;
> +	iocur_top->ino_buf = 0;
> +	iocur_top->dquot_buf  = 0;

Extra spaces.

> +
> +	if (t->typnm == TYP_INODE)
> +		iocur_top->ino_buf = 1;
> +	else if (t->typnm == TYP_DQBLK)
> +		iocur_top->dquot_buf = 1;

iocur_top->dquot_buf = (t->typnm == TYP_DQBLK); ?

--D

>  
>  	/* verify the buffer if the type has one. */
>  	if (!bp)
> 
> 
> --
> 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] 9+ messages in thread

* Re: [PATCH] xfs_db: allow write -d to dqblks
  2017-04-12  3:45 ` [PATCH] xfs_db: allow write -d to dqblks Eric Sandeen
@ 2017-04-12  6:11   ` Darrick J. Wong
  0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2017-04-12  6:11 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eric Sandeen, xfs

On Tue, Apr 11, 2017 at 10:45:27PM -0500, Eric Sandeen wrote:
> Allow write -d to write bad data and recalculate CRC
> for dqblks.
> 
> Inspired-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

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

> ---
> 
> diff --git a/db/io.c b/db/io.c
> index 67ed5f9..9918a51 100644
> --- a/db/io.c
> +++ b/db/io.c
> @@ -476,6 +476,17 @@ xfs_verify_recalc_inode_crc(
>  }
>  
>  void
> +xfs_verify_recalc_dquot_crc(
> +	struct xfs_buf *bp)
> +{
> +	ASSERT((iocur_top->dquot_buf));
> +	ASSERT(iocur_top->bp == bp);
> +
> +	xfs_update_cksum(iocur_top->data, sizeof(struct xfs_dqblk),
> +			 XFS_DQUOT_CRC_OFF);
> +}
> +
> +void
>  xfs_verify_recalc_crc(
>  	struct xfs_buf *bp)
>  {
> diff --git a/db/io.h b/db/io.h
> index 12d96c2..b415b82 100644
> --- a/db/io.h
> +++ b/db/io.h
> @@ -65,6 +65,7 @@ extern void     ring_add(void);
>  extern void	set_iocur_type(const struct typ *t);
>  extern void	xfs_dummy_verify(struct xfs_buf *bp);
>  extern void	xfs_verify_recalc_inode_crc(struct xfs_buf *bp);
> +extern void	xfs_verify_recalc_dquot_crc(struct xfs_buf *bp);
>  extern void	xfs_verify_recalc_crc(struct xfs_buf *bp);
>  
>  /*
> diff --git a/db/write.c b/db/write.c
> index 70c9865..d24ea05 100644
> --- a/db/write.c
> +++ b/db/write.c
> @@ -139,7 +139,8 @@ write_f(
>  
>  	if (invalid_data &&
>  	    iocur_top->typ->crc_off == TYP_F_NO_CRC_OFF &&
> -	    !iocur_top->ino_buf) {
> +	    !iocur_top->ino_buf &&
> +	    !iocur_top->dquot_buf) {
>  		dbprintf(_("Cannot recalculate CRCs on this type of object\n"));
>  		return 0;
>  	}
> @@ -169,6 +170,9 @@ write_f(
>  	} else if (iocur_top->ino_buf) {
>  		local_ops.verify_write = xfs_verify_recalc_inode_crc;
>  		dbprintf(_("Allowing write of corrupted inode with good CRC\n"));
> +	} else if (iocur_top->dquot_buf) {
> +		local_ops.verify_write = xfs_verify_recalc_dquot_crc;
> +		dbprintf(_("Allowing write of corrupted dquot with good CRC\n"));
>  	} else { /* invalid data */
>  		local_ops.verify_write = xfs_verify_recalc_crc;
>  		dbprintf(_("Allowing write of corrupted data with good CRC\n"));
> 
> --
> 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] 9+ messages in thread

* Re: [PATCH] xfs_db: allow write -d to inodes
  2017-04-12  2:58   ` Eric Sandeen
@ 2017-04-12  6:11     ` Darrick J. Wong
  0 siblings, 0 replies; 9+ messages in thread
From: Darrick J. Wong @ 2017-04-12  6:11 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eric Sandeen, xfs

On Tue, Apr 11, 2017 at 09:58:44PM -0500, Eric Sandeen wrote:
> On 4/11/17 9:09 PM, Eric Sandeen wrote:
> > On 4/11/17 5:53 PM, Darrick J. Wong wrote:
> >> From: Darrick J. Wong <darrick.wong@oracle.com>
> >>
> >> Add a helper function to xfs_db so that we can recalculate the CRC of an
> >> inode whose field we just wrote.  This enables us to write arbitrary
> >> values with a good CRC for the purpose of checking the read verifiers on
> >> a v5 filesystem.  Mention this newfound ability in the manpage.
> > 
> > -d isn't new, so I'd really rather have a patch to document the existing
> > behavior, followed by a patch to extend it to inodes.
> > 
> > There are actually several structures for which -d still won't work, so
> > it would be good if that fact were documented - reading "including
> > inodes" may leave some heads scratching.
> > 
> > Let me ruminate on the inode crc writing, I have this nagging feeling
> > that there's an easier generic way to handle these structures,
> > ...but I'm probably wrong.
> 
> I don't know where I was going with that idea.  :/
> 
> I can split this up into a manpage patch and an inode crc patch on
> the way in if you'd like.

Works for me.

--D

> 
> For the content as-is, 
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> -Eric
> 
> > Thanks,
> > -Eric
> > 
> >> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> >> ---
> >>  db/io.c           |   11 +++++++++++
> >>  db/io.h           |    1 +
> >>  db/write.c        |    7 ++++++-
> >>  man/man8/xfs_db.8 |    7 ++++++-
> >>  4 files changed, 24 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/db/io.c b/db/io.c
> >> index f398195..67ed5f9 100644
> >> --- a/db/io.c
> >> +++ b/db/io.c
> >> @@ -465,6 +465,17 @@ xfs_dummy_verify(
> >>  }
> >>  
> >>  void
> >> +xfs_verify_recalc_inode_crc(
> >> +	struct xfs_buf *bp)
> >> +{
> >> +	ASSERT(iocur_top->ino_buf);
> >> +	ASSERT(iocur_top->bp == bp);
> >> +
> >> +	libxfs_dinode_calc_crc(mp, iocur_top->data);
> >> +	iocur_top->ino_crc_ok = 1;
> >> +}
> >> +
> >> +void
> >>  xfs_verify_recalc_crc(
> >>  	struct xfs_buf *bp)
> >>  {
> >> diff --git a/db/io.h b/db/io.h
> >> index c69e9ce..12d96c2 100644
> >> --- a/db/io.h
> >> +++ b/db/io.h
> >> @@ -64,6 +64,7 @@ extern void	set_cur(const struct typ *t, __int64_t d, int c, int ring_add,
> >>  extern void     ring_add(void);
> >>  extern void	set_iocur_type(const struct typ *t);
> >>  extern void	xfs_dummy_verify(struct xfs_buf *bp);
> >> +extern void	xfs_verify_recalc_inode_crc(struct xfs_buf *bp);
> >>  extern void	xfs_verify_recalc_crc(struct xfs_buf *bp);
> >>  
> >>  /*
> >> diff --git a/db/write.c b/db/write.c
> >> index 5c83874..70c9865 100644
> >> --- a/db/write.c
> >> +++ b/db/write.c
> >> @@ -137,7 +137,9 @@ write_f(
> >>  		return 0;
> >>  	}
> >>  
> >> -	if (invalid_data && iocur_top->typ->crc_off == TYP_F_NO_CRC_OFF) {
> >> +	if (invalid_data &&
> >> +	    iocur_top->typ->crc_off == TYP_F_NO_CRC_OFF &&
> >> +	    !iocur_top->ino_buf) {
> >>  		dbprintf(_("Cannot recalculate CRCs on this type of object\n"));
> >>  		return 0;
> >>  	}
> >> @@ -164,6 +166,9 @@ write_f(
> >>  	if (corrupt) {
> >>  		local_ops.verify_write = xfs_dummy_verify;
> >>  		dbprintf(_("Allowing write of corrupted data and bad CRC\n"));
> >> +	} else if (iocur_top->ino_buf) {
> >> +		local_ops.verify_write = xfs_verify_recalc_inode_crc;
> >> +		dbprintf(_("Allowing write of corrupted inode with good CRC\n"));
> >>  	} else { /* invalid data */
> >>  		local_ops.verify_write = xfs_verify_recalc_crc;
> >>  		dbprintf(_("Allowing write of corrupted data with good CRC\n"));
> >> diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8
> >> index 460d89d..b1c341d 100644
> >> --- a/man/man8/xfs_db.8
> >> +++ b/man/man8/xfs_db.8
> >> @@ -755,7 +755,7 @@ and
> >>  bits respectively, and their string equivalent reported
> >>  (but no modifications are made).
> >>  .TP
> >> -.BI "write [\-c] [" "field value" "] ..."
> >> +.BI "write [\-c] [\-d] [" "field value" "] ..."
> >>  Write a value to disk.
> >>  Specific fields can be set in structures (struct mode),
> >>  or a block can be set to data values (data mode),
> >> @@ -778,6 +778,11 @@ with no arguments gives more information on the allowed commands.
> >>  .B \-c
> >>  Skip write verifiers and CRC recalculation; allows invalid data to be written
> >>  to disk.
> >> +.TP 0.4i
> >> +.B \-d
> >> +Skip write verifiers but perform CRC recalculation.
> >> +This allows invalid data, including inodes, to be written to disk to
> >> +test detection of invalid data.
> >>  .RE
> >>  .SH TYPES
> >>  This section gives the fields in each structure type and their meanings.
> >>
> > 
> > --
> > 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
> > 
> --
> 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] 9+ messages in thread

* Re: [PATCH] xfs_db: set buf flag for inode or dqblk with "type"
  2017-04-12  6:07   ` Darrick J. Wong
@ 2017-04-12 12:35     ` Eric Sandeen
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Sandeen @ 2017-04-12 12:35 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Eric Sandeen, xfs

On 4/12/17 1:07 AM, Darrick J. Wong wrote:
> On Tue, Apr 11, 2017 at 10:54:15PM -0500, Eric Sandeen wrote:
>> Currently, setting "type" to inode or dqblk does not
>> appropriately set the buffer type flags for these
>> types.  i.e.:
>>
>> xfs_db> inode 128
>>
>> sets ino_buf, but
>>
>> xfs_db> convert inode 128 fsblock
>> 0x10 (16)
>> xfs_db> fsblock 16
>> xfs_db> type inode
>>
>> does not.  This is apparent if we try to use "type" to
>> switch to an inode or dqblk, then try to use "write -d"
>> which tests these flags.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/db/io.c b/db/io.c
>> index 9918a51..d78fc66 100644
>> --- a/db/io.c
>> +++ b/db/io.c
>> @@ -617,6 +617,13 @@ set_iocur_type(
>>  	struct xfs_buf	*bp = iocur_top->bp;
>>  
>>  	iocur_top->typ = t;
>> +	iocur_top->ino_buf = 0;
>> +	iocur_top->dquot_buf  = 0;
> 
> Extra spaces.

yeah, derp.

>> +
>> +	if (t->typnm == TYP_INODE)
>> +		iocur_top->ino_buf = 1;
>> +	else if (t->typnm == TYP_DQBLK)
>> +		iocur_top->dquot_buf = 1;
> 
> iocur_top->dquot_buf = (t->typnm == TYP_DQBLK); ?

*shrug* sure.  I'm not actually sure this patch is quite right,
though, I think it's a band-aid.  Will think about it some more.

Thanks,
-Eric

> --D
> 
>>  
>>  	/* verify the buffer if the type has one. */
>>  	if (!bp)
>>
>>
>> --
>> 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] 9+ messages in thread

end of thread, other threads:[~2017-04-12 12:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-11 22:53 [PATCH] xfs_db: allow write -d to inodes Darrick J. Wong
2017-04-12  2:09 ` Eric Sandeen
2017-04-12  2:58   ` Eric Sandeen
2017-04-12  6:11     ` Darrick J. Wong
2017-04-12  3:45 ` [PATCH] xfs_db: allow write -d to dqblks Eric Sandeen
2017-04-12  6:11   ` Darrick J. Wong
2017-04-12  3:54 ` [PATCH] xfs_db: set buf flag for inode or dqblk with "type" Eric Sandeen
2017-04-12  6:07   ` Darrick J. Wong
2017-04-12 12:35     ` Eric Sandeen

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.