[PATCH 0/4] xfsprogs: mini patch-bomb

[PATCH 0/4] xfsprogs: mini patch-bomb

[Eric Sandeen]

This short series is aimed at cleaning up xfs_metadump's log handling,
so that if the log is clean, it's zeroed out and not included, and if
it's not clean, the user is alerted that metadata in the log won't be
obfuscated.

We get there by first making a common log helper, and converting xfs_repair
and xfs_db to use it as well.

Thanks,
-Eric

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

[PATCH 1/4] xfsprogs: remove unused write-only var l_logsize

[Eric Sandeen]

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

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 db/sb.c             |    1 -
 include/libxlog.h   |    1 -
 logprint/logprint.c |    1 -
 repair/phase2.c     |    1 -
 4 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/db/sb.c b/db/sb.c
index cd12f83..991478d 100644
--- a/db/sb.c
+++ b/db/sb.c
@@ -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);
diff --git a/include/libxlog.h b/include/libxlog.h
index d6ba9a5..e6d915c 100644
--- a/include/libxlog.h
+++ b/include/libxlog.h
@@ -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 */
diff --git a/logprint/logprint.c b/logprint/logprint.c
index 960a267..fdf106b 100644
--- a/logprint/logprint.c
+++ b/logprint/logprint.c
@@ -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);
diff --git a/repair/phase2.c b/repair/phase2.c
index 0192346..983aaa7 100644
--- a/repair/phase2.c
+++ b/repair/phase2.c
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/4] xfsprogs: add xlog_is_empty() helper

[Eric Sandeen]

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 <sandeen@redhat.com>
---
 db/sb.c           |   21 ++++---------------
 include/libxlog.h |    2 +
 libxlog/util.c    |   56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 repair/phase2.c   |   55 +++++++++++----------------------------------------
 4 files changed, 75 insertions(+), 59 deletions(-)

diff --git a/db/sb.c b/db/sb.c
index 991478d..f9d39e6 100644
--- a/db/sb.c
+++ b/db/sb.c
@@ -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;
 }
 
diff --git a/include/libxlog.h b/include/libxlog.h
index e6d915c..c6640a8 100644
--- a/include/libxlog.h
+++ b/include/libxlog.h
@@ -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,
diff --git a/libxlog/util.c b/libxlog/util.c
index 053cf04..498c06c 100644
--- a/libxlog/util.c
+++ b/libxlog/util.c
@@ -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)
 {
diff --git a/repair/phase2.c b/repair/phase2.c
index 983aaa7..a23f472 100644
--- a/repair/phase2.c
+++ b/repair/phase2.c
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/4] xfsprogs: rename dont_obfuscate in xfs_metadump

[Eric Sandeen]

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

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 db/metadump.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/db/metadump.c b/db/metadump.c
index b227a6d..57b53d9 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/4] xfsprogs: zero out clean log in xfs_metadump

[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 <sandeen@redhat.com>
---

diff --git a/db/metadump.c b/db/metadump.c
index bea4e00..eb5e9da 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -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;
 	}
+
+	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 */
+		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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/4] xfsprogs: remove unused write-only var l_logsize

[Brian Foster]

On Fri, May 29, 2015 at 02:03:37PM -0500, Eric Sandeen wrote:
> l_logsize is only ever set, and is never read.
> Remove it.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  db/sb.c             |    1 -
>  include/libxlog.h   |    1 -
>  logprint/logprint.c |    1 -
>  repair/phase2.c     |    1 -
>  4 files changed, 0 insertions(+), 4 deletions(-)
> 
> diff --git a/db/sb.c b/db/sb.c
> index cd12f83..991478d 100644
> --- a/db/sb.c
> +++ b/db/sb.c
> @@ -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);
> diff --git a/include/libxlog.h b/include/libxlog.h
> index d6ba9a5..e6d915c 100644
> --- a/include/libxlog.h
> +++ b/include/libxlog.h
> @@ -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 */
> diff --git a/logprint/logprint.c b/logprint/logprint.c
> index 960a267..fdf106b 100644
> --- a/logprint/logprint.c
> +++ b/logprint/logprint.c
> @@ -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);
> diff --git a/repair/phase2.c b/repair/phase2.c
> index 0192346..983aaa7 100644
> --- a/repair/phase2.c
> +++ b/repair/phase2.c
> @@ -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
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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

Re: [PATCH 2/4] xfsprogs: add xlog_is_empty() helper

[Brian Foster]

On Fri, May 29, 2015 at 02:06:30PM -0500, Eric Sandeen wrote:
> 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 <sandeen@redhat.com>
> ---

Yeah, messing with the libxfs_init structure seems strange, but it looks
like it preserves current behavior to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  db/sb.c           |   21 ++++---------------
>  include/libxlog.h |    2 +
>  libxlog/util.c    |   56 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  repair/phase2.c   |   55 +++++++++++----------------------------------------
>  4 files changed, 75 insertions(+), 59 deletions(-)
> 
> diff --git a/db/sb.c b/db/sb.c
> index 991478d..f9d39e6 100644
> --- a/db/sb.c
> +++ b/db/sb.c
> @@ -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;
>  }
>  
> diff --git a/include/libxlog.h b/include/libxlog.h
> index e6d915c..c6640a8 100644
> --- a/include/libxlog.h
> +++ b/include/libxlog.h
> @@ -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,
> diff --git a/libxlog/util.c b/libxlog/util.c
> index 053cf04..498c06c 100644
> --- a/libxlog/util.c
> +++ b/libxlog/util.c
> @@ -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)
>  {
> diff --git a/repair/phase2.c b/repair/phase2.c
> index 983aaa7..a23f472 100644
> --- a/repair/phase2.c
> +++ b/repair/phase2.c
> @@ -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
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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

Re: [PATCH 3/4] xfsprogs: rename dont_obfuscate in xfs_metadump

[Brian Foster]

On Fri, May 29, 2015 at 02:13:24PM -0500, Eric Sandeen wrote:
> Seeing "if (!dont_obfuscate)" hurts my brain; rename it to
> "obfuscate" and invert the logic.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

We could have gone with dont_not_obfuscate, but this works too. ;)

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  db/metadump.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/db/metadump.c b/db/metadump.c
> index b227a6d..57b53d9 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -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
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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

Re: [PATCH 4/4] xfsprogs: zero out clean log in xfs_metadump

[Brian Foster]

On Fri, May 29, 2015 at 02:17:48PM -0500, Eric Sandeen wrote:
> 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 <sandeen@redhat.com>
> ---
> 
> diff --git a/db/metadump.c b/db/metadump.c
> index bea4e00..eb5e9da 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -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;
>  	}
> +
> +	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?

Brian

> +		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
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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

Re: [PATCH 4/4] xfsprogs: zero out clean log in xfs_metadump

[Eric Sandeen]

On 6/3/15 12:11 PM, Brian Foster wrote:
> On Fri, May 29, 2015 at 02:17:48PM -0500, Eric Sandeen wrote:
>> 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 <sandeen@redhat.com>
>> ---
>>
>> diff --git a/db/metadump.c b/db/metadump.c
>> index bea4e00..eb5e9da 100644
>> --- a/db/metadump.c
>> +++ b/db/metadump.c
>> @@ -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;
>>  	}
>> +
>> +	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?

Yeah, I had thought about that too... now, why didn't I do it ... probably
worth doing, I'll send V2 unless I think of a reason not to :)

-Eric

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

[PATCH 4/4 V2] xfsprogs: zero out clean log in xfs_metadump

[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 <sandeen@redhat.com>
---

V2: If not obfuscating, copy log as-is

diff --git a/db/metadump.c b/db/metadump.c
index bea4e00..67de0c5 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -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,
+			mp->m_sb.sb_logblocks * mp->m_sb.sb_blocksize);
+		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;
+	}
+
+done:
 	return !write_buf(iocur_top);
 }
 

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

Re: [PATCH 4/4 V2] xfsprogs: zero out clean log in xfs_metadump

[Brian Foster]

On Mon, Jun 08, 2015 at 10:06:34AM -0500, Eric Sandeen wrote:
> 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 <sandeen@redhat.com>
> ---
> 
> V2: If not obfuscating, copy log as-is
> 

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

> diff --git a/db/metadump.c b/db/metadump.c
> index bea4e00..67de0c5 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -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,
> +			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.

Brian

> +			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
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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

Re: [PATCH 4/4 V2] xfsprogs: zero out clean log in xfs_metadump

[Eric Sandeen]

On 6/8/15 1:29 PM, Brian Foster wrote:
> On Mon, Jun 08, 2015 at 10:06:34AM -0500, Eric Sandeen wrote:
>> 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 <sandeen@redhat.com>
>> ---
>>
>> 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?) :)

>> diff --git a/db/metadump.c b/db/metadump.c
>> index bea4e00..67de0c5 100644
>> --- a/db/metadump.c
>> +++ b/db/metadump.c
>> @@ -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,
>> +			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.

Oh hell, I forgot I had those.

V3 coming.

-Eric

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

Re: [PATCH 4/4 V3] xfsprogs: zero out clean log in xfs_metadump

[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 <sandeen@redhat.com>
---

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

diff --git a/db/metadump.c b/db/metadump.c
index bea4e00..bdc48a0 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 4/4 V3] xfsprogs: zero out clean log in xfs_metadump

[Brian Foster]

On Mon, Jun 08, 2015 at 01:46:37PM -0500, Eric Sandeen wrote:
> 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 <sandeen@redhat.com>
> ---
> 
> V2: If not obfuscating, copy log as-is
> V3: Remove pointless "obfuscate" checks after goto
> 

Looks good to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> diff --git a/db/metadump.c b/db/metadump.c
> index bea4e00..bdc48a0 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -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
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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

Re: [PATCH 4/4 V2] xfsprogs: zero out clean log in xfs_metadump

[Dave Chinner]

On Mon, Jun 08, 2015 at 01:32:17PM -0500, Eric Sandeen wrote:
> On 6/8/15 1:29 PM, Brian Foster wrote:
> > On Mon, Jun 08, 2015 at 10:06:34AM -0500, Eric Sandeen wrote:
> >> 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 <sandeen@redhat.com>
> >> ---
> >>
> >> 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?) :)

Just send a delta patch against what I committed, that way I don't
need to rebase the tree.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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