[PATCH] xfs_metadump: tag metadump image with attribute flags

[PATCH] xfs_metadump: tag metadump image with attribute flags

[Eric Sandeen]

After the long discussion about warning the user and/or consumer
of xfs_metadumps about dirty logs, it crossed my mind that we
could use the reserved slot in the metadump header to tag the
file with attributes, so the consumer of the metadump knows how
it was created.

This patch adds 3 flags to describe the metadump: dirty log,
obfuscated, and full blocks (unused portions of metadata blocks
are not zeroed out).

It then adds a new option to xfs_mdrestore, "-i" to show info,
which can be used with or without a target file:

# xfs_mdrestore -i metadumpfile
metadumpfile: not obfuscated, clean log, full metadata blocks

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

diff --git a/db/metadump.c b/db/metadump.c
index fe068ef..2062415 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -2820,6 +2820,27 @@ metadump_f(
 	metablock->mb_blocklog = BBSHIFT;
 	metablock->mb_magic = cpu_to_be32(XFS_MD_MAGIC);
 
+	/* Set flags indicating state of metadump */
+	if (obfuscate)
+		metablock->mb_flags = XFS_METADUMP_OBFUSCATED;
+	if (!zero_stale_data)
+		metablock->mb_flags |= XFS_METADUMP_FULLBLOCKS;
+
+	/* If we'll copy the log, see if the log is dirty */
+	if (mp->m_sb.sb_logstart) {
+		push_cur();
+		set_cur(&typtab[TYP_LOG],
+			XFS_FSB_TO_DADDR(mp, mp->m_sb.sb_logstart),
+			mp->m_sb.sb_logblocks * blkbb, DB_RING_IGN, NULL);
+		if (iocur_top->data) {	/* best effort */
+			struct xlog	log;
+
+			if (xlog_is_dirty(mp, &log, &x, 0))
+				metablock->mb_flags |= XFS_METADUMP_DIRTYLOG;
+		}
+		pop_cur();
+	}
+
 	block_index = (__be64 *)((char *)metablock + sizeof(xfs_metablock_t));
 	block_buffer = (char *)metablock + BBSIZE;
 	num_indices = (BBSIZE - sizeof(xfs_metablock_t)) / sizeof(__be64);
diff --git a/include/xfs_metadump.h b/include/xfs_metadump.h
index f4be51b..dfe804c 100644
--- a/include/xfs_metadump.h
+++ b/include/xfs_metadump.h
@@ -25,8 +25,13 @@ typedef struct xfs_metablock {
 	__be32		mb_magic;
 	__be16		mb_count;
 	__uint8_t	mb_blocklog;
-	__uint8_t	mb_reserved;
+	__uint8_t	mb_flags;
 	/* followed by an array of xfs_daddr_t */
 } xfs_metablock_t;
 
+/* mb_flags */
+#define XFS_METADUMP_OBFUSCATED	(1 << 0)
+#define XFS_METADUMP_FULLBLOCKS	(1 << 1)
+#define XFS_METADUMP_DIRTYLOG	(1 << 2)
+
 #endif /* _XFS_METADUMP_H_ */
diff --git a/man/man8/xfs_mdrestore.8 b/man/man8/xfs_mdrestore.8
index 2095f15..10c00b2 100644
--- a/man/man8/xfs_mdrestore.8
+++ b/man/man8/xfs_mdrestore.8
@@ -4,11 +4,15 @@ xfs_mdrestore \- restores an XFS metadump image to a filesystem image
 .SH SYNOPSIS
 .B xfs_mdrestore
 [
-.B \-g
+.B \-gi
 ]
 .I source
 .I target
 .br
+.B xfs_mdrestore
+.B \-i
+.I source
+.br
 .B xfs_mdrestore \-V
 .SH DESCRIPTION
 .B xfs_mdrestore
@@ -39,6 +43,11 @@ can be destroyed.
 .B \-g
 Shows restore progress on stdout.
 .TP
+.B \-i
+Shows metadump information on stdout.  If no
+.I target
+is specified, exits after displaying information.
+.TP
 .B \-V
 Prints the version number and exits.
 .SH DIAGNOSTICS
diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index 0d399f1..1fbb1e6 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -21,6 +21,7 @@
 
 char 		*progname;
 int		show_progress = 0;
+int		show_info = 0;
 int		progress_since_warning = 0;
 
 static void
@@ -213,11 +214,14 @@ main(
 
 	progname = basename(argv[0]);
 
-	while ((c = getopt(argc, argv, "gV")) != EOF) {
+	while ((c = getopt(argc, argv, "giV")) != EOF) {
 		switch (c) {
 			case 'g':
 				show_progress = 1;
 				break;
+			case 'i':
+				show_info = 1;
+				break;
 			case 'V':
 				printf("%s version %s\n", progname, VERSION);
 				exit(0);
@@ -226,7 +230,11 @@ main(
 		}
 	}
 
-	if (argc - optind != 2)
+	if (argc - optind < 1 || argc - optind > 2)
+		usage();
+
+	/* show_info without a target is ok */
+	if (!show_info && argc - optind != 2)
 		usage();
 
 	/* open source */
@@ -239,6 +247,29 @@ main(
 		if (src_f == NULL)
 			fatal("cannot open source dump file\n");
 	}
+
+	if (show_info) {
+		xfs_metablock_t		mb;
+
+		if (fread(&mb, sizeof(mb), 1, src_f) != 1)
+			fatal("error reading from file: %s\n", strerror(errno));
+
+		if (be32_to_cpu(mb.mb_magic) != XFS_MD_MAGIC)
+			fatal("specified file is not a metadata dump\n");
+
+		printf("%s: %sobfuscated, %s log, %s metadata blocks\n",
+			argv[optind],
+			mb.mb_flags & XFS_METADUMP_OBFUSCATED ? "" : "not ",
+			mb.mb_flags & XFS_METADUMP_DIRTYLOG ? "dirty" : "clean",
+			mb.mb_flags & XFS_METADUMP_FULLBLOCKS ? "full" : "zeroed");
+
+		if (argc - optind == 1)
+			exit(0);
+
+		/* Go back to the beginning for the restore function */
+		fseek(src_f, 0L, SEEK_SET);
+	}
+
 	optind++;
 
 	/* check and open target */

Re: [PATCH] xfs_metadump: tag metadump image with attribute flags

[Darrick J. Wong]

On Mon, May 22, 2017 at 07:33:35PM -0500, Eric Sandeen wrote:
> After the long discussion about warning the user and/or consumer
> of xfs_metadumps about dirty logs, it crossed my mind that we
> could use the reserved slot in the metadump header to tag the
> file with attributes, so the consumer of the metadump knows how
> it was created.
> 
> This patch adds 3 flags to describe the metadump: dirty log,
> obfuscated, and full blocks (unused portions of metadata blocks
> are not zeroed out).
> 
> It then adds a new option to xfs_mdrestore, "-i" to show info,
> which can be used with or without a target file:
> 
> # xfs_mdrestore -i metadumpfile
> metadumpfile: not obfuscated, clean log, full metadata blocks
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/db/metadump.c b/db/metadump.c
> index fe068ef..2062415 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -2820,6 +2820,27 @@ metadump_f(
>  	metablock->mb_blocklog = BBSHIFT;
>  	metablock->mb_magic = cpu_to_be32(XFS_MD_MAGIC);
>  
> +	/* Set flags indicating state of metadump */
> +	if (obfuscate)
> +		metablock->mb_flags = XFS_METADUMP_OBFUSCATED;
> +	if (!zero_stale_data)
> +		metablock->mb_flags |= XFS_METADUMP_FULLBLOCKS;
> +
> +	/* If we'll copy the log, see if the log is dirty */
> +	if (mp->m_sb.sb_logstart) {
> +		push_cur();
> +		set_cur(&typtab[TYP_LOG],
> +			XFS_FSB_TO_DADDR(mp, mp->m_sb.sb_logstart),
> +			mp->m_sb.sb_logblocks * blkbb, DB_RING_IGN, NULL);
> +		if (iocur_top->data) {	/* best effort */
> +			struct xlog	log;
> +
> +			if (xlog_is_dirty(mp, &log, &x, 0))
> +				metablock->mb_flags |= XFS_METADUMP_DIRTYLOG;
> +		}
> +		pop_cur();
> +	}
> +
>  	block_index = (__be64 *)((char *)metablock + sizeof(xfs_metablock_t));
>  	block_buffer = (char *)metablock + BBSIZE;
>  	num_indices = (BBSIZE - sizeof(xfs_metablock_t)) / sizeof(__be64);
> diff --git a/include/xfs_metadump.h b/include/xfs_metadump.h
> index f4be51b..dfe804c 100644
> --- a/include/xfs_metadump.h
> +++ b/include/xfs_metadump.h
> @@ -25,8 +25,13 @@ typedef struct xfs_metablock {
>  	__be32		mb_magic;
>  	__be16		mb_count;
>  	__uint8_t	mb_blocklog;
> -	__uint8_t	mb_reserved;
> +	__uint8_t	mb_flags;
>  	/* followed by an array of xfs_daddr_t */
>  } xfs_metablock_t;
>  
> +/* mb_flags */
> +#define XFS_METADUMP_OBFUSCATED	(1 << 0)
> +#define XFS_METADUMP_FULLBLOCKS	(1 << 1)
> +#define XFS_METADUMP_DIRTYLOG	(1 << 2)

If we always wrote zero for mb_reserved previously, how do we
distinguish between a non-obfuscated partial-block clean-log metadump
and an old metadump?  Do we care?

I imagine metadumps are fairly transitory in nature, so it might not be
a big deal, and probably not worth burning 1/8 of our flag-space over
since AFAICT we don't use the(se) flags for any serious behavioral
changes.

--D

> +
>  #endif /* _XFS_METADUMP_H_ */
> diff --git a/man/man8/xfs_mdrestore.8 b/man/man8/xfs_mdrestore.8
> index 2095f15..10c00b2 100644
> --- a/man/man8/xfs_mdrestore.8
> +++ b/man/man8/xfs_mdrestore.8
> @@ -4,11 +4,15 @@ xfs_mdrestore \- restores an XFS metadump image to a filesystem image
>  .SH SYNOPSIS
>  .B xfs_mdrestore
>  [
> -.B \-g
> +.B \-gi
>  ]
>  .I source
>  .I target
>  .br
> +.B xfs_mdrestore
> +.B \-i
> +.I source
> +.br
>  .B xfs_mdrestore \-V
>  .SH DESCRIPTION
>  .B xfs_mdrestore
> @@ -39,6 +43,11 @@ can be destroyed.
>  .B \-g
>  Shows restore progress on stdout.
>  .TP
> +.B \-i
> +Shows metadump information on stdout.  If no
> +.I target
> +is specified, exits after displaying information.
> +.TP
>  .B \-V
>  Prints the version number and exits.
>  .SH DIAGNOSTICS
> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> index 0d399f1..1fbb1e6 100644
> --- a/mdrestore/xfs_mdrestore.c
> +++ b/mdrestore/xfs_mdrestore.c
> @@ -21,6 +21,7 @@
>  
>  char 		*progname;
>  int		show_progress = 0;
> +int		show_info = 0;
>  int		progress_since_warning = 0;
>  
>  static void
> @@ -213,11 +214,14 @@ main(
>  
>  	progname = basename(argv[0]);
>  
> -	while ((c = getopt(argc, argv, "gV")) != EOF) {
> +	while ((c = getopt(argc, argv, "giV")) != EOF) {
>  		switch (c) {
>  			case 'g':
>  				show_progress = 1;
>  				break;
> +			case 'i':
> +				show_info = 1;
> +				break;
>  			case 'V':
>  				printf("%s version %s\n", progname, VERSION);
>  				exit(0);
> @@ -226,7 +230,11 @@ main(
>  		}
>  	}
>  
> -	if (argc - optind != 2)
> +	if (argc - optind < 1 || argc - optind > 2)
> +		usage();
> +
> +	/* show_info without a target is ok */
> +	if (!show_info && argc - optind != 2)
>  		usage();
>  
>  	/* open source */
> @@ -239,6 +247,29 @@ main(
>  		if (src_f == NULL)
>  			fatal("cannot open source dump file\n");
>  	}
> +
> +	if (show_info) {
> +		xfs_metablock_t		mb;
> +
> +		if (fread(&mb, sizeof(mb), 1, src_f) != 1)
> +			fatal("error reading from file: %s\n", strerror(errno));
> +
> +		if (be32_to_cpu(mb.mb_magic) != XFS_MD_MAGIC)
> +			fatal("specified file is not a metadata dump\n");
> +
> +		printf("%s: %sobfuscated, %s log, %s metadata blocks\n",
> +			argv[optind],
> +			mb.mb_flags & XFS_METADUMP_OBFUSCATED ? "" : "not ",
> +			mb.mb_flags & XFS_METADUMP_DIRTYLOG ? "dirty" : "clean",
> +			mb.mb_flags & XFS_METADUMP_FULLBLOCKS ? "full" : "zeroed");
> +
> +		if (argc - optind == 1)
> +			exit(0);
> +
> +		/* Go back to the beginning for the restore function */
> +		fseek(src_f, 0L, SEEK_SET);
> +	}
> +
>  	optind++;
>  
>  	/* check and open target */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] xfs_metadump: tag metadump image with attribute flags

[Eric Sandeen]

On 5/23/17 10:46 AM, Darrick J. Wong wrote:
> On Mon, May 22, 2017 at 07:33:35PM -0500, Eric Sandeen wrote:
>> After the long discussion about warning the user and/or consumer
>> of xfs_metadumps about dirty logs, it crossed my mind that we
>> could use the reserved slot in the metadump header to tag the
>> file with attributes, so the consumer of the metadump knows how
>> it was created.
>>
>> This patch adds 3 flags to describe the metadump: dirty log,
>> obfuscated, and full blocks (unused portions of metadata blocks
>> are not zeroed out).
>>
>> It then adds a new option to xfs_mdrestore, "-i" to show info,
>> which can be used with or without a target file:


>> +/* mb_flags */
>> +#define XFS_METADUMP_OBFUSCATED	(1 << 0)
>> +#define XFS_METADUMP_FULLBLOCKS	(1 << 1)
>> +#define XFS_METADUMP_DIRTYLOG	(1 << 2)
> 
> If we always wrote zero for mb_reserved previously, how do we
> distinguish between a non-obfuscated partial-block clean-log metadump
> and an old metadump?  Do we care?

Oh right.  ;)

> I imagine metadumps are fairly transitory in nature, so it might not be
> a big deal, and probably not worth burning 1/8 of our flag-space over
> since AFAICT we don't use the(se) flags for any serious behavioral
> changes.

OTOH, how many flags would we possibly have?  I'm ok with adding a
"we have flags" flag, too.

-Eric

> --D
> 
>> +
>>  #endif /* _XFS_METADUMP_H_ */
>> diff --git a/man/man8/xfs_mdrestore.8 b/man/man8/xfs_mdrestore.8
>> index 2095f15..10c00b2 100644
>> --- a/man/man8/xfs_mdrestore.8
>> +++ b/man/man8/xfs_mdrestore.8
>> @@ -4,11 +4,15 @@ xfs_mdrestore \- restores an XFS metadump image to a filesystem image
>>  .SH SYNOPSIS
>>  .B xfs_mdrestore
>>  [
>> -.B \-g
>> +.B \-gi
>>  ]
>>  .I source
>>  .I target
>>  .br
>> +.B xfs_mdrestore
>> +.B \-i
>> +.I source
>> +.br
>>  .B xfs_mdrestore \-V
>>  .SH DESCRIPTION
>>  .B xfs_mdrestore
>> @@ -39,6 +43,11 @@ can be destroyed.
>>  .B \-g
>>  Shows restore progress on stdout.
>>  .TP
>> +.B \-i
>> +Shows metadump information on stdout.  If no
>> +.I target
>> +is specified, exits after displaying information.
>> +.TP
>>  .B \-V
>>  Prints the version number and exits.
>>  .SH DIAGNOSTICS
>> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
>> index 0d399f1..1fbb1e6 100644
>> --- a/mdrestore/xfs_mdrestore.c
>> +++ b/mdrestore/xfs_mdrestore.c
>> @@ -21,6 +21,7 @@
>>  
>>  char 		*progname;
>>  int		show_progress = 0;
>> +int		show_info = 0;
>>  int		progress_since_warning = 0;
>>  
>>  static void
>> @@ -213,11 +214,14 @@ main(
>>  
>>  	progname = basename(argv[0]);
>>  
>> -	while ((c = getopt(argc, argv, "gV")) != EOF) {
>> +	while ((c = getopt(argc, argv, "giV")) != EOF) {
>>  		switch (c) {
>>  			case 'g':
>>  				show_progress = 1;
>>  				break;
>> +			case 'i':
>> +				show_info = 1;
>> +				break;
>>  			case 'V':
>>  				printf("%s version %s\n", progname, VERSION);
>>  				exit(0);
>> @@ -226,7 +230,11 @@ main(
>>  		}
>>  	}
>>  
>> -	if (argc - optind != 2)
>> +	if (argc - optind < 1 || argc - optind > 2)
>> +		usage();
>> +
>> +	/* show_info without a target is ok */
>> +	if (!show_info && argc - optind != 2)
>>  		usage();
>>  
>>  	/* open source */
>> @@ -239,6 +247,29 @@ main(
>>  		if (src_f == NULL)
>>  			fatal("cannot open source dump file\n");
>>  	}
>> +
>> +	if (show_info) {
>> +		xfs_metablock_t		mb;
>> +
>> +		if (fread(&mb, sizeof(mb), 1, src_f) != 1)
>> +			fatal("error reading from file: %s\n", strerror(errno));
>> +
>> +		if (be32_to_cpu(mb.mb_magic) != XFS_MD_MAGIC)
>> +			fatal("specified file is not a metadata dump\n");
>> +
>> +		printf("%s: %sobfuscated, %s log, %s metadata blocks\n",
>> +			argv[optind],
>> +			mb.mb_flags & XFS_METADUMP_OBFUSCATED ? "" : "not ",
>> +			mb.mb_flags & XFS_METADUMP_DIRTYLOG ? "dirty" : "clean",
>> +			mb.mb_flags & XFS_METADUMP_FULLBLOCKS ? "full" : "zeroed");
>> +
>> +		if (argc - optind == 1)
>> +			exit(0);
>> +
>> +		/* Go back to the beginning for the restore function */
>> +		fseek(src_f, 0L, SEEK_SET);
>> +	}
>> +
>>  	optind++;
>>  
>>  	/* check and open target */
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] xfs_metadump: tag metadump image with attribute flags

[Dave Chinner]

On Tue, May 23, 2017 at 10:47:50AM -0500, Eric Sandeen wrote:
> On 5/23/17 10:46 AM, Darrick J. Wong wrote:
> > On Mon, May 22, 2017 at 07:33:35PM -0500, Eric Sandeen wrote:
> >> After the long discussion about warning the user and/or consumer
> >> of xfs_metadumps about dirty logs, it crossed my mind that we
> >> could use the reserved slot in the metadump header to tag the
> >> file with attributes, so the consumer of the metadump knows how
> >> it was created.
> >>
> >> This patch adds 3 flags to describe the metadump: dirty log,
> >> obfuscated, and full blocks (unused portions of metadata blocks
> >> are not zeroed out).
> >>
> >> It then adds a new option to xfs_mdrestore, "-i" to show info,
> >> which can be used with or without a target file:
> 
> 
> >> +/* mb_flags */
> >> +#define XFS_METADUMP_OBFUSCATED	(1 << 0)
> >> +#define XFS_METADUMP_FULLBLOCKS	(1 << 1)
> >> +#define XFS_METADUMP_DIRTYLOG	(1 << 2)
> > 
> > If we always wrote zero for mb_reserved previously, how do we
> > distinguish between a non-obfuscated partial-block clean-log metadump
> > and an old metadump?  Do we care?
> 
> Oh right.  ;)
> 
> > I imagine metadumps are fairly transitory in nature, so it might not be
> > a big deal, and probably not worth burning 1/8 of our flag-space over
> > since AFAICT we don't use the(se) flags for any serious behavioral
> > changes.
> 
> OTOH, how many flags would we possibly have?  I'm ok with adding a
> "we have flags" flag, too.

We can't extend the metadump header in a backwards compatible way
unless mdrestore already rejects metadumps with non-zero mb_reserved
fields....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs_metadump: tag metadump image with attribute flags

[Eric Sandeen]

On 5/23/17 10:26 PM, Dave Chinner wrote:
> On Tue, May 23, 2017 at 10:47:50AM -0500, Eric Sandeen wrote:
>> On 5/23/17 10:46 AM, Darrick J. Wong wrote:
>>> On Mon, May 22, 2017 at 07:33:35PM -0500, Eric Sandeen wrote:
>>>> After the long discussion about warning the user and/or consumer
>>>> of xfs_metadumps about dirty logs, it crossed my mind that we
>>>> could use the reserved slot in the metadump header to tag the
>>>> file with attributes, so the consumer of the metadump knows how
>>>> it was created.
>>>>
>>>> This patch adds 3 flags to describe the metadump: dirty log,
>>>> obfuscated, and full blocks (unused portions of metadata blocks
>>>> are not zeroed out).
>>>>
>>>> It then adds a new option to xfs_mdrestore, "-i" to show info,
>>>> which can be used with or without a target file:
>>
>>
>>>> +/* mb_flags */
>>>> +#define XFS_METADUMP_OBFUSCATED	(1 << 0)
>>>> +#define XFS_METADUMP_FULLBLOCKS	(1 << 1)
>>>> +#define XFS_METADUMP_DIRTYLOG	(1 << 2)
>>>
>>> If we always wrote zero for mb_reserved previously, how do we
>>> distinguish between a non-obfuscated partial-block clean-log metadump
>>> and an old metadump?  Do we care?
>>
>> Oh right.  ;)
>>
>>> I imagine metadumps are fairly transitory in nature, so it might not be
>>> a big deal, and probably not worth burning 1/8 of our flag-space over
>>> since AFAICT we don't use the(se) flags for any serious behavioral
>>> changes.
>>
>> OTOH, how many flags would we possibly have?  I'm ok with adding a
>> "we have flags" flag, too.
> 
> We can't extend the metadump header in a backwards compatible way
> unless mdrestore already rejects metadumps with non-zero mb_reserved
> fields....

There's no action taken based on the contents; it's information only,
and essentially optional.  It doesn't affect the primary functionality of
mdrestore in any way.

The reserved field has always been zeroed before; if newer mdrestore finds
all zeros, it can say "no info available."

If newer mdrestore finds a "we have flags" flag, it can report information
contained there.

What hole am I missing?

Thanks,
-Eric

Re: [PATCH] xfs_metadump: tag metadump image with attribute flags

[Eric Sandeen]

On 5/23/17 10:29 PM, Eric Sandeen wrote:
> On 5/23/17 10:26 PM, Dave Chinner wrote:
>> On Tue, May 23, 2017 at 10:47:50AM -0500, Eric Sandeen wrote:
>>> On 5/23/17 10:46 AM, Darrick J. Wong wrote:
>>>> On Mon, May 22, 2017 at 07:33:35PM -0500, Eric Sandeen wrote:
>>>>> After the long discussion about warning the user and/or consumer
>>>>> of xfs_metadumps about dirty logs, it crossed my mind that we
>>>>> could use the reserved slot in the metadump header to tag the
>>>>> file with attributes, so the consumer of the metadump knows how
>>>>> it was created.
>>>>>
>>>>> This patch adds 3 flags to describe the metadump: dirty log,
>>>>> obfuscated, and full blocks (unused portions of metadata blocks
>>>>> are not zeroed out).
>>>>>
>>>>> It then adds a new option to xfs_mdrestore, "-i" to show info,
>>>>> which can be used with or without a target file:
>>>
>>>
>>>>> +/* mb_flags */
>>>>> +#define XFS_METADUMP_OBFUSCATED	(1 << 0)
>>>>> +#define XFS_METADUMP_FULLBLOCKS	(1 << 1)
>>>>> +#define XFS_METADUMP_DIRTYLOG	(1 << 2)
>>>>
>>>> If we always wrote zero for mb_reserved previously, how do we
>>>> distinguish between a non-obfuscated partial-block clean-log metadump
>>>> and an old metadump?  Do we care?
>>>
>>> Oh right.  ;)
>>>
>>>> I imagine metadumps are fairly transitory in nature, so it might not be
>>>> a big deal, and probably not worth burning 1/8 of our flag-space over
>>>> since AFAICT we don't use the(se) flags for any serious behavioral
>>>> changes.
>>>
>>> OTOH, how many flags would we possibly have?  I'm ok with adding a
>>> "we have flags" flag, too.
>>
>> We can't extend the metadump header in a backwards compatible way
>> unless mdrestore already rejects metadumps with non-zero mb_reserved
>> fields....
> 
> There's no action taken based on the contents; it's information only,
> and essentially optional.  It doesn't affect the primary functionality of
> mdrestore in any way.
> 
> The reserved field has always been zeroed before:

61983f67 (Barry Naujok 2007-06-05 04:03:18 +0000 2810) metablock = (xfs_metablock_t *)calloc(BBSIZE + 1, BBSIZE);

commit 61983f67786db6770ff2ce5e3086ce2561c4cc8c

Author: Barry Naujok <bnaujok@sgi.com>
Date:   Tue Jun 5 04:03:18 2007 +0000

    XFS metadata dump tool

Can certainly add that info to the commit log if it helps.

 if newer mdrestore finds
> all zeros, it can say "no info available."
> 
> If newer mdrestore finds a "we have flags" flag, it can report information
> contained there.
> 
> What hole am I missing?
> 
> Thanks,
> -Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] xfs_metadump: tag metadump image with attribute flags

[Dave Chinner]

On Tue, May 23, 2017 at 10:29:33PM -0500, Eric Sandeen wrote:
> On 5/23/17 10:26 PM, Dave Chinner wrote:
> > On Tue, May 23, 2017 at 10:47:50AM -0500, Eric Sandeen wrote:
> >> On 5/23/17 10:46 AM, Darrick J. Wong wrote:
> >>> On Mon, May 22, 2017 at 07:33:35PM -0500, Eric Sandeen wrote:
> >>>> After the long discussion about warning the user and/or consumer
> >>>> of xfs_metadumps about dirty logs, it crossed my mind that we
> >>>> could use the reserved slot in the metadump header to tag the
> >>>> file with attributes, so the consumer of the metadump knows how
> >>>> it was created.
> >>>>
> >>>> This patch adds 3 flags to describe the metadump: dirty log,
> >>>> obfuscated, and full blocks (unused portions of metadata blocks
> >>>> are not zeroed out).
> >>>>
> >>>> It then adds a new option to xfs_mdrestore, "-i" to show info,
> >>>> which can be used with or without a target file:
> >>
> >>
> >>>> +/* mb_flags */
> >>>> +#define XFS_METADUMP_OBFUSCATED	(1 << 0)
> >>>> +#define XFS_METADUMP_FULLBLOCKS	(1 << 1)
> >>>> +#define XFS_METADUMP_DIRTYLOG	(1 << 2)
> >>>
> >>> If we always wrote zero for mb_reserved previously, how do we
> >>> distinguish between a non-obfuscated partial-block clean-log metadump
> >>> and an old metadump?  Do we care?
> >>
> >> Oh right.  ;)
> >>
> >>> I imagine metadumps are fairly transitory in nature, so it might not be
> >>> a big deal, and probably not worth burning 1/8 of our flag-space over
> >>> since AFAICT we don't use the(se) flags for any serious behavioral
> >>> changes.
> >>
> >> OTOH, how many flags would we possibly have?  I'm ok with adding a
> >> "we have flags" flag, too.
> > 
> > We can't extend the metadump header in a backwards compatible way
> > unless mdrestore already rejects metadumps with non-zero mb_reserved
> > fields....
> 
> There's no action taken based on the contents; it's information only,
> and essentially optional.  It doesn't affect the primary functionality of
> mdrestore in any way.
> 
> The reserved field has always been zeroed before; if newer mdrestore finds
> all zeros, it can say "no info available."
> 
> If newer mdrestore finds a "we have flags" flag, it can report information
> contained there.
> 
> What hole am I missing?

That I read "we have flags flag" as a "we have another flags field"
flag. Which can't be done because the block index is packed hard
against the struct xfs_metablock header.

typedef struct xfs_metablock {
        __be32          mb_magic;
        __be16          mb_count;
        __uint8_t       mb_blocklog;
        __uint8_t       mb_reserved;
        /* followed by an array of xfs_daddr_t */      <<<<<<<<<
} xfs_metablock_t;

And:

block_index = (__be64 *)((char *)metablock + sizeof(xfs_metablock_t));

So, if you change the size of the struct xfs_metablock - which would
need to be done to version the structure and add a specific flags
field - then it changes the location of the first block index in the
metadump header. Older mdrestore binaries will not understand this
and do the wrong thing....

As such, even if we don't change the size of the header, the flags
can only be informational. They can't indicate a change in metadata
format or behaviour such feature flags requires older mdrestorei
binaries to reject flags they don't understand. In that case,
calling the field "mb_info" might be more appropriate....

Cheers,

Dave.
> 
> Thanks,
> -Eric
> 

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfs_metadump: tag metadump image with attribute flags

[Eric Sandeen]

On 5/24/17 5:25 PM, Dave Chinner wrote:
> On Tue, May 23, 2017 at 10:29:33PM -0500, Eric Sandeen wrote:
>> On 5/23/17 10:26 PM, Dave Chinner wrote:
>>> On Tue, May 23, 2017 at 10:47:50AM -0500, Eric Sandeen wrote:
>>>> On 5/23/17 10:46 AM, Darrick J. Wong wrote:
>>>>> On Mon, May 22, 2017 at 07:33:35PM -0500, Eric Sandeen wrote:
>>>>>> After the long discussion about warning the user and/or consumer
>>>>>> of xfs_metadumps about dirty logs, it crossed my mind that we
>>>>>> could use the reserved slot in the metadump header to tag the
>>>>>> file with attributes, so the consumer of the metadump knows how
>>>>>> it was created.
>>>>>>
>>>>>> This patch adds 3 flags to describe the metadump: dirty log,
>>>>>> obfuscated, and full blocks (unused portions of metadata blocks
>>>>>> are not zeroed out).
>>>>>>
>>>>>> It then adds a new option to xfs_mdrestore, "-i" to show info,
>>>>>> which can be used with or without a target file:
>>>>
>>>>
>>>>>> +/* mb_flags */
>>>>>> +#define XFS_METADUMP_OBFUSCATED	(1 << 0)
>>>>>> +#define XFS_METADUMP_FULLBLOCKS	(1 << 1)
>>>>>> +#define XFS_METADUMP_DIRTYLOG	(1 << 2)
>>>>>
>>>>> If we always wrote zero for mb_reserved previously, how do we
>>>>> distinguish between a non-obfuscated partial-block clean-log metadump
>>>>> and an old metadump?  Do we care?
>>>>
>>>> Oh right.  ;)
>>>>
>>>>> I imagine metadumps are fairly transitory in nature, so it might not be
>>>>> a big deal, and probably not worth burning 1/8 of our flag-space over
>>>>> since AFAICT we don't use the(se) flags for any serious behavioral
>>>>> changes.
>>>>
>>>> OTOH, how many flags would we possibly have?  I'm ok with adding a
>>>> "we have flags" flag, too.
>>>
>>> We can't extend the metadump header in a backwards compatible way
>>> unless mdrestore already rejects metadumps with non-zero mb_reserved
>>> fields....
>>
>> There's no action taken based on the contents; it's information only,
>> and essentially optional.  It doesn't affect the primary functionality of
>> mdrestore in any way.
>>
>> The reserved field has always been zeroed before; if newer mdrestore finds
>> all zeros, it can say "no info available."
>>
>> If newer mdrestore finds a "we have flags" flag, it can report information
>> contained there.
>>
>> What hole am I missing?
> 
> That I read "we have flags flag" as a "we have another flags field"
> flag. Which can't be done because the block index is packed hard
> against the struct xfs_metablock header.

Right, I'm not doing that.

I'm suggesting that mb_reserved has been zeroed out from day 1, so
the presence of /anything/ there indicates a newer format.

As Darrick pointed out, the absence of anything there isn't helpful,
unless we:

#define XFS_METADUMP_INFO_FLAGS (1 << 0)

and set that in newer metadump - then we'll know that the presence
or absence of any other feature flags will tell us something
about the dump.  If the INFO_FLAGS flag isn't set, it's an
old dump, and we can't learn anything.

<snip>

> As such, even if we don't change the size of the header, the flags
> can only be informational. They can't indicate a change in metadata
> format or behaviour such feature flags requires older mdrestorei
> binaries to reject flags they don't understand. In that case,
> calling the field "mb_info" might be more appropriate....

Right, that's why I made the option to read them "-i" (info),
and nothing else.

Certainly can't use them for format changes.

Sure, I can change the field name to make it more clear.

Thanks,
-Eric
 
> Cheers,
> 
> Dave.
>>
>> Thanks,
>> -Eric
>>
> 

[PATCH V2] xfs_metadump: tag metadump image with informational flags

[Eric Sandeen]

After the long discussion about warning the user and/or consumer
of xfs_metadumps about dirty logs, it crossed my mind that we
could use the reserved slot in the metadump header to tag the
file with attributes, so the consumer of the metadump knows how
it was created.

This patch adds 4 flags to describe the metadump: The first simply
indicates the presence of any (or no) informational flags.
The old mb_reserved field has been 0 on disk since inception, so
the presence of XFS_METADUMP_INFO_FLAGS indicates that this metadump
may contain the informational flags:

 - dirty log
 - obfuscated
 - full blocks (unused portions of metadata blocks
                are not zeroed out).

It then adds a new option to xfs_mdrestore, "-i" to show info,
which can be used with or without a target file:

# xfs_mdrestore -i metadumpfile
metadumpfile: not obfuscated, clean log, full metadata blocks

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

V2: rename to mb_info, add the "we have flags" flag

diff --git a/db/metadump.c b/db/metadump.c
index fe068ef..2f5b5b5 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -2820,6 +2820,28 @@ metadump_f(
 	metablock->mb_blocklog = BBSHIFT;
 	metablock->mb_magic = cpu_to_be32(XFS_MD_MAGIC);
 
+	/* Set flags about state of metadump */
+	metablock->mb_info = XFS_METADUMP_INFO_FLAGS;
+	if (obfuscate)
+		metablock->mb_info |= XFS_METADUMP_OBFUSCATED;
+	if (!zero_stale_data)
+		metablock->mb_info |= XFS_METADUMP_FULLBLOCKS;
+
+	/* If we'll copy the log, see if the log is dirty */
+	if (mp->m_sb.sb_logstart) {
+		push_cur();
+		set_cur(&typtab[TYP_LOG],
+			XFS_FSB_TO_DADDR(mp, mp->m_sb.sb_logstart),
+			mp->m_sb.sb_logblocks * blkbb, DB_RING_IGN, NULL);
+		if (iocur_top->data) {	/* best effort */
+			struct xlog	log;
+
+			if (xlog_is_dirty(mp, &log, &x, 0))
+				metablock->mb_info |= XFS_METADUMP_DIRTYLOG;
+		}
+		pop_cur();
+	}
+
 	block_index = (__be64 *)((char *)metablock + sizeof(xfs_metablock_t));
 	block_buffer = (char *)metablock + BBSIZE;
 	num_indices = (BBSIZE - sizeof(xfs_metablock_t)) / sizeof(__be64);
diff --git a/include/xfs_metadump.h b/include/xfs_metadump.h
index f4be51b..7f3039e 100644
--- a/include/xfs_metadump.h
+++ b/include/xfs_metadump.h
@@ -25,8 +25,14 @@ typedef struct xfs_metablock {
 	__be32		mb_magic;
 	__be16		mb_count;
 	__uint8_t	mb_blocklog;
-	__uint8_t	mb_reserved;
+	__uint8_t	mb_info;
 	/* followed by an array of xfs_daddr_t */
 } xfs_metablock_t;
 
+/* These flags are informational only, not backwards compatible */
+#define XFS_METADUMP_INFO_FLAGS	(1 << 0) /* This image has informative flags */
+#define XFS_METADUMP_OBFUSCATED	(1 << 1)
+#define XFS_METADUMP_FULLBLOCKS	(1 << 2)
+#define XFS_METADUMP_DIRTYLOG	(1 << 3)
+
 #endif /* _XFS_METADUMP_H_ */
diff --git a/man/man8/xfs_mdrestore.8 b/man/man8/xfs_mdrestore.8
index 2095f15..72f3b29 100644
--- a/man/man8/xfs_mdrestore.8
+++ b/man/man8/xfs_mdrestore.8
@@ -4,11 +4,15 @@ xfs_mdrestore \- restores an XFS metadump image to a filesystem image
 .SH SYNOPSIS
 .B xfs_mdrestore
 [
-.B \-g
+.B \-gi
 ]
 .I source
 .I target
 .br
+.B xfs_mdrestore
+.B \-i
+.I source
+.br
 .B xfs_mdrestore \-V
 .SH DESCRIPTION
 .B xfs_mdrestore
@@ -39,6 +43,12 @@ can be destroyed.
 .B \-g
 Shows restore progress on stdout.
 .TP
+.B \-i
+Shows metadump information on stdout.  If no
+.I target
+is specified, exits after displaying information.  Older metadumps man not
+include any descriptive information.
+.TP
 .B \-V
 Prints the version number and exits.
 .SH DIAGNOSTICS
diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index 0d399f1..9d1b4e8 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -21,6 +21,7 @@
 
 char 		*progname;
 int		show_progress = 0;
+int		show_info = 0;
 int		progress_since_warning = 0;
 
 static void
@@ -213,11 +214,14 @@ main(
 
 	progname = basename(argv[0]);
 
-	while ((c = getopt(argc, argv, "gV")) != EOF) {
+	while ((c = getopt(argc, argv, "giV")) != EOF) {
 		switch (c) {
 			case 'g':
 				show_progress = 1;
 				break;
+			case 'i':
+				show_info = 1;
+				break;
 			case 'V':
 				printf("%s version %s\n", progname, VERSION);
 				exit(0);
@@ -226,7 +230,11 @@ main(
 		}
 	}
 
-	if (argc - optind != 2)
+	if (argc - optind < 1 || argc - optind > 2)
+		usage();
+
+	/* show_info without a target is ok */
+	if (!show_info && argc - optind != 2)
 		usage();
 
 	/* open source */
@@ -239,6 +247,34 @@ main(
 		if (src_f == NULL)
 			fatal("cannot open source dump file\n");
 	}
+
+	if (show_info) {
+		xfs_metablock_t		mb;
+
+		if (fread(&mb, sizeof(mb), 1, src_f) != 1)
+			fatal("error reading from file: %s\n", strerror(errno));
+
+		if (be32_to_cpu(mb.mb_magic) != XFS_MD_MAGIC)
+			fatal("specified file is not a metadata dump\n");
+
+		if (mb.mb_info & XFS_METADUMP_INFO_FLAGS) {
+			printf("%s: %sobfuscated, %s log, %s metadata blocks\n",
+			argv[optind],
+			mb.mb_info & XFS_METADUMP_OBFUSCATED ? "":"not ",
+			mb.mb_info & XFS_METADUMP_DIRTYLOG ? "dirty":"clean",
+			mb.mb_info & XFS_METADUMP_FULLBLOCKS ? "full":"zeroed");
+		} else {
+			printf("%s: no informational flags present\n",
+				argv[optind]);
+		}
+
+		if (argc - optind == 1)
+			exit(0);
+
+		/* Go back to the beginning for the restore function */
+		fseek(src_f, 0L, SEEK_SET);
+	}
+
 	optind++;
 
 	/* check and open target */

Re: [PATCH V2] xfs_metadump: tag metadump image with informational flags

[Eric Sandeen]

On 5/26/17 11:13 AM, Eric Sandeen wrote:
> After the long discussion about warning the user and/or consumer
> of xfs_metadumps about dirty logs, it crossed my mind that we
> could use the reserved slot in the metadump header to tag the
> file with attributes, so the consumer of the metadump knows how
> it was created.
> 
> This patch adds 4 flags to describe the metadump: The first simply
> indicates the presence of any (or no) informational flags.
> The old mb_reserved field has been 0 on disk since inception, so
> the presence of XFS_METADUMP_INFO_FLAGS indicates that this metadump
> may contain the informational flags:
> 
>  - dirty log
>  - obfuscated
>  - full blocks (unused portions of metadata blocks
>                 are not zeroed out).
> 
> It then adds a new option to xfs_mdrestore, "-i" to show info,
> which can be used with or without a target file:
> 
> # xfs_mdrestore -i metadumpfile
> metadumpfile: not obfuscated, clean log, full metadata blocks
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> V2: rename to mb_info, add the "we have flags" flag

Ping, any thoughts on this V2?
 
> diff --git a/db/metadump.c b/db/metadump.c
> index fe068ef..2f5b5b5 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -2820,6 +2820,28 @@ metadump_f(
>  	metablock->mb_blocklog = BBSHIFT;
>  	metablock->mb_magic = cpu_to_be32(XFS_MD_MAGIC);
>  
> +	/* Set flags about state of metadump */
> +	metablock->mb_info = XFS_METADUMP_INFO_FLAGS;
> +	if (obfuscate)
> +		metablock->mb_info |= XFS_METADUMP_OBFUSCATED;
> +	if (!zero_stale_data)
> +		metablock->mb_info |= XFS_METADUMP_FULLBLOCKS;
> +
> +	/* If we'll copy the log, see if the log is dirty */
> +	if (mp->m_sb.sb_logstart) {
> +		push_cur();
> +		set_cur(&typtab[TYP_LOG],
> +			XFS_FSB_TO_DADDR(mp, mp->m_sb.sb_logstart),
> +			mp->m_sb.sb_logblocks * blkbb, DB_RING_IGN, NULL);
> +		if (iocur_top->data) {	/* best effort */
> +			struct xlog	log;
> +
> +			if (xlog_is_dirty(mp, &log, &x, 0))
> +				metablock->mb_info |= XFS_METADUMP_DIRTYLOG;
> +		}
> +		pop_cur();
> +	}
> +
>  	block_index = (__be64 *)((char *)metablock + sizeof(xfs_metablock_t));
>  	block_buffer = (char *)metablock + BBSIZE;
>  	num_indices = (BBSIZE - sizeof(xfs_metablock_t)) / sizeof(__be64);
> diff --git a/include/xfs_metadump.h b/include/xfs_metadump.h
> index f4be51b..7f3039e 100644
> --- a/include/xfs_metadump.h
> +++ b/include/xfs_metadump.h
> @@ -25,8 +25,14 @@ typedef struct xfs_metablock {
>  	__be32		mb_magic;
>  	__be16		mb_count;
>  	__uint8_t	mb_blocklog;
> -	__uint8_t	mb_reserved;
> +	__uint8_t	mb_info;
>  	/* followed by an array of xfs_daddr_t */
>  } xfs_metablock_t;
>  
> +/* These flags are informational only, not backwards compatible */
> +#define XFS_METADUMP_INFO_FLAGS	(1 << 0) /* This image has informative flags */
> +#define XFS_METADUMP_OBFUSCATED	(1 << 1)
> +#define XFS_METADUMP_FULLBLOCKS	(1 << 2)
> +#define XFS_METADUMP_DIRTYLOG	(1 << 3)
> +
>  #endif /* _XFS_METADUMP_H_ */
> diff --git a/man/man8/xfs_mdrestore.8 b/man/man8/xfs_mdrestore.8
> index 2095f15..72f3b29 100644
> --- a/man/man8/xfs_mdrestore.8
> +++ b/man/man8/xfs_mdrestore.8
> @@ -4,11 +4,15 @@ xfs_mdrestore \- restores an XFS metadump image to a filesystem image
>  .SH SYNOPSIS
>  .B xfs_mdrestore
>  [
> -.B \-g
> +.B \-gi
>  ]
>  .I source
>  .I target
>  .br
> +.B xfs_mdrestore
> +.B \-i
> +.I source
> +.br
>  .B xfs_mdrestore \-V
>  .SH DESCRIPTION
>  .B xfs_mdrestore
> @@ -39,6 +43,12 @@ can be destroyed.
>  .B \-g
>  Shows restore progress on stdout.
>  .TP
> +.B \-i
> +Shows metadump information on stdout.  If no
> +.I target
> +is specified, exits after displaying information.  Older metadumps man not
> +include any descriptive information.
> +.TP
>  .B \-V
>  Prints the version number and exits.
>  .SH DIAGNOSTICS
> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> index 0d399f1..9d1b4e8 100644
> --- a/mdrestore/xfs_mdrestore.c
> +++ b/mdrestore/xfs_mdrestore.c
> @@ -21,6 +21,7 @@
>  
>  char 		*progname;
>  int		show_progress = 0;
> +int		show_info = 0;
>  int		progress_since_warning = 0;
>  
>  static void
> @@ -213,11 +214,14 @@ main(
>  
>  	progname = basename(argv[0]);
>  
> -	while ((c = getopt(argc, argv, "gV")) != EOF) {
> +	while ((c = getopt(argc, argv, "giV")) != EOF) {
>  		switch (c) {
>  			case 'g':
>  				show_progress = 1;
>  				break;
> +			case 'i':
> +				show_info = 1;
> +				break;
>  			case 'V':
>  				printf("%s version %s\n", progname, VERSION);
>  				exit(0);
> @@ -226,7 +230,11 @@ main(
>  		}
>  	}
>  
> -	if (argc - optind != 2)
> +	if (argc - optind < 1 || argc - optind > 2)
> +		usage();
> +
> +	/* show_info without a target is ok */
> +	if (!show_info && argc - optind != 2)
>  		usage();
>  
>  	/* open source */
> @@ -239,6 +247,34 @@ main(
>  		if (src_f == NULL)
>  			fatal("cannot open source dump file\n");
>  	}
> +
> +	if (show_info) {
> +		xfs_metablock_t		mb;
> +
> +		if (fread(&mb, sizeof(mb), 1, src_f) != 1)
> +			fatal("error reading from file: %s\n", strerror(errno));
> +
> +		if (be32_to_cpu(mb.mb_magic) != XFS_MD_MAGIC)
> +			fatal("specified file is not a metadata dump\n");
> +
> +		if (mb.mb_info & XFS_METADUMP_INFO_FLAGS) {
> +			printf("%s: %sobfuscated, %s log, %s metadata blocks\n",
> +			argv[optind],
> +			mb.mb_info & XFS_METADUMP_OBFUSCATED ? "":"not ",
> +			mb.mb_info & XFS_METADUMP_DIRTYLOG ? "dirty":"clean",
> +			mb.mb_info & XFS_METADUMP_FULLBLOCKS ? "full":"zeroed");
> +		} else {
> +			printf("%s: no informational flags present\n",
> +				argv[optind]);
> +		}
> +
> +		if (argc - optind == 1)
> +			exit(0);
> +
> +		/* Go back to the beginning for the restore function */
> +		fseek(src_f, 0L, SEEK_SET);
> +	}
> +
>  	optind++;
>  
>  	/* check and open target */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH V2] xfs_metadump: tag metadump image with informational flags

[Darrick J. Wong]

On Wed, Jun 21, 2017 at 02:18:23PM -0500, Eric Sandeen wrote:
> On 5/26/17 11:13 AM, Eric Sandeen wrote:
> > After the long discussion about warning the user and/or consumer
> > of xfs_metadumps about dirty logs, it crossed my mind that we
> > could use the reserved slot in the metadump header to tag the
> > file with attributes, so the consumer of the metadump knows how
> > it was created.
> > 
> > This patch adds 4 flags to describe the metadump: The first simply
> > indicates the presence of any (or no) informational flags.
> > The old mb_reserved field has been 0 on disk since inception, so
> > the presence of XFS_METADUMP_INFO_FLAGS indicates that this metadump
> > may contain the informational flags:
> > 
> >  - dirty log
> >  - obfuscated
> >  - full blocks (unused portions of metadata blocks
> >                 are not zeroed out).
> > 
> > It then adds a new option to xfs_mdrestore, "-i" to show info,
> > which can be used with or without a target file:
> > 
> > # xfs_mdrestore -i metadumpfile
> > metadumpfile: not obfuscated, clean log, full metadata blocks
> > 
> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > ---
> > 
> > V2: rename to mb_info, add the "we have flags" flag
> 
> Ping, any thoughts on this V2?

/me wonder if show_info mode ought to complain if we see an "obviously"
bogus value (flags are set but XFS_METADUMP_INFO_FLAGS is clear, flags
we don't know about are set) and dump the raw hex value...

...but on the other hand it's redefining a probably-zero field in a
semi-ephemeral data file format to be an optional informational field,
so maybe it's good enough.

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

--D

>  
> > diff --git a/db/metadump.c b/db/metadump.c
> > index fe068ef..2f5b5b5 100644
> > --- a/db/metadump.c
> > +++ b/db/metadump.c
> > @@ -2820,6 +2820,28 @@ metadump_f(
> >  	metablock->mb_blocklog = BBSHIFT;
> >  	metablock->mb_magic = cpu_to_be32(XFS_MD_MAGIC);
> >  
> > +	/* Set flags about state of metadump */
> > +	metablock->mb_info = XFS_METADUMP_INFO_FLAGS;
> > +	if (obfuscate)
> > +		metablock->mb_info |= XFS_METADUMP_OBFUSCATED;
> > +	if (!zero_stale_data)
> > +		metablock->mb_info |= XFS_METADUMP_FULLBLOCKS;
> > +
> > +	/* If we'll copy the log, see if the log is dirty */
> > +	if (mp->m_sb.sb_logstart) {
> > +		push_cur();
> > +		set_cur(&typtab[TYP_LOG],
> > +			XFS_FSB_TO_DADDR(mp, mp->m_sb.sb_logstart),
> > +			mp->m_sb.sb_logblocks * blkbb, DB_RING_IGN, NULL);
> > +		if (iocur_top->data) {	/* best effort */
> > +			struct xlog	log;
> > +
> > +			if (xlog_is_dirty(mp, &log, &x, 0))
> > +				metablock->mb_info |= XFS_METADUMP_DIRTYLOG;
> > +		}
> > +		pop_cur();
> > +	}
> > +
> >  	block_index = (__be64 *)((char *)metablock + sizeof(xfs_metablock_t));
> >  	block_buffer = (char *)metablock + BBSIZE;
> >  	num_indices = (BBSIZE - sizeof(xfs_metablock_t)) / sizeof(__be64);
> > diff --git a/include/xfs_metadump.h b/include/xfs_metadump.h
> > index f4be51b..7f3039e 100644
> > --- a/include/xfs_metadump.h
> > +++ b/include/xfs_metadump.h
> > @@ -25,8 +25,14 @@ typedef struct xfs_metablock {
> >  	__be32		mb_magic;
> >  	__be16		mb_count;
> >  	__uint8_t	mb_blocklog;
> > -	__uint8_t	mb_reserved;
> > +	__uint8_t	mb_info;
> >  	/* followed by an array of xfs_daddr_t */
> >  } xfs_metablock_t;
> >  
> > +/* These flags are informational only, not backwards compatible */
> > +#define XFS_METADUMP_INFO_FLAGS	(1 << 0) /* This image has informative flags */
> > +#define XFS_METADUMP_OBFUSCATED	(1 << 1)
> > +#define XFS_METADUMP_FULLBLOCKS	(1 << 2)
> > +#define XFS_METADUMP_DIRTYLOG	(1 << 3)
> > +
> >  #endif /* _XFS_METADUMP_H_ */
> > diff --git a/man/man8/xfs_mdrestore.8 b/man/man8/xfs_mdrestore.8
> > index 2095f15..72f3b29 100644
> > --- a/man/man8/xfs_mdrestore.8
> > +++ b/man/man8/xfs_mdrestore.8
> > @@ -4,11 +4,15 @@ xfs_mdrestore \- restores an XFS metadump image to a filesystem image
> >  .SH SYNOPSIS
> >  .B xfs_mdrestore
> >  [
> > -.B \-g
> > +.B \-gi
> >  ]
> >  .I source
> >  .I target
> >  .br
> > +.B xfs_mdrestore
> > +.B \-i
> > +.I source
> > +.br
> >  .B xfs_mdrestore \-V
> >  .SH DESCRIPTION
> >  .B xfs_mdrestore
> > @@ -39,6 +43,12 @@ can be destroyed.
> >  .B \-g
> >  Shows restore progress on stdout.
> >  .TP
> > +.B \-i
> > +Shows metadump information on stdout.  If no
> > +.I target
> > +is specified, exits after displaying information.  Older metadumps man not
> > +include any descriptive information.
> > +.TP
> >  .B \-V
> >  Prints the version number and exits.
> >  .SH DIAGNOSTICS
> > diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> > index 0d399f1..9d1b4e8 100644
> > --- a/mdrestore/xfs_mdrestore.c
> > +++ b/mdrestore/xfs_mdrestore.c
> > @@ -21,6 +21,7 @@
> >  
> >  char 		*progname;
> >  int		show_progress = 0;
> > +int		show_info = 0;
> >  int		progress_since_warning = 0;
> >  
> >  static void
> > @@ -213,11 +214,14 @@ main(
> >  
> >  	progname = basename(argv[0]);
> >  
> > -	while ((c = getopt(argc, argv, "gV")) != EOF) {
> > +	while ((c = getopt(argc, argv, "giV")) != EOF) {
> >  		switch (c) {
> >  			case 'g':
> >  				show_progress = 1;
> >  				break;
> > +			case 'i':
> > +				show_info = 1;
> > +				break;
> >  			case 'V':
> >  				printf("%s version %s\n", progname, VERSION);
> >  				exit(0);
> > @@ -226,7 +230,11 @@ main(
> >  		}
> >  	}
> >  
> > -	if (argc - optind != 2)
> > +	if (argc - optind < 1 || argc - optind > 2)
> > +		usage();
> > +
> > +	/* show_info without a target is ok */
> > +	if (!show_info && argc - optind != 2)
> >  		usage();
> >  
> >  	/* open source */
> > @@ -239,6 +247,34 @@ main(
> >  		if (src_f == NULL)
> >  			fatal("cannot open source dump file\n");
> >  	}
> > +
> > +	if (show_info) {
> > +		xfs_metablock_t		mb;
> > +
> > +		if (fread(&mb, sizeof(mb), 1, src_f) != 1)
> > +			fatal("error reading from file: %s\n", strerror(errno));
> > +
> > +		if (be32_to_cpu(mb.mb_magic) != XFS_MD_MAGIC)
> > +			fatal("specified file is not a metadata dump\n");
> > +
> > +		if (mb.mb_info & XFS_METADUMP_INFO_FLAGS) {
> > +			printf("%s: %sobfuscated, %s log, %s metadata blocks\n",
> > +			argv[optind],
> > +			mb.mb_info & XFS_METADUMP_OBFUSCATED ? "":"not ",
> > +			mb.mb_info & XFS_METADUMP_DIRTYLOG ? "dirty":"clean",
> > +			mb.mb_info & XFS_METADUMP_FULLBLOCKS ? "full":"zeroed");
> > +		} else {
> > +			printf("%s: no informational flags present\n",
> > +				argv[optind]);
> > +		}
> > +
> > +		if (argc - optind == 1)
> > +			exit(0);
> > +
> > +		/* Go back to the beginning for the restore function */
> > +		fseek(src_f, 0L, SEEK_SET);
> > +	}
> > +
> >  	optind++;
> >  
> >  	/* check and open target */
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] xfs_metadump: tag metadump image with informational flags

[Eric Sandeen]

On 6/21/17 3:34 PM, Darrick J. Wong wrote:
> On Wed, Jun 21, 2017 at 02:18:23PM -0500, Eric Sandeen wrote:
>> On 5/26/17 11:13 AM, Eric Sandeen wrote:
>>> After the long discussion about warning the user and/or consumer
>>> of xfs_metadumps about dirty logs, it crossed my mind that we
>>> could use the reserved slot in the metadump header to tag the
>>> file with attributes, so the consumer of the metadump knows how
>>> it was created.
>>>
>>> This patch adds 4 flags to describe the metadump: The first simply
>>> indicates the presence of any (or no) informational flags.
>>> The old mb_reserved field has been 0 on disk since inception, so
>>> the presence of XFS_METADUMP_INFO_FLAGS indicates that this metadump
>>> may contain the informational flags:
>>>
>>>  - dirty log
>>>  - obfuscated
>>>  - full blocks (unused portions of metadata blocks
>>>                 are not zeroed out).
>>>
>>> It then adds a new option to xfs_mdrestore, "-i" to show info,
>>> which can be used with or without a target file:
>>>
>>> # xfs_mdrestore -i metadumpfile
>>> metadumpfile: not obfuscated, clean log, full metadata blocks
>>>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>> ---
>>>
>>> V2: rename to mb_info, add the "we have flags" flag
>>
>> Ping, any thoughts on this V2?
> 
> /me wonder if show_info mode ought to complain if we see an "obviously"
> bogus value (flags are set but XFS_METADUMP_INFO_FLAGS is clear, flags
> we don't know about are set) and dump the raw hex value...

well, it's been memset to 0 since inception.  Sure, something could have
gone wrong in transit, or whatever, I guess...

I guess it might be nice to sanity check it, but to avoid another
round trip:

> ...but on the other hand it's redefining a probably-zero field in a
> semi-ephemeral data file format to be an optional informational field,
> so maybe it's good enough.
> 
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

thanks,
-Eric

> --D
> 
>>  
>>> diff --git a/db/metadump.c b/db/metadump.c
>>> index fe068ef..2f5b5b5 100644
>>> --- a/db/metadump.c
>>> +++ b/db/metadump.c
>>> @@ -2820,6 +2820,28 @@ metadump_f(
>>>  	metablock->mb_blocklog = BBSHIFT;
>>>  	metablock->mb_magic = cpu_to_be32(XFS_MD_MAGIC);
>>>  
>>> +	/* Set flags about state of metadump */
>>> +	metablock->mb_info = XFS_METADUMP_INFO_FLAGS;
>>> +	if (obfuscate)
>>> +		metablock->mb_info |= XFS_METADUMP_OBFUSCATED;
>>> +	if (!zero_stale_data)
>>> +		metablock->mb_info |= XFS_METADUMP_FULLBLOCKS;
>>> +
>>> +	/* If we'll copy the log, see if the log is dirty */
>>> +	if (mp->m_sb.sb_logstart) {
>>> +		push_cur();
>>> +		set_cur(&typtab[TYP_LOG],
>>> +			XFS_FSB_TO_DADDR(mp, mp->m_sb.sb_logstart),
>>> +			mp->m_sb.sb_logblocks * blkbb, DB_RING_IGN, NULL);
>>> +		if (iocur_top->data) {	/* best effort */
>>> +			struct xlog	log;
>>> +
>>> +			if (xlog_is_dirty(mp, &log, &x, 0))
>>> +				metablock->mb_info |= XFS_METADUMP_DIRTYLOG;
>>> +		}
>>> +		pop_cur();
>>> +	}
>>> +
>>>  	block_index = (__be64 *)((char *)metablock + sizeof(xfs_metablock_t));
>>>  	block_buffer = (char *)metablock + BBSIZE;
>>>  	num_indices = (BBSIZE - sizeof(xfs_metablock_t)) / sizeof(__be64);
>>> diff --git a/include/xfs_metadump.h b/include/xfs_metadump.h
>>> index f4be51b..7f3039e 100644
>>> --- a/include/xfs_metadump.h
>>> +++ b/include/xfs_metadump.h
>>> @@ -25,8 +25,14 @@ typedef struct xfs_metablock {
>>>  	__be32		mb_magic;
>>>  	__be16		mb_count;
>>>  	__uint8_t	mb_blocklog;
>>> -	__uint8_t	mb_reserved;
>>> +	__uint8_t	mb_info;
>>>  	/* followed by an array of xfs_daddr_t */
>>>  } xfs_metablock_t;
>>>  
>>> +/* These flags are informational only, not backwards compatible */
>>> +#define XFS_METADUMP_INFO_FLAGS	(1 << 0) /* This image has informative flags */
>>> +#define XFS_METADUMP_OBFUSCATED	(1 << 1)
>>> +#define XFS_METADUMP_FULLBLOCKS	(1 << 2)
>>> +#define XFS_METADUMP_DIRTYLOG	(1 << 3)
>>> +
>>>  #endif /* _XFS_METADUMP_H_ */
>>> diff --git a/man/man8/xfs_mdrestore.8 b/man/man8/xfs_mdrestore.8
>>> index 2095f15..72f3b29 100644
>>> --- a/man/man8/xfs_mdrestore.8
>>> +++ b/man/man8/xfs_mdrestore.8
>>> @@ -4,11 +4,15 @@ xfs_mdrestore \- restores an XFS metadump image to a filesystem image
>>>  .SH SYNOPSIS
>>>  .B xfs_mdrestore
>>>  [
>>> -.B \-g
>>> +.B \-gi
>>>  ]
>>>  .I source
>>>  .I target
>>>  .br
>>> +.B xfs_mdrestore
>>> +.B \-i
>>> +.I source
>>> +.br
>>>  .B xfs_mdrestore \-V
>>>  .SH DESCRIPTION
>>>  .B xfs_mdrestore
>>> @@ -39,6 +43,12 @@ can be destroyed.
>>>  .B \-g
>>>  Shows restore progress on stdout.
>>>  .TP
>>> +.B \-i
>>> +Shows metadump information on stdout.  If no
>>> +.I target
>>> +is specified, exits after displaying information.  Older metadumps man not
>>> +include any descriptive information.
>>> +.TP
>>>  .B \-V
>>>  Prints the version number and exits.
>>>  .SH DIAGNOSTICS
>>> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
>>> index 0d399f1..9d1b4e8 100644
>>> --- a/mdrestore/xfs_mdrestore.c
>>> +++ b/mdrestore/xfs_mdrestore.c
>>> @@ -21,6 +21,7 @@
>>>  
>>>  char 		*progname;
>>>  int		show_progress = 0;
>>> +int		show_info = 0;
>>>  int		progress_since_warning = 0;
>>>  
>>>  static void
>>> @@ -213,11 +214,14 @@ main(
>>>  
>>>  	progname = basename(argv[0]);
>>>  
>>> -	while ((c = getopt(argc, argv, "gV")) != EOF) {
>>> +	while ((c = getopt(argc, argv, "giV")) != EOF) {
>>>  		switch (c) {
>>>  			case 'g':
>>>  				show_progress = 1;
>>>  				break;
>>> +			case 'i':
>>> +				show_info = 1;
>>> +				break;
>>>  			case 'V':
>>>  				printf("%s version %s\n", progname, VERSION);
>>>  				exit(0);
>>> @@ -226,7 +230,11 @@ main(
>>>  		}
>>>  	}
>>>  
>>> -	if (argc - optind != 2)
>>> +	if (argc - optind < 1 || argc - optind > 2)
>>> +		usage();
>>> +
>>> +	/* show_info without a target is ok */
>>> +	if (!show_info && argc - optind != 2)
>>>  		usage();
>>>  
>>>  	/* open source */
>>> @@ -239,6 +247,34 @@ main(
>>>  		if (src_f == NULL)
>>>  			fatal("cannot open source dump file\n");
>>>  	}
>>> +
>>> +	if (show_info) {
>>> +		xfs_metablock_t		mb;
>>> +
>>> +		if (fread(&mb, sizeof(mb), 1, src_f) != 1)
>>> +			fatal("error reading from file: %s\n", strerror(errno));
>>> +
>>> +		if (be32_to_cpu(mb.mb_magic) != XFS_MD_MAGIC)
>>> +			fatal("specified file is not a metadata dump\n");
>>> +
>>> +		if (mb.mb_info & XFS_METADUMP_INFO_FLAGS) {
>>> +			printf("%s: %sobfuscated, %s log, %s metadata blocks\n",
>>> +			argv[optind],
>>> +			mb.mb_info & XFS_METADUMP_OBFUSCATED ? "":"not ",
>>> +			mb.mb_info & XFS_METADUMP_DIRTYLOG ? "dirty":"clean",
>>> +			mb.mb_info & XFS_METADUMP_FULLBLOCKS ? "full":"zeroed");
>>> +		} else {
>>> +			printf("%s: no informational flags present\n",
>>> +				argv[optind]);
>>> +		}
>>> +
>>> +		if (argc - optind == 1)
>>> +			exit(0);
>>> +
>>> +		/* Go back to the beginning for the restore function */
>>> +		fseek(src_f, 0L, SEEK_SET);
>>> +	}
>>> +
>>>  	optind++;
>>>  
>>>  	/* check and open target */
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>