[PATCH v2] xfsdump: handle dump files with checksum bug

[Bill Kendall <wkendall@xxxxxxx>]

xfsdump previously contained a bug in the code which generated
a checksum on the header for extended attributes. This bug
was recently fixed, but a new xfsrestore will fail if it
encounters an old dump file which had checksums enabled. (This
is unlikely since checksums have just recently been enabled in
the build, and the above-mentioned bug was fixed at the same time.)

This patch uses a new flag in an extattrhdr_t to indicate a
checksum is present. If this is set, the checksum is validated.
If instead the old checksum flag is set, a warning is issued saying
the header could not be validated, and xfsrestore will assume the
header is valid.

Note that with this change a new dump cannot be restored with an
old restore which has checksums enabled. But as I mentioned, old
restores do not have checksums enabled.

Signed-off-by: Bill Kendall <wkendall@xxxxxxx>
---
Changes in v2:
    - reworded the warning message
    - moved the definition of 'warned' to the block where it is used

 common/content_inode.h |   10 +++++++---
 restore/content.c      |   25 +++++++++++++++++++------
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/common/content_inode.h b/common/content_inode.h
index 0f21840..85e60df 100644
--- a/common/content_inode.h
+++ b/common/content_inode.h
@@ -339,13 +339,17 @@ typedef struct extattrhdr extattrhdr_t;
 #define EXTATTRHDR_FLAGS_NULL		( 1 << 1 )
 	/* marks the end of the attributes associated with the leading filehdr_t
 	 */
-#define EXTATTRHDR_FLAGS_CHECKSUM	( 1 << 2 )
-	/* checksum is present
+#define EXTATTRHDR_FLAGS_OLD_CHECKSUM	( 1 << 2 )
+	/* old xfsdumps used this flag to indicate a checksum is present,
+	 * but the checksum was not calculated properly. the presence of
+	 * this flag now indicates a checksum that cannot be verified.
 	 */
-
 #define EXTATTRHDR_FLAGS_SECURE		( 1 << 3 )
 	/* a linux "secure" mode attribute
 	 */
+#define EXTATTRHDR_FLAGS_CHECKSUM	( 1 << 4 )
+	/* checksum is present.
+	 */
 
 /* Routines for calculating and validating checksums on xfsdump headers.
  * The header length must be an integral number of u_int32_t's.
diff --git a/restore/content.c b/restore/content.c
index a98a9c7..9ff5b93 100644
--- a/restore/content.c
+++ b/restore/content.c
@@ -8197,16 +8197,29 @@ read_extattrhdr( drive_t *drivep, extattrhdr_t *ahdrp, bool_t ahcs )
 	      ahdrp->ah_checksum );
 
 	if ( ahcs ) {
-		if ( ! ( ahdrp->ah_flags & EXTATTRHDR_FLAGS_CHECKSUM )) {
+		if ( ahdrp->ah_flags & EXTATTRHDR_FLAGS_CHECKSUM ) {
+			if ( !is_checksum_valid( ahdrp, EXTATTRHDR_SZ )) {
+				mlog( MLOG_NORMAL | MLOG_WARNING, _(
+					"bad extattr header checksum\n") );
+				return RV_CORRUPT;
+			}
+		} else if ( ahdrp->ah_flags & EXTATTRHDR_FLAGS_OLD_CHECKSUM ) {
+			/* possibly a corrupt header, but most likely an old
+			 * header, which cannot be verified due to a bug in how
+			 * its checksum was calculated.
+			 */
+			static bool_t warned = BOOL_FALSE;
+			if ( !warned ) {
+				mlog( MLOG_NORMAL | MLOG_WARNING, _(
+					"ignoring old-style extattr "
+					"header checksums\n") );
+				warned = BOOL_TRUE;
+			}
+		} else {
 			mlog( MLOG_NORMAL | MLOG_WARNING, _(
 			      "corrupt extattr header\n") );
 			return RV_CORRUPT;
 		}
-		if ( !is_checksum_valid( ahdrp, EXTATTRHDR_SZ )) {
-			mlog( MLOG_NORMAL | MLOG_WARNING, _(
-			      "bad extattr header checksum\n") );
-			return RV_CORRUPT;
-		}
 	}
 
 	return RV_OK;
-- 
1.7.0.4

_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs