* [PATCH] xfs_mdrestore: Don't restore over a dump file @ 2018-02-01 20:37 Marco A Benatto 2018-02-01 20:50 ` Eric Sandeen 2018-02-01 21:54 ` Bill O'Donnell 0 siblings, 2 replies; 6+ messages in thread From: Marco A Benatto @ 2018-02-01 20:37 UTC (permalink / raw) To: linux-xfs 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. Signed-off-by: Marco A Benatto <marco.antonio.780@gmail.com> --- 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 { -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs_mdrestore: Don't restore over a dump file 2018-02-01 20:37 [PATCH] xfs_mdrestore: Don't restore over a dump file Marco A Benatto @ 2018-02-01 20:50 ` Eric Sandeen 2018-02-01 22:28 ` Darrick J. Wong 2018-02-01 21:54 ` Bill O'Donnell 1 sibling, 1 reply; 6+ messages in thread From: Eric Sandeen @ 2018-02-01 20:50 UTC (permalink / raw) To: Marco A Benatto, linux-xfs 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@gmail.com> > --- > 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 { > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs_mdrestore: Don't restore over a dump file 2018-02-01 20:50 ` Eric Sandeen @ 2018-02-01 22:28 ` Darrick J. Wong 2018-02-01 22:39 ` Eric Sandeen 0 siblings, 1 reply; 6+ messages in thread From: Darrick J. Wong @ 2018-02-01 22:28 UTC (permalink / raw) To: Eric Sandeen; +Cc: Marco A Benatto, linux-xfs On Thu, Feb 01, 2018 at 02:50:19PM -0600, Eric Sandeen wrote: > 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); So now you got /me/ looking at mdrestore and wondering, if the metadump source is a pipe (as in your example above), won't this seek fail? We already read the metadump super from the stream, so we can't seek backwards and read it again: # cat foo.md | xfs_mdrestore -i - /dev/sda -: not obfuscated, clean log, full metadata blocks xfs_mdrestore: specified file is not a metadata dump Not to mention xfs_mdrestore --help doesn't tell you there's an -i option. Also, we already /do/ check that the source actually has a metadump header, the problem is that we open the destination O_TRUNC before we read the header and so if the target was a file, *poof* it's empty. I think we'll need to fix both of those problems. Maybe we solve it by opening src_f, checking the magic, optionally printing the metadump info, and then passing both src_f and the header into perform_restore? Separate patch, though. > 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@gmail.com> > > --- > > 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 could be declared here, right? > > + > > + 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"); > > + } if (fread(&mb, sizeof(mb), 1, dst_f) == 1 && mb.mb_magic == cpu_to_be32(XFS_MD_MAGIC)) fatal("..."); ? --D > > + > > + 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs_mdrestore: Don't restore over a dump file 2018-02-01 22:28 ` Darrick J. Wong @ 2018-02-01 22:39 ` Eric Sandeen 2018-02-02 17:52 ` Marco Benatto 0 siblings, 1 reply; 6+ messages in thread From: Eric Sandeen @ 2018-02-01 22:39 UTC (permalink / raw) To: Darrick J. Wong; +Cc: Marco A Benatto, linux-xfs On 2/1/18 4:28 PM, Darrick J. Wong wrote: > On Thu, Feb 01, 2018 at 02:50:19PM -0600, Eric Sandeen wrote: >> 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); > > So now you got /me/ looking at mdrestore and wondering, if the metadump > source is a pipe (as in your example above), won't this seek fail? We > already read the metadump super from the stream, so we can't seek > backwards and read it again: > > # cat foo.md | xfs_mdrestore -i - /dev/sda > -: not obfuscated, clean log, full metadata blocks > xfs_mdrestore: specified file is not a metadata dump > > Not to mention xfs_mdrestore --help doesn't tell you there's an -i > option. <there is no --help> ;) Neato! > Also, we already /do/ check that the source actually has a metadump > header, the problem is that we open the destination O_TRUNC before we > read the header and so if the target was a file, *poof* it's empty. hhmm yeah. > I think we'll need to fix both of those problems. Maybe we solve it by > opening src_f, checking the magic, optionally printing the metadump > info, and then passing both src_f and the header into perform_restore? > > Separate patch, though. right, and make perform_restore know that we're already past the first header block, right. Simply moving the first fread out of perform_restore would do the trick. Would need to pass the read metablock into the restore function. yeah that makes sense. >> 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@gmail.com> >>> --- >>> 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 could be declared here, right? > >>> + >>> + 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"); >>> + } > > if (fread(&mb, sizeof(mb), 1, dst_f) == 1 && > mb.mb_magic == cpu_to_be32(XFS_MD_MAGIC)) > fatal("..."); *nod* -Eric ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs_mdrestore: Don't restore over a dump file 2018-02-01 22:39 ` Eric Sandeen @ 2018-02-02 17:52 ` Marco Benatto 0 siblings, 0 replies; 6+ messages in thread From: Marco Benatto @ 2018-02-02 17:52 UTC (permalink / raw) To: Eric Sandeen; +Cc: Darrick J. Wong, linux-xfs Thanks all for the feedback and nice catch regarding the rewind + stdin reading. I'm currently working on a patch which should cover (following Sandeen's suggestions) the items Darrick has brought. I should send this soon. Thanks, On Thu, Feb 1, 2018 at 8:39 PM, Eric Sandeen <sandeen@sandeen.net> wrote: > On 2/1/18 4:28 PM, Darrick J. Wong wrote: >> On Thu, Feb 01, 2018 at 02:50:19PM -0600, Eric Sandeen wrote: >>> 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); >> >> So now you got /me/ looking at mdrestore and wondering, if the metadump >> source is a pipe (as in your example above), won't this seek fail? We >> already read the metadump super from the stream, so we can't seek >> backwards and read it again: >> >> # cat foo.md | xfs_mdrestore -i - /dev/sda >> -: not obfuscated, clean log, full metadata blocks >> xfs_mdrestore: specified file is not a metadata dump >> >> Not to mention xfs_mdrestore --help doesn't tell you there's an -i >> option. > > <there is no --help> ;) > > Neato! > >> Also, we already /do/ check that the source actually has a metadump >> header, the problem is that we open the destination O_TRUNC before we >> read the header and so if the target was a file, *poof* it's empty. > > hhmm yeah. > >> I think we'll need to fix both of those problems. Maybe we solve it by >> opening src_f, checking the magic, optionally printing the metadump >> info, and then passing both src_f and the header into perform_restore? >> >> Separate patch, though. > > right, and make perform_restore know that we're already past the first > header block, right. Simply moving the first fread out of perform_restore > would do the trick. Would need to pass the read metablock into the > restore function. > > yeah that makes sense. > >>> 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@gmail.com> >>>> --- >>>> 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 could be declared here, right? >> >>>> + >>>> + 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"); >>>> + } >> >> if (fread(&mb, sizeof(mb), 1, dst_f) == 1 && >> mb.mb_magic == cpu_to_be32(XFS_MD_MAGIC)) >> fatal("..."); > > *nod* > > -Eric -- Marco Antonio Benatto Linux user ID: #506236 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs_mdrestore: Don't restore over a dump file 2018-02-01 20:37 [PATCH] xfs_mdrestore: Don't restore over a dump file Marco A Benatto 2018-02-01 20:50 ` Eric Sandeen @ 2018-02-01 21:54 ` Bill O'Donnell 1 sibling, 0 replies; 6+ messages in thread From: Bill O'Donnell @ 2018-02-01 21:54 UTC (permalink / raw) To: Marco A Benatto; +Cc: linux-xfs On Thu, Feb 01, 2018 at 06:37:43PM -0200, 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. > > Signed-off-by: Marco A Benatto <marco.antonio.780@gmail.com> a few nits...otherwise, Reviewed-by: Bill O'Donnell <billodo@redhat.com> > --- > 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++; > > + ^^^ extra space. > /* 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"); ^^^ exceeded 80 columns...probably ok for stdout messages... shrug. > + } > + > + fclose(dst_f); > + > open_flags |= O_TRUNC; > is_target_file = 1; > } else { > -- > 1.8.3.1 > > -- > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-02-02 17:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-02-01 20:37 [PATCH] xfs_mdrestore: Don't restore over a dump file Marco A Benatto 2018-02-01 20:50 ` Eric Sandeen 2018-02-01 22:28 ` Darrick J. Wong 2018-02-01 22:39 ` Eric Sandeen 2018-02-02 17:52 ` Marco Benatto 2018-02-01 21:54 ` Bill O'Donnell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).