* [PATCH] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files @ 2010-01-14 21:53 Eric Sandeen 2010-01-15 18:01 ` Alex Elder 2010-01-15 18:47 ` [PATCH V2] " Eric Sandeen 0 siblings, 2 replies; 5+ messages in thread From: Eric Sandeen @ 2010-01-14 21:53 UTC (permalink / raw) To: xfs mailing list # /sbin/mkfs.xfs -dfile,name=grrr,size=100g mkfs.xfs: Use the -f option to force overwrite. check_overwrite is failing, because blkid_new_probe_from_filename() is failing, because the (new) image file is 0 length. We already return 0 (carry on!) for other internal failures, and nobody tests for a -1 return. So let's just let this one pass too. Signed-off-by: Eric Sandeen <sandeen@sandeen.net> --- (note, i'm open to comments if we want to differentiate 1/0/-1 in a cleaner fashion...) diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index 9a8eff3..53568bc 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -296,7 +296,7 @@ check_overwrite( pr = blkid_new_probe_from_filename(device); if (!pr) - return -1; + return 0; if (blkid_probe_enable_partitions(pr, 1)) goto out_free_probe; _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files 2010-01-14 21:53 [PATCH] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files Eric Sandeen @ 2010-01-15 18:01 ` Alex Elder 2010-01-15 18:47 ` [PATCH V2] " Eric Sandeen 1 sibling, 0 replies; 5+ messages in thread From: Alex Elder @ 2010-01-15 18:01 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs mailing list Eric Sandeen wrote: > # /sbin/mkfs.xfs -dfile,name=grrr,size=100g > mkfs.xfs: Use the -f option to force overwrite. > > check_overwrite is failing, because blkid_new_probe_from_filename() > is failing, because the (new) image file is 0 length. > > We already return 0 (carry on!) for other internal failures, > and nobody tests for a -1 return. So let's just let this > one pass too. I discussed this with Eric. He suggested that instead of simply ignoring the case of not recognizing the underlying file system type, we could report that, and then allow the user to intentionally overwrite it by specifying "-f". I thought that was a better approach, and he agreed to re-submit a patch that does that. -Alex > Signed-off-by: Eric Sandeen <sandeen@sandeen.net> > --- > > (note, i'm open to comments if we want to differentiate > 1/0/-1 in a cleaner fashion...) > > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > index 9a8eff3..53568bc 100644 > --- a/mkfs/xfs_mkfs.c > +++ b/mkfs/xfs_mkfs.c > @@ -296,7 +296,7 @@ check_overwrite( > > pr = blkid_new_probe_from_filename(device); > if (!pr) > - return -1; > + return 0; > > if (blkid_probe_enable_partitions(pr, 1)) > goto out_free_probe; > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH V2] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files 2010-01-14 21:53 [PATCH] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files Eric Sandeen 2010-01-15 18:01 ` Alex Elder @ 2010-01-15 18:47 ` Eric Sandeen 2010-01-15 19:11 ` Alex Elder 1 sibling, 1 reply; 5+ messages in thread From: Eric Sandeen @ 2010-01-15 18:47 UTC (permalink / raw) To: xfs mailing list # /sbin/mkfs.xfs -dfile,name=grrr,size=100g mkfs.xfs: Use the -f option to force overwrite. check_overwrite is failing, because blkid_new_probe_from_filename() is failing, because the (new) image file is 0 length. It's easy to test for 0 length, and if found, there is nothing to overwrite so return 0. Also, if testing itself failed for some reason, print a message to that effect: # mkfs/mkfs.xfs -dfile,name=newfile,size=1g mkfs.xfs: probe of newfile failed, cannot detect existing filesystem. mkfs.xfs: Use the -f option to force overwrite. Signed-off-by: Eric Sandeen <sandeen@sandeen.net> --- diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index faaafed..b6801dd 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -283,27 +283,47 @@ calc_stripe_factors( } #ifdef ENABLE_BLKID +/* + * Check for existing filesystem or partition table on device. + * Returns: + * 1 for existing fs or partition + * 0 for nothing found + * -1 for internal error + */ static int check_overwrite( char *device) { const char *type; - blkid_probe pr; - int ret = 0; + blkid_probe pr = NULL; + int ret; + struct stat statbuf; if (!device || !*device) return 0; + ret = -1; /* will reset on success of all setup calls */ + + if (stat(device, &statbuf) < 0) + goto out; + + /* nothing to overwrite on a 0-length device */ + if (statbuf.st_size == 0) { + ret = 0; + goto out; + } + pr = blkid_new_probe_from_filename(device); if (!pr) - return -1; + goto out; if (blkid_probe_enable_partitions(pr, 1)) - goto out_free_probe; + goto out; if (blkid_do_fullprobe(pr)) - goto out_free_probe; + goto out; + ret = 0; if (!blkid_probe_lookup_value(pr, "TYPE", &type, NULL)) { fprintf(stderr, _("%s: %s appears to contain an existing " @@ -316,8 +336,13 @@ check_overwrite( ret = 1; } -out_free_probe: - blkid_free_probe(pr); +out: + if (pr) + blkid_free_probe(pr); + if (ret == -1) + fprintf(stderr, + _("%s: probe of %s failed, cannot detect " + "existing filesystem.\n"), progname, device); return ret; } _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH V2] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files 2010-01-15 18:47 ` [PATCH V2] " Eric Sandeen @ 2010-01-15 19:11 ` Alex Elder 2010-01-15 22:44 ` Eric Sandeen 0 siblings, 1 reply; 5+ messages in thread From: Alex Elder @ 2010-01-15 19:11 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs mailing list Eric Sandeen wrote: > # /sbin/mkfs.xfs -dfile,name=grrr,size=100g > mkfs.xfs: Use the -f option to force overwrite. > > check_overwrite is failing, because blkid_new_probe_from_filename() > is failing, because the (new) image file is 0 length. > > It's easy to test for 0 length, and if found, there is > nothing to overwrite so return 0. > > Also, if testing itself failed for some reason, print > a message to that effect: > > # mkfs/mkfs.xfs -dfile,name=newfile,size=1g > mkfs.xfs: probe of newfile failed, cannot detect existing filesystem. > mkfs.xfs: Use the -f option to force overwrite. This looks good. > Signed-off-by: Eric Sandeen <sandeen@sandeen.net> > --- Reviewed-by: Alex Elder <aelder@sgi.com> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > index faaafed..b6801dd 100644 > --- a/mkfs/xfs_mkfs.c > +++ b/mkfs/xfs_mkfs.c > @@ -283,27 +283,47 @@ calc_stripe_factors( > } > > #ifdef ENABLE_BLKID > +/* > + * Check for existing filesystem or partition table on device. > + * Returns: > + * 1 for existing fs or partition > + * 0 for nothing found > + * -1 for internal error > + */ > static int > check_overwrite( > char *device) > { > const char *type; > - blkid_probe pr; > - int ret = 0; > + blkid_probe pr = NULL; > + int ret; > + struct stat statbuf; > > if (!device || !*device) > return 0; > > + ret = -1; /* will reset on success of all setup calls */ > + > + if (stat(device, &statbuf) < 0) > + goto out; > + > + /* nothing to overwrite on a 0-length device */ > + if (statbuf.st_size == 0) { > + ret = 0; > + goto out; > + } > + > pr = blkid_new_probe_from_filename(device); > if (!pr) > - return -1; > + goto out; > > if (blkid_probe_enable_partitions(pr, 1)) > - goto out_free_probe; > + goto out; > > if (blkid_do_fullprobe(pr)) > - goto out_free_probe; > + goto out; > > + ret = 0; > if (!blkid_probe_lookup_value(pr, "TYPE", &type, NULL)) { > fprintf(stderr, > _("%s: %s appears to contain an existing " > @@ -316,8 +336,13 @@ check_overwrite( > ret = 1; > } > > -out_free_probe: > - blkid_free_probe(pr); > +out: > + if (pr) > + blkid_free_probe(pr); > + if (ret == -1) > + fprintf(stderr, > + _("%s: probe of %s failed, cannot detect " > + "existing filesystem.\n"), progname, device); > return ret; > } > > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files 2010-01-15 19:11 ` Alex Elder @ 2010-01-15 22:44 ` Eric Sandeen 0 siblings, 0 replies; 5+ messages in thread From: Eric Sandeen @ 2010-01-15 22:44 UTC (permalink / raw) To: Alex Elder; +Cc: xfs mailing list Alex Elder wrote: > Eric Sandeen wrote: >> # /sbin/mkfs.xfs -dfile,name=grrr,size=100g >> mkfs.xfs: Use the -f option to force overwrite. >> >> check_overwrite is failing, because blkid_new_probe_from_filename() >> is failing, because the (new) image file is 0 length. >> >> It's easy to test for 0 length, and if found, there is >> nothing to overwrite so return 0. >> >> Also, if testing itself failed for some reason, print >> a message to that effect: >> >> # mkfs/mkfs.xfs -dfile,name=newfile,size=1g >> mkfs.xfs: probe of newfile failed, cannot detect existing filesystem. >> mkfs.xfs: Use the -f option to force overwrite. > > This looks good. except of course that st_size for devices is 0. Oh, I suck lately :/ Ok, will send fixup patch later. And then I'm gonna stop ;) -Eric >> Signed-off-by: Eric Sandeen <sandeen@sandeen.net> >> --- > > Reviewed-by: Alex Elder <aelder@sgi.com> > >> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c >> index faaafed..b6801dd 100644 >> --- a/mkfs/xfs_mkfs.c >> +++ b/mkfs/xfs_mkfs.c >> @@ -283,27 +283,47 @@ calc_stripe_factors( >> } >> >> #ifdef ENABLE_BLKID >> +/* >> + * Check for existing filesystem or partition table on device. >> + * Returns: >> + * 1 for existing fs or partition >> + * 0 for nothing found >> + * -1 for internal error >> + */ >> static int >> check_overwrite( >> char *device) >> { >> const char *type; >> - blkid_probe pr; >> - int ret = 0; >> + blkid_probe pr = NULL; >> + int ret; >> + struct stat statbuf; >> >> if (!device || !*device) >> return 0; >> >> + ret = -1; /* will reset on success of all setup calls */ >> + >> + if (stat(device, &statbuf) < 0) >> + goto out; >> + >> + /* nothing to overwrite on a 0-length device */ >> + if (statbuf.st_size == 0) { >> + ret = 0; >> + goto out; >> + } >> + >> pr = blkid_new_probe_from_filename(device); >> if (!pr) >> - return -1; >> + goto out; >> >> if (blkid_probe_enable_partitions(pr, 1)) >> - goto out_free_probe; >> + goto out; >> >> if (blkid_do_fullprobe(pr)) >> - goto out_free_probe; >> + goto out; >> >> + ret = 0; >> if (!blkid_probe_lookup_value(pr, "TYPE", &type, NULL)) { >> fprintf(stderr, >> _("%s: %s appears to contain an existing " >> @@ -316,8 +336,13 @@ check_overwrite( >> ret = 1; >> } >> >> -out_free_probe: >> - blkid_free_probe(pr); >> +out: >> + if (pr) >> + blkid_free_probe(pr); >> + if (ret == -1) >> + fprintf(stderr, >> + _("%s: probe of %s failed, cannot detect " >> + "existing filesystem.\n"), progname, device); >> return ret; >> } >> >> >> _______________________________________________ >> 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 > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-01-15 22:45 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-01-14 21:53 [PATCH] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files Eric Sandeen 2010-01-15 18:01 ` Alex Elder 2010-01-15 18:47 ` [PATCH V2] " Eric Sandeen 2010-01-15 19:11 ` Alex Elder 2010-01-15 22:44 ` Eric Sandeen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox