[PATCH] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files

[PATCH] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files

[Eric Sandeen]

# /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

RE: [PATCH] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files

[Alex Elder]

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

[PATCH V2] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files

[Eric Sandeen]

# /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

RE: [PATCH V2] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files

[Alex Elder]

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

Re: [PATCH V2] mkfs: fix mkfs.xfs -dfile,name=$NAME for new files

[Eric Sandeen]

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