[PATCH -v2 1/3] mke2fs: print a message when creating a regular file

[PATCH -v2 1/3] mke2fs: print a message when creating a regular file

[Theodore Ts'o]

We've added the ability to automatically recreate a file if it doesn't
exist prior to creating the file system, since this is often used (for
example) when managing file system images for use in virtual machines.
We should at least notify the user that this is going on to avoid
surprises in the case of misspelled device/file names.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 misc/mke2fs.c |  6 ++++--
 misc/util.c   | 11 +++++++----
 misc/util.h   |  1 +
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index 2c51999..51532ef 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -103,6 +103,7 @@ static int	quotatype = -1;  /* Initialize both user and group quotas by default
 static __u64	offset;
 static blk64_t journal_location = ~0LL;
 static int	proceed_delay = -1;
+static blk64_t	dev_size;
 
 static struct ext2_super_block fs_param;
 static char *fs_uuid = NULL;
@@ -1402,7 +1403,6 @@ static void PRS(int argc, char *argv[])
 	char *		extended_opts = 0;
 	char *		fs_type = 0;
 	char *		usage_types = 0;
-	blk64_t		dev_size;
 	/*
 	 * NOTE: A few words about fs_blocks_count and blocksize:
 	 *
@@ -1768,6 +1768,8 @@ profile_error:
 	flags = CREATE_FILE;
 	if (isatty(0) && isatty(1))
 		flags |= CHECK_FS_EXIST;
+	if (!quiet)
+		flags |= VERBOSE_CREATE;
 	if (!check_plausibility(device_name, flags, &is_device) && !force)
 		proceed_question(proceed_delay);
 
@@ -2573,7 +2575,7 @@ int main (int argc, char *argv[])
 		journal_blocks = figure_journal_size(journal_size, fs);
 
 	/* Can't undo discard ... */
-	if (!noaction && discard && (io_ptr != undo_io_manager)) {
+	if (!noaction && discard && dev_size && (io_ptr != undo_io_manager)) {
 		retval = mke2fs_discard_device(fs);
 		if (!retval && io_channel_discard_zeroes_data(fs->io)) {
 			if (verbose)
diff --git a/misc/util.c b/misc/util.c
index be16ebe..15b4ce5 100644
--- a/misc/util.c
+++ b/misc/util.c
@@ -106,7 +106,7 @@ void proceed_question(int delay)
 }
 
 /*
- * return 1 if the device looks plausible
+ * return 1 if the device looks plausible, creating the file if necessary
  */
 int check_plausibility(const char *device, int flags, int *ret_is_dev)
 {
@@ -117,10 +117,13 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev)
 	char *fs_type = NULL;
 	char *fs_label = NULL;
 
-	if (flags & CREATE_FILE)
-		fl |= O_CREAT;
-
 	fd = open(device, fl, 0666);
+	if ((fd < 0) && (errno == ENOENT) && (flags & CREATE_FILE)) {
+		fl |= O_CREAT;
+		fd = open(device, fl, 0666);
+		if (fd >= 0 && (flags & VERBOSE_CREATE))
+			printf(_("Creating regular file %s\n"), device);
+	}
 	if (fd < 0) {
 		fprintf(stderr, _("Could not open %s: %s\n"),
 			device, error_message(errno));
diff --git a/misc/util.h b/misc/util.h
index 745568e..476164b 100644
--- a/misc/util.h
+++ b/misc/util.h
@@ -21,6 +21,7 @@ extern char	*journal_location_string;
 #define CHECK_BLOCK_DEV	0x0001
 #define CREATE_FILE	0x0002
 #define CHECK_FS_EXIST	0x0004
+#define VERBOSE_CREATE	0x0008
 
 #ifndef HAVE_STRCASECMP
 extern int strcasecmp (char *s1, char *s2);
-- 
1.9.0

[PATCH -v2 2/3] mke2fs: print extra information about existing ext2/3/4 file systems

[Theodore Ts'o]

The basic idea is to provide a bit more context in this situation:

% ./misc/mke2fs -t ext4 /dev/sdc3
mke2fs 1.42.9 (4-Feb-2014)
/dev/sdc3 contains a ext4 file system
Proceed anyway? (y,n)

... by adding this bit of context:

% ./misc/mke2fs -t ext4 /dev/sdc3
mke2fs 1.42.9 (4-Feb-2014)
/dev/sdc3 contains a ext4 file system
	last mounted on /SOX-backups on Mon May  5 08:59:53 2014
Proceed anyway? (y,n)

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 misc/util.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/misc/util.c b/misc/util.c
index 15b4ce5..d63e21b 100644
--- a/misc/util.c
+++ b/misc/util.c
@@ -105,6 +105,41 @@ void proceed_question(int delay)
 	signal(SIGALRM, SIG_IGN);
 }
 
+static void print_ext2_info(const char *device)
+
+{
+	struct ext2_super_block	*sb;
+	ext2_filsys		fs;
+	errcode_t		retval;
+	time_t 			tm;
+	char			buf[80];
+
+	retval = ext2fs_open2(device, 0, 0, 0, 0, unix_io_manager, &fs);
+	if (retval)
+		return;
+	sb = fs->super;
+
+	if (sb->s_mtime) {
+		tm = sb->s_mtime;
+		if (sb->s_last_mounted[0]) {
+			memset(buf, 0, sizeof(buf));
+			strncpy(buf, sb->s_last_mounted,
+				sizeof(sb->s_last_mounted));
+			printf(_("\tlast mounted on %s on %s"), buf,
+			       ctime(&tm));
+		} else
+			printf(_("\tlast mounted on %s"), ctime(&tm));
+	} else if (sb->s_mkfs_time) {
+		tm = sb->s_mkfs_time;
+		printf(_("\tcreated on %s"), ctime(&tm));
+	} else if (sb->s_mkfs_time) {
+		tm = sb->s_mtime;
+		printf(_("\tlast modified on %s"), ctime(&tm));
+	}
+	ext2fs_close(fs);
+}
+
+
 /*
  * return 1 if the device looks plausible, creating the file if necessary
  */
@@ -168,6 +203,8 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev)
 		else
 			printf(_("%s contains a %s file system\n"), device,
 			       fs_type);
+		if (strncmp(fs_type, "ext", 3) == 0)
+			print_ext2_info(device);
 		free(fs_type);
 		free(fs_label);
 		return 0;
-- 
1.9.0

Re: [PATCH -v2 2/3] mke2fs: print extra information about existing ext2/3/4 file systems

[Lukáš Czerner]

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

> Date: Mon,  5 May 2014 21:02:12 -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 -v2 2/3] mke2fs: print extra information about existing
>     ext2/3/4 file systems
> 
> The basic idea is to provide a bit more context in this situation:
> 
> % ./misc/mke2fs -t ext4 /dev/sdc3
> mke2fs 1.42.9 (4-Feb-2014)
> /dev/sdc3 contains a ext4 file system
> Proceed anyway? (y,n)
> 
> ... by adding this bit of context:
> 
> % ./misc/mke2fs -t ext4 /dev/sdc3
> mke2fs 1.42.9 (4-Feb-2014)
> /dev/sdc3 contains a ext4 file system
> 	last mounted on /SOX-backups on Mon May  5 08:59:53 2014
> Proceed anyway? (y,n)
> 
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
>  misc/util.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/misc/util.c b/misc/util.c
> index 15b4ce5..d63e21b 100644
> --- a/misc/util.c
> +++ b/misc/util.c
> @@ -105,6 +105,41 @@ void proceed_question(int delay)
>  	signal(SIGALRM, SIG_IGN);
>  }
>  
> +static void print_ext2_info(const char *device)
> +
> +{
> +	struct ext2_super_block	*sb;
> +	ext2_filsys		fs;
> +	errcode_t		retval;
> +	time_t 			tm;
> +	char			buf[80];
> +
> +	retval = ext2fs_open2(device, 0, 0, 0, 0, unix_io_manager, &fs);

Sorry for not noticing this earlier, but we might want to pass
EXT2_FLAG_64BITS.

> +	if (retval)
> +		return;
> +	sb = fs->super;
> +
> +	if (sb->s_mtime) {
> +		tm = sb->s_mtime;
> +		if (sb->s_last_mounted[0]) {
> +			memset(buf, 0, sizeof(buf));
> +			strncpy(buf, sb->s_last_mounted,
> +				sizeof(sb->s_last_mounted));
> +			printf(_("\tlast mounted on %s on %s"), buf,
> +			       ctime(&tm));
> +		} else
> +			printf(_("\tlast mounted on %s"), ctime(&tm));
> +	} else if (sb->s_mkfs_time) {
> +		tm = sb->s_mkfs_time;
> +		printf(_("\tcreated on %s"), ctime(&tm));
> +	} else if (sb->s_mkfs_time) {

This does not seem right, you've already checked for s_mkfs_time and
if was not set if you got here. I guess, it should be something else
? s_wtime perhaps ? But, can this be set when the fs was not mounted
(like using ext2fs library ?)

> +		tm = sb->s_mtime;

If you got here, then again this is not set.

Thanks!
-Lukas

> +		printf(_("\tlast modified on %s"), ctime(&tm));
> +	}
> +	ext2fs_close(fs);
> +}
> +
> +
>  /*
>   * return 1 if the device looks plausible, creating the file if necessary
>   */
> @@ -168,6 +203,8 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev)
>  		else
>  			printf(_("%s contains a %s file system\n"), device,
>  			       fs_type);
> +		if (strncmp(fs_type, "ext", 3) == 0)
> +			print_ext2_info(device);
>  		free(fs_type);
>  		free(fs_label);
>  		return 0;
> 

Re: [PATCH -v2 2/3] mke2fs: print extra information about existing ext2/3/4 file systems

[Theodore Ts'o]

On Tue, May 06, 2014 at 03:42:37PM +0200, Lukáš Czerner wrote:
> > +	retval = ext2fs_open2(device, 0, 0, 0, 0, unix_io_manager, &fs);
> 
> Sorry for not noticing this earlier, but we might want to pass
> EXT2_FLAG_64BITS.

Done.

> > +	} else if (sb->s_mkfs_time) {
> > +		tm = sb->s_mkfs_time;
> > +		printf(_("\tcreated on %s"), ctime(&tm));
> > +	} else if (sb->s_mkfs_time) {
> 
> This does not seem right, you've already checked for s_mkfs_time and
> if was not set if you got here. I guess, it should be something else
> ? s_wtime perhaps ? But, can this be set when the fs was not mounted
> (like using ext2fs library ?)
> 
> > +		tm = sb->s_mtime;
> 
> If you got here, then again this is not set.


Yes, that's a cut and paste typo, thanks for spotting it.  It should
have been:

	} else if (sb->s_mkfs_time) {
		tm = sb->s_mkfs_time;
		printf(_("\tcreated on %s"), ctime(&tm));
	} else if (sb->s_mtime) {  <========
		tm = sb->s_mtime;
		printf(_("\tlast modified on %s"), ctime(&tm));
	}


Cheers,

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -v2 2/3] mke2fs: print extra information about existing ext2/3/4 file systems

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1742 bytes --]

On Tue, 6 May 2014, Theodore Ts'o wrote:

> Date: Tue, 6 May 2014 22:46:21 -0400
> From: Theodore Ts'o <tytso@mit.edu>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>
> Subject: Re: [PATCH -v2 2/3] mke2fs: print extra information about existing
>     ext2/3/4 file systems
> 
> On Tue, May 06, 2014 at 03:42:37PM +0200, Lukáš Czerner wrote:
> > > +	retval = ext2fs_open2(device, 0, 0, 0, 0, unix_io_manager, &fs);
> > 
> > Sorry for not noticing this earlier, but we might want to pass
> > EXT2_FLAG_64BITS.
> 
> Done.
> 
> > > +	} else if (sb->s_mkfs_time) {
> > > +		tm = sb->s_mkfs_time;
> > > +		printf(_("\tcreated on %s"), ctime(&tm));
> > > +	} else if (sb->s_mkfs_time) {
> > 
> > This does not seem right, you've already checked for s_mkfs_time and
> > if was not set if you got here. I guess, it should be something else
> > ? s_wtime perhaps ? But, can this be set when the fs was not mounted
> > (like using ext2fs library ?)
> > 
> > > +		tm = sb->s_mtime;
> > 
> > If you got here, then again this is not set.
> 
> 
> Yes, that's a cut and paste typo, thanks for spotting it.  It should
> have been:
> 
> 	} else if (sb->s_mkfs_time) {
> 		tm = sb->s_mkfs_time;
> 		printf(_("\tcreated on %s"), ctime(&tm));
> 	} else if (sb->s_mtime) {  <========

But you're already checking for sb->s_mtime in the first condition,
am I missing something ?

Thanks!
-Lukas

> 		tm = sb->s_mtime;
> 		printf(_("\tlast modified on %s"), ctime(&tm));
> 	}
> 
> 
> Cheers,
> 
> 					- Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH -v2 2/3] mke2fs: print extra information about existing ext2/3/4 file systemsG

[Theodore Ts'o]

On Wed, May 07, 2014 at 10:15:56AM +0200, Lukáš Czerner wrote:
> > Yes, that's a cut and paste typo, thanks for spotting it.  It should
> > have been:
> > 
> > 	} else if (sb->s_mkfs_time) {
> > 		tm = sb->s_mkfs_time;
> > 		printf(_("\tcreated on %s"), ctime(&tm));
> > 	} else if (sb->s_mtime) {  <========
> 
> But you're already checking for sb->s_mtime in the first condition,
> am I missing something ?

The basic idea is to give one bit of context, and whatever would be
the most useful.  In order of preference, it's:

1)  Last mount directory (if available) and last mount time
2)  Time file system was created
3)  Time file system was written

#2 or #3 is only needed if the file system was never mounted.

#3 is only needed for those file systems that (a) were never mounted,
(b) was modified/filled via e2tools or debugfs.  (Or Windows FS SDK
based hacks, etc.)

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -v2 2/3] mke2fs: print extra information about existing ext2/3/4 file systemsGjj

[Lukáš Czerner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3301 bytes --]

On Wed, 7 May 2014, Theodore Ts'o wrote:

> Date: Wed, 7 May 2014 08:39:13 -0400
> From: Theodore Ts'o <tytso@mit.edu>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>
> Subject: Re: [PATCH -v2 2/3] mke2fs: print extra information about existing
>     ext2/3/4 file systemsG
> 
> On Wed, May 07, 2014 at 10:15:56AM +0200, Lukáš Czerner wrote:
> > > Yes, that's a cut and paste typo, thanks for spotting it.  It should
> > > have been:
> > > 
> > > 	} else if (sb->s_mkfs_time) {
> > > 		tm = sb->s_mkfs_time;
> > > 		printf(_("\tcreated on %s"), ctime(&tm));
> > > 	} else if (sb->s_mtime) {  <========
> > 
> > But you're already checking for sb->s_mtime in the first condition,
> > am I missing something ?
> 
> The basic idea is to give one bit of context, and whatever would be
> the most useful.  In order of preference, it's:
> 
> 1)  Last mount directory (if available) and last mount time
> 2)  Time file system was created
> 3)  Time file system was written
> 
> #2 or #3 is only needed if the file system was never mounted.
> 
> #3 is only needed for those file systems that (a) were never mounted,
> (b) was modified/filled via e2tools or debugfs.  (Or Windows FS SDK
> based hacks, etc.)
> 
> 					- Ted
> 

I understand that, but here is what is in your patch:


 +      if (sb->s_mtime) {
 +              tm = sb->s_mtime;
 +              if (sb->s_last_mounted[0]) {
 +                      memset(buf, 0, sizeof(buf));
 +                      strncpy(buf, sb->s_last_mounted,
 +                              sizeof(sb->s_last_mounted));
 +                      printf(_("\tlast mounted on %s on %s"), buf,
 +                             ctime(&tm));
 +              } else
 +                      printf(_("\tlast mounted on %s"), ctime(&tm));
 +      } else if (sb->s_mkfs_time) {
 +              tm = sb->s_mkfs_time;
 +              printf(_("\tcreated on %s"), ctime(&tm));
 +      } else if (sb->s_mkfs_time) {
 +              tm = sb->s_mtime;
 +              printf(_("\tlast modified on %s"), ctime(&tm));
 +      }

Now you're saying that the last condition should really be

	} else if (sb->s_mtime) {  <========

But that does not make sense because it's the same as the first
condition, so it would either never get there, or never be true.

So it really should be

	} else if (sb->s_wtime) {


so the whole thing should look like:

 +      if (sb->s_mtime) {
 +              tm = sb->s_mtime;
 +              if (sb->s_last_mounted[0]) {
 +                      memset(buf, 0, sizeof(buf));
 +                      strncpy(buf, sb->s_last_mounted,
 +                              sizeof(sb->s_last_mounted));
 +                      printf(_("\tlast mounted on %s on %s"), buf,
 +                             ctime(&tm));
 +              } else
 +                      printf(_("\tlast mounted on %s"), ctime(&tm));
 +      } else if (sb->s_mkfs_time) {
 +              tm = sb->s_mkfs_time;
 +              printf(_("\tcreated on %s"), ctime(&tm));
 +      } else if (sb->s_wtime) {
 +              tm = sb->s_wtime;
 +              printf(_("\tlast modified on %s"), ctime(&tm));
 +      }

Also I wonder whether we need to use 'tm' variable, can't we use the
sb->s_*time directly ? But that's nitpicking.

Thanks!
-Lukas

Re: [PATCH -v2 2/3] mke2fs: print extra information about existing ext2/3/4 file systemsGjj

[Theodore Ts'o]

On Wed, May 07, 2014 at 03:43:09PM +0200, Lukáš Czerner wrote:
> so the whole thing should look like:
> 
>  +      if (sb->s_mtime) {
>  +              tm = sb->s_mtime;
>  +              if (sb->s_last_mounted[0]) {
>  +                      memset(buf, 0, sizeof(buf));
>  +                      strncpy(buf, sb->s_last_mounted,
>  +                              sizeof(sb->s_last_mounted));
>  +                      printf(_("\tlast mounted on %s on %s"), buf,
>  +                             ctime(&tm));
>  +              } else
>  +                      printf(_("\tlast mounted on %s"), ctime(&tm));
>  +      } else if (sb->s_mkfs_time) {
>  +              tm = sb->s_mkfs_time;
>  +              printf(_("\tcreated on %s"), ctime(&tm));
>  +      } else if (sb->s_wtime) {
>  +              tm = sb->s_wtime;
>  +              printf(_("\tlast modified on %s"), ctime(&tm));
>  +      }

Sorry, that's in fact what I have.

> Also I wonder whether we need to use 'tm' variable, can't we use the
> sb->s_*time directly ? But that's nitpicking.

That's because sb->s_mkfs_time is a __u32, and time_t isn't
necessarily be a 32-bit type (and in fact isn't on x86_64, x32,
apropos of current discussion happening on ksummit-discuss.)  Yes,
that means the ext4 superblock has a 2038 problem, but that's a
separate issue that we should fix one of these days....

	 	    	  	 	       - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Theodore Ts'o]

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     | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
 4 files changed, 59 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..ce3c416 100644
--- a/misc/util.c
+++ b/misc/util.c
@@ -139,13 +139,54 @@ 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_probe_enable_superblocks(pr, 0);
+	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;
@@ -189,6 +230,15 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev)
 		return 0;
 	}
 
+	/*
+	 * Note: we use the older-style blkid API's here because we
+	 * want as much functionality to be available when using the
+	 * internal blkid library, when e2fsprogs is compiled for
+	 * non-Linux systems that will probably not have the libraries
+	 * from util-linux available.  We only use the newer
+	 * blkid-probe interfaces to access functionality not
+	 * available in the original blkid library.
+	 */
 	if ((flags & CHECK_FS_EXIST) && blkid_get_cache(&cache, NULL) >= 0) {
 		fs_type = blkid_get_tag_value(cache, "TYPE", device);
 		if (fs_type)
@@ -210,12 +260,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);
+	if (ret >= 0)
+		return ret;
 
 #ifdef HAVE_LINUX_MAJOR_H
 #ifndef MAJOR
-- 
1.9.0

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

[Lukáš Czerner]

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

> Date: Mon,  5 May 2014 21:02:13 -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 -v2 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     | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
>  4 files changed, 59 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..ce3c416 100644
> --- a/misc/util.c
> +++ b/misc/util.c
> @@ -139,13 +139,54 @@ 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)

Wrong indentation (spaces instead of tab), but otherwise it looks
good. Thanks!

Reviewed-by: Lukas Czerner <lczerner@redhat.com>

> +		goto errout;
> +
> +	ret = blkid_probe_enable_superblocks(pr, 0);
> +	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;
> @@ -189,6 +230,15 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev)
>  		return 0;
>  	}
>  
> +	/*
> +	 * Note: we use the older-style blkid API's here because we
> +	 * want as much functionality to be available when using the
> +	 * internal blkid library, when e2fsprogs is compiled for
> +	 * non-Linux systems that will probably not have the libraries
> +	 * from util-linux available.  We only use the newer
> +	 * blkid-probe interfaces to access functionality not
> +	 * available in the original blkid library.
> +	 */
>  	if ((flags & CHECK_FS_EXIST) && blkid_get_cache(&cache, NULL) >= 0) {
>  		fs_type = blkid_get_tag_value(cache, "TYPE", device);
>  		if (fs_type)
> @@ -210,12 +260,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);
> +	if (ret >= 0)
> +		return ret;
>  
>  #ifdef HAVE_LINUX_MAJOR_H
>  #ifndef MAJOR
> 

Re: [PATCH -v2 1/3] mke2fs: print a message when creating a regular file

[Lukáš Czerner]

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

> Date: Mon,  5 May 2014 21:02:11 -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 -v2 1/3] mke2fs: print a message when creating a regular file
> 
> We've added the ability to automatically recreate a file if it doesn't
> exist prior to creating the file system, since this is often used (for
> example) when managing file system images for use in virtual machines.
> We should at least notify the user that this is going on to avoid
> surprises in the case of misspelled device/file names.

Looks good, thanks!

Reviewed-by: Lukas Czerner <lczerner@redhat.com>

> 
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
>  misc/mke2fs.c |  6 ++++--
>  misc/util.c   | 11 +++++++----
>  misc/util.h   |  1 +
>  3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> index 2c51999..51532ef 100644
> --- a/misc/mke2fs.c
> +++ b/misc/mke2fs.c
> @@ -103,6 +103,7 @@ static int	quotatype = -1;  /* Initialize both user and group quotas by default
>  static __u64	offset;
>  static blk64_t journal_location = ~0LL;
>  static int	proceed_delay = -1;
> +static blk64_t	dev_size;
>  
>  static struct ext2_super_block fs_param;
>  static char *fs_uuid = NULL;
> @@ -1402,7 +1403,6 @@ static void PRS(int argc, char *argv[])
>  	char *		extended_opts = 0;
>  	char *		fs_type = 0;
>  	char *		usage_types = 0;
> -	blk64_t		dev_size;
>  	/*
>  	 * NOTE: A few words about fs_blocks_count and blocksize:
>  	 *
> @@ -1768,6 +1768,8 @@ profile_error:
>  	flags = CREATE_FILE;
>  	if (isatty(0) && isatty(1))
>  		flags |= CHECK_FS_EXIST;
> +	if (!quiet)
> +		flags |= VERBOSE_CREATE;
>  	if (!check_plausibility(device_name, flags, &is_device) && !force)
>  		proceed_question(proceed_delay);
>  
> @@ -2573,7 +2575,7 @@ int main (int argc, char *argv[])
>  		journal_blocks = figure_journal_size(journal_size, fs);
>  
>  	/* Can't undo discard ... */
> -	if (!noaction && discard && (io_ptr != undo_io_manager)) {
> +	if (!noaction && discard && dev_size && (io_ptr != undo_io_manager)) {
>  		retval = mke2fs_discard_device(fs);
>  		if (!retval && io_channel_discard_zeroes_data(fs->io)) {
>  			if (verbose)
> diff --git a/misc/util.c b/misc/util.c
> index be16ebe..15b4ce5 100644
> --- a/misc/util.c
> +++ b/misc/util.c
> @@ -106,7 +106,7 @@ void proceed_question(int delay)
>  }
>  
>  /*
> - * return 1 if the device looks plausible
> + * return 1 if the device looks plausible, creating the file if necessary
>   */
>  int check_plausibility(const char *device, int flags, int *ret_is_dev)
>  {
> @@ -117,10 +117,13 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev)
>  	char *fs_type = NULL;
>  	char *fs_label = NULL;
>  
> -	if (flags & CREATE_FILE)
> -		fl |= O_CREAT;
> -
>  	fd = open(device, fl, 0666);
> +	if ((fd < 0) && (errno == ENOENT) && (flags & CREATE_FILE)) {
> +		fl |= O_CREAT;
> +		fd = open(device, fl, 0666);
> +		if (fd >= 0 && (flags & VERBOSE_CREATE))
> +			printf(_("Creating regular file %s\n"), device);
> +	}
>  	if (fd < 0) {
>  		fprintf(stderr, _("Could not open %s: %s\n"),
>  			device, error_message(errno));
> diff --git a/misc/util.h b/misc/util.h
> index 745568e..476164b 100644
> --- a/misc/util.h
> +++ b/misc/util.h
> @@ -21,6 +21,7 @@ extern char	*journal_location_string;
>  #define CHECK_BLOCK_DEV	0x0001
>  #define CREATE_FILE	0x0002
>  #define CHECK_FS_EXIST	0x0004
> +#define VERBOSE_CREATE	0x0008
>  
>  #ifndef HAVE_STRCASECMP
>  extern int strcasecmp (char *s1, char *s2);
>