On 2/1/18 2:37 PM, Marco A Benatto wrote:
> Currently, if the user switch source and target parameters
> position, xfs_mdrestore truncates the dumpfile before abort
> the execution.
>
> This patch checks the target parameter and if XFS_MD_MAGIC is
> found, it aborts execution and leave dump file intact.
Yeah, I think this is a good idea. It's pretty unforgiving
to get it wrong and destroy your metadump in the process.
(though tbh I usually get a compressed dump, and
# bzcat dumpfile.bz2 | xfs_mdrestore - target
works quite well and is more foolproof)
So, despite the fact that I already talked with you about
this a bit online, I think my subconscious was working on
it in the meantime. What you've got is generally ok, but
I wonder if the goal is to detect switched arguments, if
it might be simpler to first validate the given source, and
stop if it's not a metadump file. We've already got that in
place, and it'd just be a question of checking the header
unconditionally, something like this, which just moves some
of the existing stuff work of the conditional:
xfs_metablock_t mb;
/* validate that our source file is really a metadump */
if (fread(&mb, sizeof(mb), 1, src_f) != 1)
fatal("error reading from source file: %s\n", strerror(errno));
if (be32_to_cpu(mb.mb_magic) != XFS_MD_MAGIC)
fatal("specified source file is not a metadata dump\n");
if (show_info) {
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);
It's not explicitly checking the target, but it would probably serve
the same purpose with fewer lines of new code. Either is ok with me,
really. Any thoughts?
Thanks,
-Eric
>
> Signed-off-by: Marco A Benatto <marco.antonio.780@xxxxxxxxx>
> ---
> mdrestore/xfs_mdrestore.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> index c49c13a..b4bf7b4 100644
> --- a/mdrestore/xfs_mdrestore.c
> +++ b/mdrestore/xfs_mdrestore.c
> @@ -205,7 +205,7 @@ main(
> int argc,
> char **argv)
> {
> - FILE *src_f;
> + FILE *src_f, *dst_f;
> int dst_fd;
> int c;
> int open_flags;
> @@ -277,6 +277,7 @@ main(
>
> optind++;
>
> +
> /* check and open target */
> open_flags = O_RDWR;
> is_target_file = 0;
> @@ -285,6 +286,19 @@ main(
> open_flags |= O_CREAT;
> is_target_file = 1;
> } else if (S_ISREG(statbuf.st_mode)) {
> + xfs_metablock_t mb;
> +
> + dst_f = fopen(argv[optind], "rb");
> + if (dst_f == NULL)
> + fatal("cannot open target\n");
> +
> + if (fread(&mb, sizeof(mb), 1, dst_f) == 1) {
> + if (be32_to_cpu(mb.mb_magic) == XFS_MD_MAGIC)
> + fatal("target file is a xfs_metadump. switched arguments?\n");
> + }
> +
> + fclose(dst_f);
> +
> open_flags |= O_TRUNC;
> is_target_file = 1;
> } else {
>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
