xfsprogs: mini patch-bomb

l_logsize is only ever set, and is never read.
Remove it.

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

---

 db/sb.c             |    1 -
 include/libxlog.h   |    1 -
 logprint/logprint.c |    1 -
 repair/phase2.c     |    1 -
 4 files changed, 0 insertions(+), 4 deletions(-)

db/sb.c +0 -1

index cd12f83..991478d 100644
@@ -251,7 +251,6 @@ sb_logcheck(void)
 		x.lbsize <<= (mp->m_sb.sb_logsectlog - BBSHIFT);
 
 	log.l_dev = mp->m_logdev_targp;
-	log.l_logsize = BBTOB(log.l_logBBsize);
 	log.l_logBBsize = x.logBBsize;
 	log.l_logBBstart = x.logBBstart;
 	log.l_sectBBsize  = BTOBB(x.lbsize);

include/libxlog.h +0 -1

index d6ba9a5..e6d915c 100644
@@ -28,7 +28,6 @@ struct xlog {
 	xfs_mount_t	*l_mp;	        /* mount point */
 	struct xfs_buftarg *l_dev;	        /* dev_t of log */
 	xfs_daddr_t	l_logBBstart;   /* start block of log */
-	int		l_logsize;      /* size of log in bytes */
 	int		l_logBBsize;    /* size of log in 512 byte chunks */
 	int		l_curr_cycle;   /* Cycle number of log writes */
 	int		l_prev_cycle;   /* Cycle # b4 last block increment */

logprint/logprint.c +0 -1

index 960a267..fdf106b 100644
@@ -242,7 +242,6 @@ main(int argc, char **argv)
 	ASSERT(x.logBBsize <= INT_MAX);
 
 	log.l_dev = mount.m_logdev_targp;
-	log.l_logsize     = BBTOB(x.logBBsize);
 	log.l_logBBstart  = x.logBBstart;
 	log.l_logBBsize   = x.logBBsize;
 	log.l_sectBBsize  = BTOBB(x.lbsize);

repair/phase2.c +0 -2

index 0192346..983aaa7 100644
@@ -50,7 +50,6 @@ zero_log(xfs_mount_t *mp)
 		x.lbsize <<= (mp->m_sb.sb_logsectlog - BBSHIFT);
 
 	log.l_dev = mp->m_logdev_targp;
-	log.l_logsize = BBTOB(x.logBBsize);
 	log.l_logBBsize = x.logBBsize;
 	log.l_logBBstart = x.logBBstart;
 	log.l_sectBBsize  = BTOBB(x.lbsize);
-- 1.7.1

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

xfs_repair and xfs_db both check for a dirty log, using roughly the same cut and pasted code.

Add a new helper to do this, xlog_is_dirty(), and use it instead.

Note, the helper (and the code before it) is a little odd in that it (re-)initializes some libxfs_init_t members before proceeding, I haven't yet worked out if that's necessary or correct, so I've just kept the same behavior for now. Still, it seems like this should be done in libxfs_init, or not at all.

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

---

 db/sb.c           |   21 ++++---------------
 include/libxlog.h |    2 +
 libxlog/util.c    |   56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 repair/phase2.c   |   55 +++++++++++----------------------------------------
 4 files changed, 75 insertions(+), 59 deletions(-)

db/sb.c +7 -18

index 991478d..f9d39e6 100644
@@ -225,8 +225,7 @@ int xlog_recover_do_trans(struct xlog *log, xlog_recover_t *t, int p)
 int
 sb_logcheck(void)
 {
-	struct xlog	log;
-	xfs_daddr_t	head_blk, tail_blk;
+	int		dirty;
 
 	if (mp->m_sb.sb_logstart) {
 		if (x.logdev && x.logdev != x.ddev) {

@@ -242,25 +241,14 @@ sb_logcheck(void)
 		}
 	}
 
-	memset(&log, 0, sizeof(log));
 	libxfs_buftarg_init(mp, x.ddev, x.logdev, x.rtdev);
-	x.logBBsize = XFS_FSB_TO_BB(mp, mp->m_sb.sb_logblocks);
-	x.logBBstart = XFS_FSB_TO_DADDR(mp, mp->m_sb.sb_logstart);
-	x.lbsize = BBSIZE;
-	if (xfs_sb_version_hassector(&mp->m_sb))
-		x.lbsize <<= (mp->m_sb.sb_logsectlog - BBSHIFT);
-
-	log.l_dev = mp->m_logdev_targp;
-	log.l_logBBsize = x.logBBsize;
-	log.l_logBBstart = x.logBBstart;
-	log.l_sectBBsize  = BTOBB(x.lbsize);
-	log.l_mp = mp;
-
-	if (xlog_find_tail(&log, &head_blk, &tail_blk)) {
+
+	dirty = xlog_is_dirty(mp, &x, 0);
+
+	if (dirty == -1) {
 		dbprintf(_("ERROR: cannot find log head/tail, run xfs_repair\n"));
 		return 0;
-	}
-	if (head_blk != tail_blk) {
+	} else if (dirty == 1) {
 		dbprintf(_(
 "ERROR: The filesystem has valuable metadata changes in a log which needs to\n"
 "be replayed.  Mount the filesystem to replay the log, and unmount it before\n"

@@ -270,6 +258,7 @@ sb_logcheck(void)
 "of the filesystem before doing this.\n"), progname);
 		return 0;
 	}
+	/* Log is clean */
 	return 1;
 }
 

include/libxlog.h +2 -0

index e6d915c..c6640a8 100644
@@ -83,6 +83,8 @@ extern int	print_record_header;
 /* libxfs parameters */
 extern libxfs_init_t	x;
 
+
+extern int xlog_is_dirty(xfs_mount_t *mp, libxfs_init_t *x, int verbose);
 extern struct xfs_buf *xlog_get_bp(struct xlog *, int);
 extern void	xlog_put_bp(struct xfs_buf *);
 extern int	xlog_bread(struct xlog *log, xfs_daddr_t blk_no, int nbblks,

libxlog/util.c +56 -0

index 053cf04..498c06c 100644
@@ -24,6 +24,62 @@ int print_skip_uuid;
 int print_record_header;
 libxfs_init_t x;
 
+/*
+ * Return 1 for dirty, 0 for clean, -1 for errors
+ */
+int
+xlog_is_dirty(
+	xfs_mount_t *mp,
+	libxfs_init_t *x,
+	int verbose)
+{
+	int error;
+	struct xlog	log;
+	xfs_daddr_t head_blk, tail_blk;
+
+	memset(&log, 0, sizeof(log));
+
+	/* We (re-)init members of libxfs_init_t here?  really? */
+	x->logBBsize = XFS_FSB_TO_BB(mp, mp->m_sb.sb_logblocks);
+	x->logBBstart = XFS_FSB_TO_DADDR(mp, mp->m_sb.sb_logstart);
+	x->lbsize = BBSIZE;
+	if (xfs_sb_version_hassector(&mp->m_sb))
+		x->lbsize <<= (mp->m_sb.sb_logsectlog - BBSHIFT);
+
+	log.l_dev = mp->m_logdev_targp;
+	log.l_logBBsize = x->logBBsize;
+	log.l_logBBstart = x->logBBstart;
+	log.l_sectBBsize = BTOBB(x->lbsize);
+	log.l_mp = mp;
+	if (xfs_sb_version_hassector(&mp->m_sb)) {
+		log.l_sectbb_log = mp->m_sb.sb_logsectlog - BBSHIFT;
+		ASSERT(log.l_sectbb_log <= mp->m_sectbb_log);
+		/* for larger sector sizes, must have v2 or external log */
+		ASSERT(log.l_sectbb_log == 0 ||
+			log.l_logBBstart == 0 ||
+			xfs_sb_version_haslogv2(&mp->m_sb));
+		ASSERT(mp->m_sb.sb_logsectlog >= BBSHIFT);
+	}
+	log.l_sectbb_mask = (1 << log.l_sectbb_log) - 1;
+
+	if ((error = xlog_find_tail(&log, &head_blk, &tail_blk))) {
+		xlog_warn(_("%s: cannot find log head/tail "
+			  "(xlog_find_tail=%d)\n"),
+			__func__, error);
+		return -1;
+	}
+
+	if (verbose)
+		xlog_warn(
+	_("%s: head block %" PRId64 " tail block %" PRId64 "\n"),
+			__func__, head_blk, tail_blk);
+
+	if (head_blk != tail_blk)
+		return 1;
+
+	return 0;
+}
+
 static int
 header_check_uuid(xfs_mount_t *mp, xlog_rec_header_t *head)
 {

repair/phase2.c +14 -46

index 983aaa7..a23f472 100644
@@ -38,62 +38,31 @@ int xlog_recover_do_trans(struct xlog *log, xlog_recover_t *t, int p)
 static void
 zero_log(xfs_mount_t *mp)
 {
-	int error;
-	struct xlog	log;
-	xfs_daddr_t head_blk, tail_blk;
-
-	memset(&log, 0, sizeof(log));
-	x.logBBsize = XFS_FSB_TO_BB(mp, mp->m_sb.sb_logblocks);
-	x.logBBstart = XFS_FSB_TO_DADDR(mp, mp->m_sb.sb_logstart);
-	x.lbsize = BBSIZE;
-	if (xfs_sb_version_hassector(&mp->m_sb))
-		x.lbsize <<= (mp->m_sb.sb_logsectlog - BBSHIFT);
-
-	log.l_dev = mp->m_logdev_targp;
-	log.l_logBBsize = x.logBBsize;
-	log.l_logBBstart = x.logBBstart;
-	log.l_sectBBsize  = BTOBB(x.lbsize);
-	log.l_mp = mp;
-	if (xfs_sb_version_hassector(&mp->m_sb)) {
-		log.l_sectbb_log = mp->m_sb.sb_logsectlog - BBSHIFT;
-		ASSERT(log.l_sectbb_log <= mp->m_sectbb_log);
-		/* for larger sector sizes, must have v2 or external log */
-		ASSERT(log.l_sectbb_log == 0 ||
-			log.l_logBBstart == 0 ||
-			xfs_sb_version_haslogv2(&mp->m_sb));
-		ASSERT(mp->m_sb.sb_logsectlog >= BBSHIFT);
-	}
-	log.l_sectbb_mask = (1 << log.l_sectbb_log) - 1;
-
-	if ((error = xlog_find_tail(&log, &head_blk, &tail_blk))) {
-		do_warn(_("zero_log: cannot find log head/tail "
-			  "(xlog_find_tail=%d), zeroing it anyway\n"),
-			error);
-	} else {
-		if (verbose) {
-			do_warn(
-	_("zero_log: head block %" PRId64 " tail block %" PRId64 "\n"),
-				head_blk, tail_blk);
-		}
-		if (head_blk != tail_blk) {
-			if (zap_log) {
-				do_warn(_(
+	int		dirty;
+
+	dirty = xlog_is_dirty(mp, &x, verbose);
+
+	if (dirty == -1)
+		do_warn(_("zero_log: cannot find log head/tail, "
+			  "zeroing it anyway\n"));
+	else if (dirty == 1) {
+		if (zap_log) {
+			do_warn(_(
 "ALERT: The filesystem has valuable metadata changes in a log which is being\n"
 "destroyed because the -L option was used.\n"));
-			} else {
-				do_warn(_(
+		} else {
+			do_warn(_(
 "ERROR: The filesystem has valuable metadata changes in a log which needs to\n"
 "be replayed.  Mount the filesystem to replay the log, and unmount it before\n"
 "re-running xfs_repair.  If you are unable to mount the filesystem, then use\n"
 "the -L option to destroy the log and attempt a repair.\n"
 "Note that destroying the log may cause corruption -- please attempt a mount\n"
 "of the filesystem before doing this.\n"));
-				exit(2);
-			}
+			exit(2);
 		}
 	}
 
-	libxfs_log_clear(log.l_dev,
+	libxfs_log_clear(mp->m_logdev_targp,
 		XFS_FSB_TO_DADDR(mp, mp->m_sb.sb_logstart),
 		(xfs_extlen_t)XFS_FSB_TO_BB(mp, mp->m_sb.sb_logblocks),
 		&mp->m_sb.sb_uuid,
-- 1.7.1

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

Seeing "if (!dont_obfuscate)" hurts my brain; rename it to "obfuscate" and invert the logic.

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

---

 db/metadump.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

db/metadump.c +7 -8

index b227a6d..57b53d9 100644
@@ -74,7 +74,7 @@ static xfs_ino_t	cur_ino;
 static int		show_progress = 0;
 static int		stop_on_read_error = 0;
 static int		max_extent_size = DEFAULT_MAX_EXT_SIZE;
-static int		dont_obfuscate = 0;
+static int		obfuscate = 1;
 static int		show_warnings = 0;
 static int		progress_since_warning = 0;
 

@@ -1368,7 +1368,7 @@ process_single_fsb_objects(
 
 		}
 
-		if (dont_obfuscate)
+		if (!obfuscate)
 			goto write;
 
 		dp = iocur_top->data;

@@ -1460,7 +1460,7 @@ process_multi_fsb_objects(
 
 			}
 
-			if (dont_obfuscate || o >= mp->m_dir_geo->leafblk) {
+			if (!obfuscate || o >= mp->m_dir_geo->leafblk) {
 				ret = write_buf(iocur_top);
 				goto out_pop;
 			}

@@ -1725,7 +1725,7 @@ process_inode_data(
 {
 	switch (dip->di_format) {
 		case XFS_DINODE_FMT_LOCAL:
-			if (!dont_obfuscate)
+			if (obfuscate)
 				switch (itype) {
 					case TYP_DIR2:
 						obfuscate_sf_dir(dip);

@@ -1769,7 +1769,7 @@ process_inode(
 	cur_ino = XFS_AGINO_TO_INO(mp, agno, agino);
 
 	/* we only care about crc recalculation if we are obfuscating names. */
-	if (!dont_obfuscate) {
+	if (obfuscate) {
 		crc_was_ok = xfs_verify_cksum((char *)dip,
 					mp->m_sb.sb_inodesize,
 					offsetof(struct xfs_dinode, di_crc));

@@ -1801,7 +1801,7 @@ process_inode(
 		switch (dip->di_aformat) {
 			case XFS_DINODE_FMT_LOCAL:
 				need_new_crc = 1;
-				if (!dont_obfuscate)
+				if (obfuscate)
 					obfuscate_sf_attr(dip);
 				break;
 

@@ -2247,7 +2247,7 @@ metadump_f(
 				}
 				break;
 			case 'o':
-				dont_obfuscate = 1;
+				obfuscate = 0;
 				break;
 			case 'w':
 				show_warnings = 1;
-- 1.7.1

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

When doing an xfs_metadump, if the log is clean, zero it out for 2 reasons:

* It'll make the image more compressible
* It'll eliminate an un-obfuscated metadata source

If the log isn't clean, and the user expected obfuscation, warn that metadata in the log will not be obfuscated.

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

---

db/metadump.c +3 -0

index bea4e00..eb5e9da 100644
@@ -17,6 +17,7 @@
  */
 
 #include <libxfs.h>
+#include <libxlog.h>
 #include "bmap.h"
 #include "command.h"
 #include "metadump.h"

@@ -2169,6 +2170,8 @@ copy_sb_inodes(void)
 static int
 copy_log(void)
 {
+	int dirty;
+
 	if (show_progress)
 		print_progress("Copying log");
 

@@ -2180,6 +2183,31 @@ copy_log(void)
 		print_warning("cannot read log");
 		return !stop_on_read_error;
 	}

db/metadump.c +15 -0

+		break;
+	case 1:
+		/* keep the dirty log */
+		if (obfuscate)
+			print_warning(
+_("Filesystem log is dirty; image will contain unobfuscated metadata in log."));
+		break;
+	case -1:
+		/* log detection error */
+		if (obfuscate)
+			print_warning(
+_("Could not discern log; image will contain unobfuscated metadata in log."));
+		break;
+	}
+
 	return !write_buf(iocur_top);
 }
 

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

Quoted 37 lines · Eric Sandeen

When doing an xfs_metadump, if the log is clean, zero it out for 2 reasons:

* It'll make the image more compressible
* It'll eliminate an un-obfuscated metadata source

If the log isn't clean, and the user expected obfuscation, warn that metadata in the log will not be obfuscated.

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

---

db/metadump.c +3 -0

index bea4e00..eb5e9da 100644
@@ -17,6 +17,7 @@
  */
 
 #include <libxfs.h>
+#include <libxlog.h>
 #include "bmap.h"
 #include "command.h"
 #include "metadump.h"

@@ -2169,6 +2170,8 @@ copy_sb_inodes(void)
 static int
 copy_log(void)
 {
+	int dirty;
+
 	if (show_progress)
 		print_progress("Copying log");
 

@@ -2180,6 +2183,31 @@ copy_log(void)
 		print_warning("cannot read log");
 		return !stop_on_read_error;
 	}

db/metadump.c copy_log(void) · line 2195

Quoted 10 lines

db/metadump.c +10 -0

+
+	dirty = xlog_is_dirty(mp, &x, 0);
+
+	switch (dirty) {
+	case 0:
+		/* clear out a clean log */
+		if (show_progress)
+			print_progress("Zeroing clean log");
+		memset(iocur_top->data, 0,
+			mp->m_sb.sb_logblocks * mp->m_sb.sb_blocksize);

Hmm, so is there any reason in the future we might want a metadump with a clean log matching what is actually on-disk? This is a debug tool after all. Perhaps this is mainly covered by the unclean and/or error cases, but that still seems like a potential loss of capability.

Anyways, I'm wondering if we should have an 'if (obfuscate)' here. Thoughts?

When doing an obfuscated xfs_metadump, if the log is clean, zero it out for 2 reasons:

* It'll make the image more compressible
* It'll eliminate an un-obfuscated metadata source

If the log isn't clean, and the user expected obfuscation, warn that metadata in the log will not be obfuscated.

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

---

V2: If not obfuscating, copy log as-is

db/metadump.c +11 -0

index bea4e00..67de0c5 100644
@@ -17,6 +17,7 @@
  */
 
 #include <libxfs.h>
+#include <libxlog.h>
 #include "bmap.h"
 #include "command.h"
 #include "metadump.h"

@@ -2169,6 +2170,8 @@ copy_sb_inodes(void)
 static int
 copy_log(void)
 {
+	int dirty;
+
 	if (show_progress)
 		print_progress("Copying log");
 

@@ -2180,6 +2183,36 @@ copy_log(void)
 		print_warning("cannot read log");
 		return !stop_on_read_error;
 	}
+
+	/* If not obfuscating, just copy the log as it is */
+	if (!obfuscate)
+		goto done;
+
+	dirty = xlog_is_dirty(mp, &x, 0);
+
+	switch (dirty) {

db/metadump.c +12 -0

+			print_warning(
+_("Filesystem log is dirty; image will contain unobfuscated metadata in log."));
+		break;
+	case -1:
+		/* log detection error */
+		if (obfuscate)
+			print_warning(
+_("Could not discern log; image will contain unobfuscated metadata in log."));
+		break;
+	}
+
+done:
 	return !write_buf(iocur_top);
 }
 

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

Quoted 9 lines · Eric Sandeen

When doing an obfuscated xfs_metadump, if the log is clean, zero it out for 2 reasons:

* It'll make the image more compressible
* It'll eliminate an un-obfuscated metadata source

If the log isn't clean, and the user expected obfuscation, warn that metadata in the log will not be obfuscated.

Quoted 5 lines

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

---

V2: If not obfuscating, copy log as-is

FYI, I think v1 of this was already pulled into the for-next branch. Anyways...

Quoted 38 lines

db/metadump.c +16 -0

index bea4e00..67de0c5 100644
@@ -17,6 +17,7 @@
  */
 
 #include <libxfs.h>
+#include <libxlog.h>
 #include "bmap.h"
 #include "command.h"
 #include "metadump.h"

@@ -2169,6 +2170,8 @@ copy_sb_inodes(void)
 static int
 copy_log(void)
 {
+	int dirty;
+
 	if (show_progress)
 		print_progress("Copying log");
 

@@ -2180,6 +2183,36 @@ copy_log(void)
 		print_warning("cannot read log");
 		return !stop_on_read_error;
 	}
+
+	/* If not obfuscating, just copy the log as it is */
+	if (!obfuscate)
+		goto done;
+
+	dirty = xlog_is_dirty(mp, &x, 0);
+
+	switch (dirty) {
+	case 0:
+		/* clear out a clean log */
+		if (show_progress)
+			print_progress("Zeroing clean log");
+		memset(iocur_top->data, 0,

db/metadump.c copy_log(void) · line 2203

Quoted 5 lines

db/metadump.c +5 -0

+			mp->m_sb.sb_logblocks * mp->m_sb.sb_blocksize);
+		break;
+	case 1:
+		/* keep the dirty log */
+		if (obfuscate)

The 'if (obfuscate)' checks here and below are pointless with the !obfuscate bypass above. Seems fine otherwise.

When doing an obfuscated xfs_metadump, if the log is clean, zero it out for 2 reasons:

* It'll make the image more compressible
* It'll eliminate an un-obfuscated metadata source

If the log isn't clean, and the user expected obfuscation, warn that metadata in the log will not be obfuscated.

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

---

V2: If not obfuscating, copy log as-is
V3: Remove pointless "obfuscate" checks after goto

db/metadump.c +31 -0

index bea4e00..bdc48a0 100644
@@ -17,6 +17,7 @@
  */
 
 #include <libxfs.h>
+#include <libxlog.h>
 #include "bmap.h"
 #include "command.h"
 #include "metadump.h"

@@ -2169,6 +2170,8 @@ copy_sb_inodes(void)
 static int
 copy_log(void)
 {
+	int dirty;
+
 	if (show_progress)
 		print_progress("Copying log");
 

@@ -2180,6 +2183,34 @@ copy_log(void)
 		print_warning("cannot read log");
 		return !stop_on_read_error;
 	}
+
+	/* If not obfuscating, just copy the log as it is */
+	if (!obfuscate)
+		goto done;
+
+	dirty = xlog_is_dirty(mp, &x, 0);
+
+	switch (dirty) {
+	case 0:
+		/* clear out a clean log */
+		if (show_progress)
+			print_progress("Zeroing clean log");
+		memset(iocur_top->data, 0,
+			mp->m_sb.sb_logblocks * mp->m_sb.sb_blocksize);
+		break;
+	case 1:
+		/* keep the dirty log */
+		print_warning(
+_("Filesystem log is dirty; image will contain unobfuscated metadata in log."));
+		break;
+	case -1:
+		/* log detection error */
+		print_warning(
+_("Could not discern log; image will contain unobfuscated metadata in log."));
+		break;
+	}
+
+done:
 	return !write_buf(iocur_top);
 }
 

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

Quoted 13 lines · Brian Foster

Quoted 12 lines · Eric Sandeen

When doing an obfuscated xfs_metadump, if the log is clean, zero it out for 2 reasons:

* It'll make the image more compressible
* It'll eliminate an un-obfuscated metadata source

If the log isn't clean, and the user expected obfuscation, warn that metadata in the log will not be obfuscated.

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

---

Quoted 5 lines

Quoted 2 lines

V2: If not obfuscating, copy log as-is

FYI, I think v1 of this was already pulled into the for-next branch. Anyways...

Yeah, I noticed - I'll ask dave to fix it (that one rebases, right?) :)