Re: [PATCH 3/3] mke2fs: check for a partition table and warn if present

["Lukáš Czerner" <lczerner@redhat.com>]

On Mon, 5 May 2014, Theodore Ts'o wrote:

> Date: Mon,  5 May 2014 09:04:04 -0400
> From: Theodore Ts'o <tytso@mit.edu>
> To: Ext4 Developers List <linux-ext4@vger.kernel.org>
> Cc: Theodore Ts'o <tytso@mit.edu>
> Subject: [PATCH 3/3] mke2fs: check for a partition table and warn if present
> 
> This supercedes the "whole disk" check, since it does a better job and
> there are times when it is quite legitimate to want to use the whole
> disk.
> 
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
>  configure       |  2 +-
>  configure.in    |  1 +
>  lib/config.h.in |  3 +++
>  misc/util.c     | 48 +++++++++++++++++++++++++++++++++++++++++-------
>  4 files changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/configure b/configure
> index 44664c3..6fe33f5 100755
> --- a/configure
> +++ b/configure
> @@ -11078,7 +11078,7 @@ if test "$ac_res" != no; then :
>  fi
>  
>  fi
> -for ac_func in  	__secure_getenv 	backtrace 	blkid_probe_get_topology 	chflags 	fadvise64 	fallocate 	fallocate64 	fchown 	fdatasync 	fstat64 	ftruncate64 	futimes 	getcwd 	getdtablesize 	getmntinfo 	getpwuid_r 	getrlimit 	getrusage 	jrand48 	llseek 	lseek64 	mallinfo 	mbstowcs 	memalign 	mempcpy 	mmap 	msync 	nanosleep 	open64 	pathconf 	posix_fadvise 	posix_fadvise64 	posix_memalign 	prctl 	secure_getenv 	setmntent 	setresgid 	setresuid 	srandom 	stpcpy 	strcasecmp 	strdup 	strnlen 	strptime 	strtoull 	sync_file_range 	sysconf 	usleep 	utime 	valloc
> +for ac_func in  	__secure_getenv 	backtrace 	blkid_probe_get_topology 	blkid_probe_enable_partitions 	chflags 	fadvise64 	fallocate 	fallocate64 	fchown 	fdatasync 	fstat64 	ftruncate64 	futimes 	getcwd 	getdtablesize 	getmntinfo 	getpwuid_r 	getrlimit 	getrusage 	jrand48 	llseek 	lseek64 	mallinfo 	mbstowcs 	memalign 	mempcpy 	mmap 	msync 	nanosleep 	open64 	pathconf 	posix_fadvise 	posix_fadvise64 	posix_memalign 	prctl 	secure_getenv 	setmntent 	setresgid 	setresuid 	srandom 	stpcpy 	strcasecmp 	strdup 	strnlen 	strptime 	strtoull 	sync_file_range 	sysconf 	usleep 	utime 	valloc
>  do :
>    as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
>  ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
> diff --git a/configure.in b/configure.in
> index e0e6d48..781b6f5 100644
> --- a/configure.in
> +++ b/configure.in
> @@ -1050,6 +1050,7 @@ AC_CHECK_FUNCS(m4_flatten([
>  	__secure_getenv
>  	backtrace
>  	blkid_probe_get_topology
> +	blkid_probe_enable_partitions
>  	chflags
>  	fadvise64
>  	fallocate
> diff --git a/lib/config.h.in b/lib/config.h.in
> index b575a5c..92b3c49 100644
> --- a/lib/config.h.in
> +++ b/lib/config.h.in
> @@ -55,6 +55,9 @@
>  /* Define to 1 if you have the `backtrace' function. */
>  #undef HAVE_BACKTRACE
>  
> +/* Define to 1 if you have the `blkid_probe_enable_partitions' function. */
> +#undef HAVE_BLKID_PROBE_ENABLE_PARTITIONS
> +
>  /* Define to 1 if you have the `blkid_probe_get_topology' function. */
>  #undef HAVE_BLKID_PROBE_GET_TOPOLOGY
>  
> diff --git a/misc/util.c b/misc/util.c
> index d63e21b..d0e4c7e 100644
> --- a/misc/util.c
> +++ b/misc/util.c
> @@ -139,13 +139,50 @@ static void print_ext2_info(const char *device)
>  	ext2fs_close(fs);
>  }
>  
> +/*
> + * return 1 if there is no partition table, 0 if a partition table is
> + * detected, and -1 on an error.
> + */
> +static int check_partition_table(const char *device)
> +{
> +#ifdef HAVE_BLKID_PROBE_ENABLE_PARTITIONS
> +	blkid_probe pr;
> +	const char *value;
> +	int ret;
> +
> +	pr = blkid_new_probe_from_filename(device);
> +	if (!pr)
> +		return -1;
> +
> +        ret = blkid_probe_enable_partitions(pr, 1);
> +        if (ret < 0)
> +		goto errout;
> +
> +	ret = blkid_do_fullprobe(pr);
> +	if (ret < 0)
> +		goto errout;
> +
> +	ret = blkid_probe_lookup_value(pr, "PTTYPE", &value, NULL);
> +	if (ret == 0)
> +		fprintf(stderr, _("Found a %s partition table in %s\n"),
> +			value, device);
> +	else
> +		ret = 1;
> +
> +errout:
> +	blkid_free_probe(pr);
> +	return ret;
> +#else
> +	return -1;
> +#endif
> +}
>  
>  /*
>   * return 1 if the device looks plausible, creating the file if necessary
>   */
>  int check_plausibility(const char *device, int flags, int *ret_is_dev)
>  {
> -	int fd, is_dev = 0;
> +	int fd, ret, is_dev = 0;
>  	ext2fs_struct_stat s;
>  	int fl = O_RDONLY;
>  	blkid_cache cache = NULL;
> @@ -210,12 +247,9 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev)
>  		return 0;
>  	}
>  
> -	/*
> -	 * We should eventually replace this with a test for the
> -	 * presence of a partition table.  Unfortunately the blkid
> -	 * library doesn't test for partition tabels, and checking for
> -	 * valid GPT and MBR and possibly others isn't quite trivial.
> -	 */
> +	ret = check_partition_table(device);

This can be actually used to check more than just partitions. So we
can use this approach to check for all rather than having separate
checks for file system signatures and partitions.

Also in your check_partition_table() you do not disable probing for
supeblocks even though you do not look to them afterwards so it's
not a big deal. But again I think that we can use it to check for
all signatures.

Thanks!
-Lukas

> +	if (ret >= 0)
> +		return ret;
>  
>  #ifdef HAVE_LINUX_MAJOR_H
>  #ifndef MAJOR
>