Re: [PATCH v2] mkfs.xfs fix detection of empty devices

[Eric Sandeen <sandeen@sandeen.net>]

Christoph Hellwig wrote:
> We currently fail to detect that a device does indeed not contain any
> signature and we are indeed fine to proceed with it due to mishandling
> the return value of blkid_do_fullprobe.  Fix that up and add some
> better diagnostics of the blkid detection.
> 
> from RH bugzilla https://bugzilla.redhat.com/show_bug.cgi?id=561870
> 
> # dd if=/dev/zero of=k bs=1MB count=2 seek=20; mkfs.xfs k 
> # mkfs.xfs: probe of k failed, cannot detect existing filesystem.
> # mkfs.xfs: Use the -f option to force overwrite
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

we discussed keeping the 0-size check until libblkid copes better;
also the absense of ret = !ret looks more readable, thanks.

Reviewed-by: Eric Sandeen <sandeen@sandeen.net>

> Index: xfsprogs-dev/mkfs/xfs_mkfs.c
> ===================================================================
> --- xfsprogs-dev.orig/mkfs/xfs_mkfs.c	2010-02-04 20:30:26.000000000 +0000
> +++ xfsprogs-dev/mkfs/xfs_mkfs.c	2010-02-04 20:43:41.000000000 +0000
> @@ -322,24 +322,40 @@ check_overwrite(
>  	if (!pr)
>  		goto out;
>  
> -	if (blkid_probe_enable_partitions(pr, 1))
> +	ret = blkid_probe_enable_partitions(pr, 1);
> +	if (ret < 0)
>  		goto out;
>  
> -	if (blkid_do_fullprobe(pr))
> +	ret = blkid_do_fullprobe(pr);
> +	if (ret < 0)
>  		goto out;
>  
> -	ret = 0;
> +	/*
> +	 * Blkid returns 1 for nothing found and 0 when it finds a signature,
> +	 * but we want the exact opposite, so reverse the return value here.
> +	 *
> +	 * In addition print some useful diagnostics about what actually is
> +	 * on the device.
> +	 */
> +	if (ret) {
> +		ret = 0;
> +		goto out;
> +	}
> +
>  	if (!blkid_probe_lookup_value(pr, "TYPE", &type, NULL)) {
>  		fprintf(stderr,
>  			_("%s: %s appears to contain an existing "
>  			"filesystem (%s).\n"), progname, device, type);
> -		ret = 1;
>  	} else if (!blkid_probe_lookup_value(pr, "PTTYPE", &type, NULL)) {
>  		fprintf(stderr,
>  			_("%s: %s appears to contain a partition "
>  			"table (%s).\n"), progname, device, type);
> -		ret = 1;
> +	} else {
> +		fprintf(stderr,
> +			_("%s: %s appears to contain something weird "
> +			"according to blkid\n"), progname, device);
>  	}
> +	ret = 1;
>  
>  out:
>  	if (pr)
> 
> _______________________________________________
> 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