Skip to contentSkip to conversation

xfsprogs: xfs_db crc fixes/changes

PATCH Superseded Eric Sandeen 2 participants 11 messages Sep 6, 2014 – Mar 9, 2015

lore
Share & tools
lore

Eric Sandeen

author
#0 lore raw

cover xfsprogs: xfs_db crc fixes/changes

2 patches related to crcs in xfs_db,

1) display inode CRC status when read, and revalidate it when written
2) add a new "crc" command to validate, invalidate, or revalidate the CRC on a disk structure

thanks,
-Eric

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#1 lore raw

PATCH 1/2 xfs_db: fix inode CRC validity state, and warn on read if invalid

Currently, the "ino_crc_ok" field on the io cursor reflects overall inode validity, not CRC correctness. Because it is only used when printing CRC validity, change it to reflect only that.

In addition, when reading an inode, print the current CRC state.

Note, if specifying an inode which doesn't actually exist, this will claim corruption; I'm not sure if that's good or bad. Today, it already issues corruption errors on the way; this adds a new message as well ;)

xfs_db> inode 129
Metadata corruption detected at block 0x80/0x2000 Metadata corruption detected at block 0x80/0x2000
...
Metadata CRC error detected for ino 129

Signed-off-by Eric Sandeen <[email protected]>

---

db/inode.c +6 -1
index 24170ba..244d03b 100644
@@ -684,13 +684,18 @@ set_cur_inode(
 		numblks, DB_RING_IGN, NULL);
 	off_cur(offset << mp->m_sb.sb_inodelog, mp->m_sb.sb_inodesize);
 	dip = iocur_top->data;
-	iocur_top->ino_crc_ok = libxfs_dinode_verify(mp, ino, dip);
+	iocur_top->ino_crc_ok = xfs_verify_cksum((char *)dip,
+						 mp->m_sb.sb_inodesize,
+						 XFS_DINODE_CRC_OFF);
 	iocur_top->ino_buf = 1;
 	iocur_top->ino = ino;
 	iocur_top->mode = be16_to_cpu(dip->di_mode);
 	if ((iocur_top->mode & S_IFMT) == S_IFDIR)
 		iocur_top->dirino = ino;
 
+	if (xfs_sb_version_hascrc(&mp->m_sb) && !iocur_top->ino_crc_ok)
+		dbprintf(_("Metadata CRC error detected for ino %lld\n"), ino);
+
 	/* track updated info in ring */
 	ring_add();
 }
db/io.c +11 -0
index 7f1b76a..93ebf5c 100644
@@ -473,6 +473,17 @@ write_cur(void)
 		write_cur_bbs();
 	else
 		write_cur_buf();
+
+	if (iocur_top->ino_buf) {
+		xfs_dinode_t    *dip;
+		xfs_ino_t       ino;
+
+		dip = iocur_top->data;
+		ino = iocur_top->ino;
+		iocur_top->ino_crc_ok = xfs_verify_cksum((char *)dip,
+						 mp->m_sb.sb_inodesize,
+						 XFS_DINODE_CRC_OFF);
+	}
 }
 
 void

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#2 lore raw

Re: [PATCH 1/2] xfs_db: fix inode CRC validity state, and warn on read if invalid

Quoted 27 lines · Eric Sandeen

Currently, the "ino_crc_ok" field on the io cursor reflects overall inode validity, not CRC correctness. Because it is only used when printing CRC validity, change it to reflect only that.

In addition, when reading an inode, print the current CRC state.

Note, if specifying an inode which doesn't actually exist, this will claim corruption; I'm not sure if that's good or bad. Today, it already issues corruption errors on the way; this adds a new message as well ;)

xfs_db> inode 129
Metadata corruption detected at block 0x80/0x2000 Metadata corruption detected at block 0x80/0x2000
...
Metadata CRC error detected for ino 129

Signed-off-by Eric Sandeen <[email protected]>

---

db/inode.c +0 -0
index 24170ba..244d03b 100644

db/inode.c set_cur_inode( · line 689

Quoted 7 lines
db/inode.c +3 -1
@@ -684,13 +684,18 @@ set_cur_inode(
 		numblks, DB_RING_IGN, NULL);
 	off_cur(offset << mp->m_sb.sb_inodelog, mp->m_sb.sb_inodesize);
 	dip = iocur_top->data;
-	iocur_top->ino_crc_ok = libxfs_dinode_verify(mp, ino, dip);
+	iocur_top->ino_crc_ok = xfs_verify_cksum((char *)dip,
+						 mp->m_sb.sb_inodesize,
+						 XFS_DINODE_CRC_OFF);

that needs to be "libxfs_verify_cksum".

db/inode.c set_cur_inode( · line 698

Quoted 8 lines
db/inode.c +3 -0
 	iocur_top->ino_buf = 1;
 	iocur_top->ino = ino;
 	iocur_top->mode = be16_to_cpu(dip->di_mode);
 	if ((iocur_top->mode & S_IFMT) == S_IFDIR)
 		iocur_top->dirino = ino;
+	if (xfs_sb_version_hascrc(&mp->m_sb) && !iocur_top->ino_crc_ok)
+		dbprintf(_("Metadata CRC error detected for ino %lld\n"), ino);
+

and we probably shoul dbe looking at making all those sb version checks "libxfs_sb_version_has...." as well.

Quoted 12 lines
db/inode.c +0 -0
 	/* track updated info in ring */
 	ring_add();
 }
db/io.c +1 -0
index 7f1b76a..93ebf5c 100644
@@ -473,6 +473,17 @@ write_cur(void)
 		write_cur_bbs();
 	else
 		write_cur_buf();
+

db/io.c write_cur(void) · line 486

Quoted 10 lines
db/io.c +10 -0
+	if (iocur_top->ino_buf) {
+		xfs_dinode_t    *dip;
+		xfs_ino_t       ino;
+
+		dip = iocur_top->data;
+		ino = iocur_top->ino;
+		iocur_top->ino_crc_ok = xfs_verify_cksum((char *)dip,
+						 mp->m_sb.sb_inodesize,
+						 XFS_DINODE_CRC_OFF);
+	}

Needs a comment explaining why we are reverifying the inode crc.

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#3 lore raw

PATCH 2/2 xfsprogs: xfs_db: add crc manipulation commands

In reply to Eric Sandeen

This adds a new "crc" command to xfs_db for CRC-enabled filesystems.

If a structure has a CRC field, we can validate it, invalidate/corrupt it, or revalidate/rewrite it:

xfs_db> sb 0
xfs_db> crc -v
crc = 0x796c814f (correct)
xfs_db> crc -i
Metadata CRC error detected at block 0x0/0x200
crc = 0x796c8150 (bad)
xfs_db> crc -r
crc = 0x796c814f (correct)

This requires temporarily replacing the write verifier with a dummy which won't recalculate the CRC on the way to disk.

It also required me to write a new flist function, which is totally foreign to me, so hopefully done right - but it seems to work here.

Signed-off-by Eric Sandeen <[email protected]>

---

db/Makefile +1 -1
index bae6154..f0175cc 100644
@@ -8,7 +8,7 @@ include $(TOPDIR)/include/builddefs
 LTCOMMAND = xfs_db
 
 HFILES = addr.h agf.h agfl.h agi.h attr.h attrshort.h bit.h block.h bmap.h \
-	btblock.h bmroot.h check.h command.h convert.h debug.h \
+	btblock.h bmroot.h check.h command.h convert.h crc.h debug.h \
 	dir2.h dir2sf.h dquot.h echo.h faddr.h field.h \
 	flist.h fprint.h frag.h freesp.h hash.h help.h init.h inode.h input.h \
 	io.h malloc.h metadump.h output.h print.h quit.h sb.h sig.h strvec.h \
db/command.c +2 -0
index b7e3165..d44e0a5 100644
@@ -48,6 +48,7 @@
 #include "write.h"
 #include "malloc.h"
 #include "dquot.h"
+#include "crc.h"
 
 cmdinfo_t	*cmdtab;
 int		ncmds;

@@ -123,6 +124,7 @@ init_commands(void)
 	bmap_init();
 	check_init();
 	convert_init();
+	crc_init();
 	debug_init();
 	echo_init();
 	frag_init();
db/crc.c new file +189 -0

189 lines not on this page · raw message

db/crc.h new file +23 -0
new file mode 100644
index 0000000..ec1ab8c
@@ -0,0 +1,23 @@
+/*
+ * Copyright (c) 2014 Red Hat, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+struct field;
+
+extern void	crc_init(void);
+extern void	crc_struct(const field_t *fields, int argc, char **argv);
+extern void	xfs_dummy_write_verify(struct xfs_buf *bp);
db/flist.c +34 -0
index 33f7da7..fa19f70 100644
@@ -411,6 +411,40 @@ flist_split(
 	return v;
 }
 
+/*
+ * Given a set of fields, scan for a field of the given type.
+ * Return an flist leading to the first found field
+ * of that type.
+ * Return NULL if no field of the given type is found.
+ */
+flist_t *
+flist_find_ftyp(
+	const field_t *fields,
+	fldt_t	type)
+{
+	flist_t	*fl;
+	const field_t	*f;
+	const ftattr_t  *fa;
+
+	for (f = fields; f->name; f++) {
+		fl = flist_make(f->name);
+		fl->fld = f;
+		if (f->ftyp == type)
+			return fl;
+		fa = &ftattrtab[f->ftyp];
+		if (fa->subfld) {
+			flist_t *nfl;
+			nfl = flist_find_ftyp(fa->subfld, type);
+			if (nfl) {
+				fl->child = nfl;
+				return fl;
+			}
+		}
+		flist_free(fl);
+	}
+	return NULL;
+}
+
 static void
 ftok_free(
 	ftok_t	*ft)
db/flist.h +1 -0
index 5c9fba0..3f4b312 100644
@@ -37,3 +37,4 @@ extern int	flist_parse(const struct field *fields, flist_t *fl, void *obj,
 			    int startoff);
 extern void	flist_print(flist_t *fl);
 extern flist_t	*flist_scan(char *name);
+extern flist_t	*flist_find_ftyp(const field_t *fields, fldt_t  type);
db/io.c +5 -2
index 5bf40aa..1ba1f90 100644
@@ -27,6 +27,7 @@
 #include "output.h"
 #include "init.h"
 #include "malloc.h"
+#include "crc.h"
 
 static int	pop_f(int argc, char **argv);
 static void     pop_help(void);

@@ -464,9 +465,11 @@ write_cur(void)
 		return;
 	}
 
-	if (iocur_top->ino_buf)
+	if (iocur_top->ino_buf &&
+	    iocur_top->bp->b_ops->verify_write != xfs_dummy_write_verify)
 		libxfs_dinode_calc_crc(mp, iocur_top->data);
-	if (iocur_top->dquot_buf)
+	if (iocur_top->dquot_buf &&
+	    iocur_top->bp->b_ops->verify_write != xfs_dummy_write_verify)
 		xfs_update_cksum(iocur_top->data, sizeof(struct xfs_dqblk),
 				 XFS_DQUOT_CRC_OFF);
 	if (iocur_top->bbmap)
db/write.h +1 -1
index 31e2665..664ddcc 100644
@@ -20,5 +20,5 @@ struct field;
 
 extern void	write_init(void);
 extern void	write_block(const field_t *fields, int argc, char **argv);
-extern void	write_string(const field_t *fields, int argc, char **argv);
 extern void	write_struct(const field_t *fields, int argc, char **argv);
+extern void	write_string(const field_t *fields, int argc, char **argv);
man/man8/xfs_db.8 +22 -5
index 4d8d4ff..0764832 100644
@@ -87,16 +87,14 @@ or
 .I filename
 read-only. This option is required if the filesystem is mounted.
 It is only necessary to omit this flag if a command that changes data
-.RB ( write ", " blocktrash )
+.RB ( write ", " blocktrash ", " crc )
 is to be used.
 .TP
 .B \-x
 Specifies expert mode.
 This enables the
-.B write
-and
-.B blocktrash
-commands.
+.RB ( write ", " blocktrash ", " crc
+invalidate/revalidate) commands.
 .TP
 .B \-V
 Prints the version number and exits.

@@ -409,6 +407,25 @@ conversions such as
 .I agb
 .BR fsblock .
 .TP
+.B crc [\-i|\-r|\-v]
+Invalidates, revalidates, or validates the CRC (checksum)
+field of the current structure, if it has one.
+This command is available only on CRC-enabled filesystems.
+With no argument, validation is performed.
+Each command will display the resulting CRC value and state.
+.RS 1.0i
+.TP 0.4i
+.B \-i
+Invalidate the structure's CRC value (incrementing it by one),
+and write it to disk.
+.TP
+.B \-r
+Recalculate the current structure's correct CRC value, and write it to disk.
+.TP
+.B \-v
+Validate and display the current value and state of the structure's CRC.
+.RE
+.TP
 .BI "daddr [" d ]
 Set current address to the daddr (512 byte block) given by
 .IR d .

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#4 lore raw

Re: [PATCH 2/2] xfsprogs: xfs_db: add crc manipulation commands

Quoted 1 line · Eric Sandeen

This adds a new "crc" command to xfs_db for CRC-enabled filesystems.

Actually, with my other patch to allow "corrupt" writes from xfs_db, maybe a dedicated "crc" command isn't needed. What do folks think?

-Eric

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#5 lore raw

PATCH v2 1/2 xfs_db: fix inode CRC validity state, and warn on read if invalid

In reply to Eric Sandeen

Currently, the "ino_crc_ok" field on the io cursor reflects overall inode validity, not CRC correctness. Because it is only used when printing CRC validity, change it to reflect only that state - and update it whenever we re-write the inode (thus updating the CRC).

In addition, when reading an inode, warn if the CRC is bad.

Note, when specifying an inode which doesn't actually exist, this will claim corruption; I'm not sure if that's good or bad. Today, it already issues corruption errors on the way; this adds a new message as well:

xfs_db> inode 129
Metadata corruption detected at block 0x80/0x2000 Metadata corruption detected at block 0x80/0x2000
...
Metadata CRC error detected for ino 129

Signed-off-by Eric Sandeen <[email protected]>

---

V2: fix whitespace damage, introduce libxfs_verify_cksum

db/inode.c +6 -1
index 24170ba..982acb7 100644
@@ -684,13 +684,18 @@ set_cur_inode(
 		numblks, DB_RING_IGN, NULL);
 	off_cur(offset << mp->m_sb.sb_inodelog, mp->m_sb.sb_inodesize);
 	dip = iocur_top->data;
-	iocur_top->ino_crc_ok = libxfs_dinode_verify(mp, ino, dip);
+	iocur_top->ino_crc_ok = libxfs_verify_cksum((char *)dip,
+						    mp->m_sb.sb_inodesize,
+						    XFS_DINODE_CRC_OFF);
 	iocur_top->ino_buf = 1;
 	iocur_top->ino = ino;
 	iocur_top->mode = be16_to_cpu(dip->di_mode);
 	if ((iocur_top->mode & S_IFMT) == S_IFDIR)
 		iocur_top->dirino = ino;
 
+	if (xfs_sb_version_hascrc(&mp->m_sb) && !iocur_top->ino_crc_ok)
+		dbprintf(_("Metadata CRC error detected for ino %lld\n"), ino);
+
 	/* track updated info in ring */
 	ring_add();
 }
db/io.c +3 -1
index 7f1b76a..f7393c5 100644
@@ -464,8 +464,10 @@ write_cur(void)
 		return;
 	}
 
-	if (iocur_top->ino_buf)
+	if (iocur_top->ino_buf) {
 		libxfs_dinode_calc_crc(mp, iocur_top->data);
+		iocur_top->ino_crc_ok = 1;
+	}
 	if (iocur_top->dquot_buf)
 		xfs_update_cksum(iocur_top->data, sizeof(struct xfs_dqblk),
 				 XFS_DQUOT_CRC_OFF);
include/libxfs.h +2 -0
index 45a924f..962e319 100644
@@ -782,6 +782,8 @@ extern uint32_t crc32c_le(uint32_t crc, unsigned char const *p, size_t len);
 
 #include <xfs/xfs_cksum.h>
 
+#define libxfs_verify_cksum	xfs_verify_cksum
+
 static inline int
 xfs_buf_verify_cksum(struct xfs_buf *bp, unsigned long cksum_offset)
 {

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#6 lore raw

PATCH v2 2/2 xfs_db: add crc manipulation commands

In reply to Eric Sandeen

This adds a new "crc" command to xfs_db for CRC-enabled filesystems.

If a structure has a CRC field, we can validate it, invalidate/corrupt it, or revalidate/rewrite it:

xfs_db> sb 0
xfs_db> crc -v
crc = 0x796c814f (correct)
xfs_db> crc -i
Metadata CRC error detected at block 0x0/0x200
crc = 0x796c8150 (bad)
xfs_db> crc -r
crc = 0x796c814f (correct)

(-i and -r require "expert" write-capable mode)

This requires temporarily replacing the write verifier with a dummy which won't recalculate the CRC on the way to disk.

It also required me to write a new flist function, which is totally foreign to me, so hopefully done right - but it seems to work here.

Signed-off-by Eric Sandeen <[email protected]>

---

Note, "-i" and "-r" are shown in help even if not in expert mode, not sure if I should fix that?

V2: Fix whitespace damage, clarify write_cur() changes a bit w/ code & comments.

db/Makefile +1 -1
index bae6154..f0175cc 100644
@@ -8,7 +8,7 @@ include $(TOPDIR)/include/builddefs
 LTCOMMAND = xfs_db
 
 HFILES = addr.h agf.h agfl.h agi.h attr.h attrshort.h bit.h block.h bmap.h \
-	btblock.h bmroot.h check.h command.h convert.h debug.h \
+	btblock.h bmroot.h check.h command.h convert.h crc.h debug.h \
 	dir2.h dir2sf.h dquot.h echo.h faddr.h field.h \
 	flist.h fprint.h frag.h freesp.h hash.h help.h init.h inode.h input.h \
 	io.h malloc.h metadump.h output.h print.h quit.h sb.h sig.h strvec.h \
db/command.c +2 -0
index b7e3165..d44e0a5 100644
@@ -48,6 +48,7 @@
 #include "write.h"
 #include "malloc.h"
 #include "dquot.h"
+#include "crc.h"
 
 cmdinfo_t	*cmdtab;
 int		ncmds;

@@ -123,6 +124,7 @@ init_commands(void)
 	bmap_init();
 	check_init();
 	convert_init();
+	crc_init();
 	debug_init();
 	echo_init();
 	frag_init();
db/crc.c new file +189 -0

189 lines not on this page · raw message

db/crc.h new file +23 -0
new file mode 100644
index 0000000..80ecec3
@@ -0,0 +1,23 @@
+/*
+ * Copyright (c) 2014 Red Hat, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+struct field;
+
+extern void	crc_init(void);
+extern void	crc_struct(const field_t *fields, int argc, char **argv);
+extern void	xfs_dummy_verify(struct xfs_buf *bp);
db/flist.c +34 -0
index 33f7da7..fa19f70 100644
@@ -411,6 +411,40 @@ flist_split(
 	return v;
 }
 
+/*
+ * Given a set of fields, scan for a field of the given type.
+ * Return an flist leading to the first found field
+ * of that type.
+ * Return NULL if no field of the given type is found.
+ */
+flist_t *
+flist_find_ftyp(
+	const field_t *fields,
+	fldt_t	type)
+{
+	flist_t	*fl;
+	const field_t	*f;
+	const ftattr_t  *fa;
+
+	for (f = fields; f->name; f++) {
+		fl = flist_make(f->name);
+		fl->fld = f;
+		if (f->ftyp == type)
+			return fl;
+		fa = &ftattrtab[f->ftyp];
+		if (fa->subfld) {
+			flist_t *nfl;
+			nfl = flist_find_ftyp(fa->subfld, type);
+			if (nfl) {
+				fl->child = nfl;
+				return fl;
+			}
+		}
+		flist_free(fl);
+	}
+	return NULL;
+}
+
 static void
 ftok_free(
 	ftok_t	*ft)
db/flist.h +1 -0
index 5c9fba0..3f4b312 100644
@@ -37,3 +37,4 @@ extern int	flist_parse(const struct field *fields, flist_t *fl, void *obj,
 			    int startoff);
 extern void	flist_print(flist_t *fl);
 extern flist_t	*flist_scan(char *name);
+extern flist_t	*flist_find_ftyp(const field_t *fields, fldt_t  type);
db/io.c +18 -2
index f7393c5..eb3daa1 100644
@@ -27,6 +27,7 @@
 #include "output.h"
 #include "init.h"
 #include "malloc.h"
+#include "crc.h"
 
 static int	pop_f(int argc, char **argv);
 static void     pop_help(void);

@@ -459,22 +460,37 @@ write_cur_bbs(void)
 void
 write_cur(void)
 {
+	int skip_crc = (iocur_top->bp->b_ops->verify_write == xfs_dummy_verify);
+
 	if (iocur_sp < 0) {
 		dbprintf(_("nothing to write\n"));
 		return;
 	}
 
-	if (iocur_top->ino_buf) {
+	if (iocur_top->ino_buf && !skip_crc) {
 		libxfs_dinode_calc_crc(mp, iocur_top->data);
 		iocur_top->ino_crc_ok = 1;
 	}
-	if (iocur_top->dquot_buf)
+
+	if (iocur_top->dquot_buf && !skip_crc)
 		xfs_update_cksum(iocur_top->data, sizeof(struct xfs_dqblk),
 				 XFS_DQUOT_CRC_OFF);
 	if (iocur_top->bbmap)
 		write_cur_bbs();
 	else
 		write_cur_buf();
+
+	/* If we didn't write the crc automatically, re-check validity */
+	if (iocur_top->ino_buf && skip_crc) {
+		xfs_dinode_t	*dip;
+		xfs_ino_t	ino;
+
+		dip = iocur_top->data;
+		ino = iocur_top->ino;
+		iocur_top->ino_crc_ok = xfs_verify_cksum((char *)dip,
+						mp->m_sb.sb_inodesize,
+						XFS_DINODE_CRC_OFF);
+	} 
 }
 
 void
db/write.h +1 -1
index 31e2665..664ddcc 100644
@@ -20,5 +20,5 @@ struct field;
 
 extern void	write_init(void);
 extern void	write_block(const field_t *fields, int argc, char **argv);
-extern void	write_string(const field_t *fields, int argc, char **argv);
 extern void	write_struct(const field_t *fields, int argc, char **argv);
+extern void	write_string(const field_t *fields, int argc, char **argv);
man/man8/xfs_db.8 +22 -5
index 4d8d4ff..0764832 100644
@@ -87,16 +87,14 @@ or
 .I filename
 read-only. This option is required if the filesystem is mounted.
 It is only necessary to omit this flag if a command that changes data
-.RB ( write ", " blocktrash )
+.RB ( write ", " blocktrash ", " crc )
 is to be used.
 .TP
 .B \-x
 Specifies expert mode.
 This enables the
-.B write
-and
-.B blocktrash
-commands.
+.RB ( write ", " blocktrash ", " crc
+invalidate/revalidate) commands.
 .TP
 .B \-V
 Prints the version number and exits.

@@ -409,6 +407,25 @@ conversions such as
 .I agb
 .BR fsblock .
 .TP
+.B crc [\-i|\-r|\-v]
+Invalidates, revalidates, or validates the CRC (checksum)
+field of the current structure, if it has one.
+This command is available only on CRC-enabled filesystems.
+With no argument, validation is performed.
+Each command will display the resulting CRC value and state.
+.RS 1.0i
+.TP 0.4i
+.B \-i
+Invalidate the structure's CRC value (incrementing it by one),
+and write it to disk.
+.TP
+.B \-r
+Recalculate the current structure's correct CRC value, and write it to disk.
+.TP
+.B \-v
+Validate and display the current value and state of the structure's CRC.
+.RE
+.TP
 .BI "daddr [" d ]
 Set current address to the daddr (512 byte block) given by
 .IR d .

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#7 lore raw

Re: [PATCH 2/2 V2] xfs_db: add crc manipulation commands

Quoted 5 lines · Eric Sandeen

This adds a new "crc" command to xfs_db for CRC-enabled filesystems.

If a structure has a CRC field, we can validate it, invalidate/corrupt it, or revalidate/rewrite it:

Quoted 8 lines

xfs_db> sb 0
xfs_db> crc -v
crc = 0x796c814f (correct)
xfs_db> crc -i
Metadata CRC error detected at block 0x0/0x200
crc = 0x796c8150 (bad)
xfs_db> crc -r
crc = 0x796c814f (correct)

On reflection, do you think these should be more verbose, something like:

xfs_db> sb 0
xfs_db> crc -v
Validating crc:
crc = 0x796c814f (correct)
xfs_db> crc -i
Invalidating CRC:
Metadata CRC error detected at block 0x0/0x200
crc = 0x796c8150 (bad)
xfs_db> crc -r
Rewriting CRC:
crc = 0x796c814f (correct)

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#8 lore raw

Re: [PATCH 2/2 V2] xfs_db: add crc manipulation commands

Quoted 17 lines · Eric Sandeen
Quoted 5 lines · Eric Sandeen

This adds a new "crc" command to xfs_db for CRC-enabled filesystems.

If a structure has a CRC field, we can validate it, invalidate/corrupt it, or revalidate/rewrite it:

Quoted 8 lines

xfs_db> sb 0
xfs_db> crc -v
crc = 0x796c814f (correct)
xfs_db> crc -i
Metadata CRC error detected at block 0x0/0x200
crc = 0x796c8150 (bad)
xfs_db> crc -r
crc = 0x796c814f (correct)

On reflection, do you think these should be more verbose, something like:

Quoted 11 lines

xfs_db> sb 0
xfs_db> crc -v
Validating crc:
crc = 0x796c814f (correct)
xfs_db> crc -i
Invalidating CRC:
Metadata CRC error detected at block 0x0/0x200
crc = 0x796c8150 (bad)
xfs_db> crc -r
Rewriting CRC:
crc = 0x796c814f (correct)

Seems like a good idea to me. ;)

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#9 lore raw

PATCH v3 2/2 xfs_db: add crc manipulation commands

In reply to Eric Sandeen

This adds a new "crc" command to xfs_db for CRC-enabled filesystems.

If a structure has a CRC field, we can validate it, invalidate/corrupt it, or revalidate/rewrite it:

xfs_db> sb 0
xfs_db> crc -v
crc = 0x796c814f (correct)
xfs_db> crc -i
Metadata CRC error detected at block 0x0/0x200
crc = 0x796c8150 (bad)
xfs_db> crc -r
crc = 0x796c814f (correct)

(-i and -r require "expert" write-capable mode)

This requires temporarily replacing the write verifier with a dummy which won't recalculate the CRC on the way to disk.

It also required me to write a new flist function, which is totally foreign to me, so hopefully done right - but it seems to work here.

Signed-off-by Eric Sandeen <[email protected]>

---

Note, "-i" and "-r" are shown in help even if not in expert mode, not sure if I should fix that?

V2: Fix whitespace damage, clarify write_cur() changes a bit w/ code & comments.

V3: Be a bit more verbose with command output

I'm not totally sure on this, most xfs_db commands aren't very chatty. Pick the one you like ;)

db/Makefile +1 -1
index bae6154..f0175cc 100644
@@ -8,7 +8,7 @@ include $(TOPDIR)/include/builddefs
 LTCOMMAND = xfs_db
 
 HFILES = addr.h agf.h agfl.h agi.h attr.h attrshort.h bit.h block.h bmap.h \
-	btblock.h bmroot.h check.h command.h convert.h debug.h \
+	btblock.h bmroot.h check.h command.h convert.h crc.h debug.h \
 	dir2.h dir2sf.h dquot.h echo.h faddr.h field.h \
 	flist.h fprint.h frag.h freesp.h hash.h help.h init.h inode.h input.h \
 	io.h malloc.h metadump.h output.h print.h quit.h sb.h sig.h strvec.h \
db/command.c +2 -0
index b7e3165..d44e0a5 100644
@@ -48,6 +48,7 @@
 #include "write.h"
 #include "malloc.h"
 #include "dquot.h"
+#include "crc.h"
 
 cmdinfo_t	*cmdtab;
 int		ncmds;

@@ -123,6 +124,7 @@ init_commands(void)
 	bmap_init();
 	check_init();
 	convert_init();
+	crc_init();
 	debug_init();
 	echo_init();
 	frag_init();
db/crc.c new file +195 -0

195 lines not on this page · raw message

db/crc.h new file +23 -0
new file mode 100644
index 0000000..80ecec3
@@ -0,0 +1,23 @@
+/*
+ * Copyright (c) 2014 Red Hat, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+struct field;
+
+extern void	crc_init(void);
+extern void	crc_struct(const field_t *fields, int argc, char **argv);
+extern void	xfs_dummy_verify(struct xfs_buf *bp);
db/flist.c +34 -0
index 33f7da7..fa19f70 100644
@@ -411,6 +411,40 @@ flist_split(
 	return v;
 }
 
+/*
+ * Given a set of fields, scan for a field of the given type.
+ * Return an flist leading to the first found field
+ * of that type.
+ * Return NULL if no field of the given type is found.
+ */
+flist_t *
+flist_find_ftyp(
+	const field_t *fields,
+	fldt_t	type)
+{
+	flist_t	*fl;
+	const field_t	*f;
+	const ftattr_t  *fa;
+
+	for (f = fields; f->name; f++) {
+		fl = flist_make(f->name);
+		fl->fld = f;
+		if (f->ftyp == type)
+			return fl;
+		fa = &ftattrtab[f->ftyp];
+		if (fa->subfld) {
+			flist_t *nfl;
+			nfl = flist_find_ftyp(fa->subfld, type);
+			if (nfl) {
+				fl->child = nfl;
+				return fl;
+			}
+		}
+		flist_free(fl);
+	}
+	return NULL;
+}
+
 static void
 ftok_free(
 	ftok_t	*ft)
db/flist.h +1 -0
index 5c9fba0..3f4b312 100644
@@ -37,3 +37,4 @@ extern int	flist_parse(const struct field *fields, flist_t *fl, void *obj,
 			    int startoff);
 extern void	flist_print(flist_t *fl);
 extern flist_t	*flist_scan(char *name);
+extern flist_t	*flist_find_ftyp(const field_t *fields, fldt_t  type);
db/io.c +18 -2
index f7393c5..eb3daa1 100644
@@ -27,6 +27,7 @@
 #include "output.h"
 #include "init.h"
 #include "malloc.h"
+#include "crc.h"
 
 static int	pop_f(int argc, char **argv);
 static void     pop_help(void);

@@ -459,22 +460,37 @@ write_cur_bbs(void)
 void
 write_cur(void)
 {
+	int skip_crc = (iocur_top->bp->b_ops->verify_write == xfs_dummy_verify);
+
 	if (iocur_sp < 0) {
 		dbprintf(_("nothing to write\n"));
 		return;
 	}
 
-	if (iocur_top->ino_buf) {
+	if (iocur_top->ino_buf && !skip_crc) {
 		libxfs_dinode_calc_crc(mp, iocur_top->data);
 		iocur_top->ino_crc_ok = 1;
 	}
-	if (iocur_top->dquot_buf)
+
+	if (iocur_top->dquot_buf && !skip_crc)
 		xfs_update_cksum(iocur_top->data, sizeof(struct xfs_dqblk),
 				 XFS_DQUOT_CRC_OFF);
 	if (iocur_top->bbmap)
 		write_cur_bbs();
 	else
 		write_cur_buf();
+
+	/* If we didn't write the crc automatically, re-check validity */
+	if (iocur_top->ino_buf && skip_crc) {
+		xfs_dinode_t	*dip;
+		xfs_ino_t	ino;
+
+		dip = iocur_top->data;
+		ino = iocur_top->ino;
+		iocur_top->ino_crc_ok = xfs_verify_cksum((char *)dip,
+						mp->m_sb.sb_inodesize,
+						XFS_DINODE_CRC_OFF);
+	} 
 }
 
 void
db/write.h +1 -1
index 31e2665..664ddcc 100644
@@ -20,5 +20,5 @@ struct field;
 
 extern void	write_init(void);
 extern void	write_block(const field_t *fields, int argc, char **argv);
-extern void	write_string(const field_t *fields, int argc, char **argv);
 extern void	write_struct(const field_t *fields, int argc, char **argv);
+extern void	write_string(const field_t *fields, int argc, char **argv);
man/man8/xfs_db.8 +22 -5
index 4d8d4ff..0764832 100644
@@ -87,16 +87,14 @@ or
 .I filename
 read-only. This option is required if the filesystem is mounted.
 It is only necessary to omit this flag if a command that changes data
-.RB ( write ", " blocktrash )
+.RB ( write ", " blocktrash ", " crc )
 is to be used.
 .TP
 .B \-x
 Specifies expert mode.
 This enables the
-.B write
-and
-.B blocktrash
-commands.
+.RB ( write ", " blocktrash ", " crc
+invalidate/revalidate) commands.
 .TP
 .B \-V
 Prints the version number and exits.

@@ -409,6 +407,25 @@ conversions such as
 .I agb
 .BR fsblock .
 .TP
+.B crc [\-i|\-r|\-v]
+Invalidates, revalidates, or validates the CRC (checksum)
+field of the current structure, if it has one.
+This command is available only on CRC-enabled filesystems.
+With no argument, validation is performed.
+Each command will display the resulting CRC value and state.
+.RS 1.0i
+.TP 0.4i
+.B \-i
+Invalidate the structure's CRC value (incrementing it by one),
+and write it to disk.
+.TP
+.B \-r
+Recalculate the current structure's correct CRC value, and write it to disk.
+.TP
+.B \-v
+Validate and display the current value and state of the structure's CRC.
+.RE
+.TP
 .BI "daddr [" d ]
 Set current address to the daddr (512 byte block) given by
 .IR d .

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#10 lore raw
In reply to Eric Sandeen

Ping on these. They still apply & build cleanly.

Thanks,
-Eric

Quoted 6 lines · Eric Sandeen

2 patches related to crcs in xfs_db,

1) display inode CRC status when read, and revalidate it when written
2) add a new "crc" command to validate, invalidate, or revalidate the CRC on a disk structure

Quoted 8 lines

thanks,
-Eric

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs