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

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

[Theodore Ts'o]

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 2/3] mke2fs: print extra information about existing ext2/3/4 file systems

[Theodore Ts'o]

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 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 09:04:03 -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 2/3] mke2fs: print extra information about existing ext2/3/4
>     file systems

Description missing.

> 
> 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);

Now I am actually confused, sorry. Which patches do I need to get this
context? I do not see this in the next branch.

Thanks!
-Lukas

>  		free(fs_type);
>  		free(fs_label);
>  		return 0;
> 

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

[Theodore Ts'o]

On Mon, May 05, 2014 at 03:45:17PM +0200, Lukáš Czerner wrote:
> 
> Now I am actually confused, sorry. Which patches do I need to get this
> context? I do not see this in the next branch.

These are based on the mke2fs patches I had sent earlier.  Sorry, I
haven't updated the maint branch in a while.

The basic idea behinid these patches is that we now get a bit more
context with the warning message for ext2/3/4 file systems:

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

I have modified those earlier patches in response to your earlier
feedback; we no longer use the 5 second delay by default --- but we
skip doing the check at all if either stdin or stdout is not a tty.
This saves us in the situation where there is some script which does
somethign like this:

mke2fs -t ext4 /dev/sdc3 > /tmp/mke2fs.out

Basically, we will only ask the user for confirmation when we are
certain a user can see both the question and be able to type a
response.  It's possible we could still get confused by someone
running mke2fs under a chat/expect script, but that seems like an
acceptable risk.


					- 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 2/3] mke2fs: print extra information about existing ext2/3/4 file systems

[Lukáš Czerner]

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

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

> Date: Mon, 5 May 2014 10:04:01 -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 2/3] mke2fs: print extra information about existing
>     ext2/3/4 file systems
> 
> On Mon, May 05, 2014 at 03:45:17PM +0200, Lukáš Czerner wrote:
> > 
> > Now I am actually confused, sorry. Which patches do I need to get this
> > context? I do not see this in the next branch.
> 
> These are based on the mke2fs patches I had sent earlier.  Sorry, I
> haven't updated the maint branch in a while.
> 
> The basic idea behinid these patches is that we now get a bit more
> context with the warning message for ext2/3/4 file systems:
> 
> % ./misc/mke2fs -t ext4 /dev/heap/media
> mke2fs 1.42.9 (4-Feb-2014)
> /dev/heap/media contains a ext4 file system labelled 'media'
> 		  last mounted on /media on Mon May  5 08:59:53 2014
> Proceed anyway? (y,n) 
> 
> I have modified those earlier patches in response to your earlier
> feedback; we no longer use the 5 second delay by default --- but we
> skip doing the check at all if either stdin or stdout is not a tty.
> This saves us in the situation where there is some script which does
> somethign like this:
> 
> mke2fs -t ext4 /dev/sdc3 > /tmp/mke2fs.out
> 
> Basically, we will only ask the user for confirmation when we are
> certain a user can see both the question and be able to type a
> response.  It's possible we could still get confused by someone
> running mke2fs under a chat/expect script, but that seems like an
> acceptable risk.

Ok, that makes sense, thanks.

-Lukas

> 
> 
> 					- Ted
> 

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

[Theodore Ts'o]

On Mon, May 05, 2014 at 10:04:01AM -0400, Theodore Ts'o wrote:
> The basic idea behinid these patches is that we now get a bit more
> context with the warning message for ext2/3/4 file systems:
> 
> % ./misc/mke2fs -t ext4 /dev/heap/media
> mke2fs 1.42.9 (4-Feb-2014)
> /dev/heap/media contains a ext4 file system labelled 'media'
> 		  last mounted on /media on Mon May  5 08:59:53 2014
> Proceed anyway? (y,n) 

And actually, the place where this context is critically important is
in this case (where let's say, the user typo's sdc3 when they meant to
type sdd3):

% ./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) 

... where this becomes a last-ditch saving through against the
accidental wiping of the enterprise's Sarbanes-Oxley records.  :-)

						- Ted

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

[Lukáš Czerner]

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

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

> Date: Mon, 5 May 2014 10:28:08 -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 2/3] mke2fs: print extra information about existing
>     ext2/3/4 file systems
> 
> On Mon, May 05, 2014 at 10:04:01AM -0400, Theodore Ts'o wrote:
> > The basic idea behinid these patches is that we now get a bit more
> > context with the warning message for ext2/3/4 file systems:
> > 
> > % ./misc/mke2fs -t ext4 /dev/heap/media
> > mke2fs 1.42.9 (4-Feb-2014)
> > /dev/heap/media contains a ext4 file system labelled 'media'
> > 		  last mounted on /media on Mon May  5 08:59:53 2014
> > Proceed anyway? (y,n) 
> 
> And actually, the place where this context is critically important is
> in this case (where let's say, the user typo's sdc3 when they meant to
> type sdd3):
> 
> % ./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) 
> 
> ... where this becomes a last-ditch saving through against the
> accidental wiping of the enterprise's Sarbanes-Oxley records.  :-)
> 
> 						- Ted

Yep, it's really useful. I just was not sure what is this all about since
there was not description and I was missing context from the other patches.

But this makes me think that it would be very useful if blkid could
gather this information for other file system if possible :). This
might be very useful if we can get some overlap with other file
system with the information provided in superblock.

Karel, what do you think ?

-Lukas

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

[Theodore Ts'o]

On Mon, May 05, 2014 at 04:38:37PM +0200, Lukáš Czerner wrote:
> > % ./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) 
> > 
> > ... where this becomes a last-ditch saving through against the
> > accidental wiping of the enterprise's Sarbanes-Oxley records.  :-)
> > 
> 
> Yep, it's really useful. I just was not sure what is this all about since
> there was not description and I was missing context from the other patches.
> 
> But this makes me think that it would be very useful if blkid could
> gather this information for other file system if possible :). This
> might be very useful if we can get some overlap with other file
> system with the information provided in superblock.

Unfortunately, as far as I know, none of the other file systems
currently save the location where the file system was last mounted.
And to be honest, the way we do it in ext4 is a horrible hack (get out
your barf bags!):

static int ext4_file_open(struct inode * inode, struct file * filp)
{
	struct super_block *sb = inode->i_sb;
	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
	struct vfsmount *mnt = filp->f_path.mnt;
	struct path path;
	char buf[64], *cp;

	if (unlikely(!(sbi->s_mount_flags & EXT4_MF_MNTDIR_SAMPLED) &&
		     !(sb->s_flags & MS_RDONLY))) {
		sbi->s_mount_flags |= EXT4_MF_MNTDIR_SAMPLED;
		/*
		 * Sample where the filesystem has been mounted and
		 * store it in the superblock for sysadmin convenience
		 * when trying to sort through large numbers of block
		 * devices or filesystem images.
		 */
		memset(buf, 0, sizeof(buf));
		path.mnt = mnt;
		path.dentry = mnt->mnt_root;
		cp = d_path(&path, buf, sizeof(buf));
		...

What we would need to do is file a feature request in the other file
systems to save this information, and then add proper support for to
pass this information from the VFS layer into the struct
super_operations's mount function, which would be the proper, sane way
to provide this functionality.

						- Ted




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

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

[Lukáš Czerner]

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

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

> Date: Mon, 5 May 2014 10:44:23 -0400
> From: Theodore Ts'o <tytso@mit.edu>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>, kzak@redhat.com,
>     linux-fsdevel@vger.kernel.org
> Subject: Re: [PATCH 2/3] mke2fs: print extra information about existing
>     ext2/3/4 file systems
> 
> On Mon, May 05, 2014 at 04:38:37PM +0200, Lukáš Czerner wrote:
> > > % ./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) 
> > > 
> > > ... where this becomes a last-ditch saving through against the
> > > accidental wiping of the enterprise's Sarbanes-Oxley records.  :-)
> > > 
> > 
> > Yep, it's really useful. I just was not sure what is this all about since
> > there was not description and I was missing context from the other patches.
> > 
> > But this makes me think that it would be very useful if blkid could
> > gather this information for other file system if possible :). This
> > might be very useful if we can get some overlap with other file
> > system with the information provided in superblock.
> 
> Unfortunately, as far as I know, none of the other file systems
> currently save the location where the file system was last mounted.
> And to be honest, the way we do it in ext4 is a horrible hack (get out
> your barf bags!):
> 
> static int ext4_file_open(struct inode * inode, struct file * filp)
> {
> 	struct super_block *sb = inode->i_sb;
> 	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> 	struct vfsmount *mnt = filp->f_path.mnt;
> 	struct path path;
> 	char buf[64], *cp;
> 
> 	if (unlikely(!(sbi->s_mount_flags & EXT4_MF_MNTDIR_SAMPLED) &&
> 		     !(sb->s_flags & MS_RDONLY))) {
> 		sbi->s_mount_flags |= EXT4_MF_MNTDIR_SAMPLED;
> 		/*
> 		 * Sample where the filesystem has been mounted and
> 		 * store it in the superblock for sysadmin convenience
> 		 * when trying to sort through large numbers of block
> 		 * devices or filesystem images.
> 		 */
> 		memset(buf, 0, sizeof(buf));
> 		path.mnt = mnt;
> 		path.dentry = mnt->mnt_root;
> 		cp = d_path(&path, buf, sizeof(buf));
> 		...
> 
> What we would need to do is file a feature request in the other file
> systems to save this information, and then add proper support for to
> pass this information from the VFS layer into the struct
> super_operations's mount function, which would be the proper, sane way
> to provide this functionality.
> 
> 						- Ted

Which would be reasonable if there was a consumer of such
information and it seemed to be useful. So I wonder what other
people think about that.

Karel, you had some suggestions about how to utilize that aside from
the mkfs...

-Lukas

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

[Theodore Ts'o]

On Mon, May 05, 2014 at 04:51:25PM +0200, Lukáš Czerner wrote:
> 
> Which would be reasonable if there was a consumer of such
> information and it seemed to be useful. So I wonder what other
> people think about that.
> 
> Karel, you had some suggestions about how to utilize that aside from
> the mkfs...

Well, if we were going to make this be more general, one other thought
I had was that might be useful to also stash the last hostname in the
superblock.  Consider the situation where you have several hundred
fibre-channel disk volumes in your SAN, and where of course the SAN
administrator hasn't done a good job naming them, and of course the
human sysadmins hadn't bothered to use file system labels.  If we were
automatically stashing the hostname into the superblock at mount time,
it might help in certain cases after the SAN directory gets smashed,
or some such.

						- 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 2/3] mke2fs: print extra information about existing ext2/3/4 file systems

[Andreas Dilger]

Note that the MMP block already contains the hostname for informational purposes. 

I'd argue that if you have several hundred SAN attached disks and they are not zoned for specific nodes then using MMP will save your bacon from foolish admins that are trying to use the same disk on multiple nodes. 

Cheers, Andreas

> On May 5, 2014, at 8:57, Theodore Ts'o <tytso@mit.edu> wrote:
> 
>> On Mon, May 05, 2014 at 04:51:25PM +0200, Lukáš Czerner wrote:
>> 
>> Which would be reasonable if there was a consumer of such
>> information and it seemed to be useful. So I wonder what other
>> people think about that.
>> 
>> Karel, you had some suggestions about how to utilize that aside from
>> the mkfs...
> 
> Well, if we were going to make this be more general, one other thought
> I had was that might be useful to also stash the last hostname in the
> superblock.  Consider the situation where you have several hundred
> fibre-channel disk volumes in your SAN, and where of course the SAN
> administrator hasn't done a good job naming them, and of course the
> human sysadmins hadn't bothered to use file system labels.  If we were
> automatically stashing the hostname into the superblock at mount time,
> it might help in certain cases after the SAN directory gets smashed,
> or some such.
> 
>                        - Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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 2/3] mke2fs: print extra information about existing ext2/3/4 file systems

[Karel Zak]

On Mon, May 05, 2014 at 04:51:25PM +0200, Lukáš Czerner wrote:
> On Mon, 5 May 2014, Theodore Ts'o wrote:
> > What we would need to do is file a feature request in the other file
> > systems to save this information, and then add proper support for to
> > pass this information from the VFS layer into the struct
> > super_operations's mount function, which would be the proper, sane way
> > to provide this functionality.
> > 
> > 						- Ted
> 
> Which would be reasonable if there was a consumer of such
> information and it seemed to be useful. So I wonder what other
> people think about that.
> 
> Karel, you had some suggestions about how to utilize that aside from
> the mkfs...

 It's really simple to add another NAME=value to the low-level part of
 the libblkid (used to feed udev db). The question is how usable will
 that, I can imagine:

   1) audit / logging purpose
   2) mount <device> --last-target
   3) automount (for example udisks and removable media)

 unfortunately 2) and 3) seem fragile as the filesystem superblocks
 have no clue about namespaces and the same filesystem is possible to
 mount in the same time to more places, etc.
 
 BTW, the current trend is to use GPT partition types to identify
 purpose of the partition filesystem (for example extra GUID for
 /home). It's FS independent solution and it allows use the right
 filesystems for the right mountpoints. It's very attractive for
 example for virtual images where you don't have to setup fstab and
 identify FS, but you still have (for example) /home on the right
 place.

     Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Theodore Ts'o]

On Mon, May 05, 2014 at 07:50:49PM +0200, Karel Zak wrote:
>    1) audit / logging purpose
>    2) mount <device> --last-target
>    3) automount (for example udisks and removable media)
> 
>  unfortunately 2) and 3) seem fragile as the filesystem superblocks
>  have no clue about namespaces and the same filesystem is possible to
>  mount in the same time to more places, etc.

Yes, "location last mounted" is really only useful as a backup
mechanism.  I've never claimed that it would be guaranteed to be the
most useful thing in the presence of bind mounts, namespaces, being
mounted in multiple locations, etc.  *Usually,* the namespace the first
time the file system is mounted is more interesting than subsequent
mounts or bind mounts, but there really is no guarantee.

>  BTW, the current trend is to use GPT partition types to identify
>  purpose of the partition filesystem (for example extra GUID for
>  /home). It's FS independent solution and it allows use the right
>  filesystems for the right mountpoints. It's very attractive for
>  example for virtual images where you don't have to setup fstab and
>  identify FS, but you still have (for example) /home on the right
>  place.

But a partition only gets one GUUID and one partition type.  So are
you saying that the GUUID partition type would be used to indicate the
concept of "this is the file system for /home", *instead* of "this is
an btrfs file system" or "this is an ext4 file system"?

Or is this some kind of GPT extension that I'm not aware of?

      	   	     	    	      	   - Ted

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

[Karel Zak]

On Mon, May 05, 2014 at 02:32:53PM -0400, Theodore Ts'o wrote:
> >  BTW, the current trend is to use GPT partition types to identify
> >  purpose of the partition filesystem (for example extra GUID for
> >  /home). It's FS independent solution and it allows use the right
> >  filesystems for the right mountpoints. It's very attractive for
> >  example for virtual images where you don't have to setup fstab and
> >  identify FS, but you still have (for example) /home on the right
> >  place.
> 
> But a partition only gets one GUUID and one partition type.  So are
> you saying that the GUUID partition type would be used to indicate the
> concept of "this is the file system for /home", *instead* of "this is
> an btrfs file system" or "this is an ext4 file system"?

 Yes, GPT partition has two UUIDs, 
 
   - UUID = an unique partition identifier
   - GUID = partition type identifier

 I talked about the type (GUID). Note that this concept is just
 another point of view how to mount partitions, I don't think it will
 be a mainstream solution for standard machines. Now it's supported by
 systemd. More details:

  http://www.freedesktop.org/wiki/Specifications/DiscoverablePartitionsSpec/
 

 For old good fstab you can use PARTUUID= to identify partitions and
 to bypass filesystem specific identifiers. In this case it's unique 
 partition identifier, no partition type.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

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

[Andreas Dilger]

The last time I asked to have the VFS add mountpoint info to the superblock, it got a strong NACK from Al Viro because of the possibility of multiple mountpoints due to bind mounts or namespaces.

I think the current hack is as good as it gets. 

Cheers, Andreas

> On May 5, 2014, at 8:44, Theodore Ts'o <tytso@mit.edu> wrote:
> 
> On Mon, May 05, 2014 at 04:38:37PM +0200, Lukáš Czerner wrote:
>>> % ./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) 
>>> 
>>> ... where this becomes a last-ditch saving through against the
>>> accidental wiping of the enterprise's Sarbanes-Oxley records.  :-)
>>> 
>> 
>> Yep, it's really useful. I just was not sure what is this all about since
>> there was not description and I was missing context from the other patches.
>> 
>> But this makes me think that it would be very useful if blkid could
>> gather this information for other file system if possible :). This
>> might be very useful if we can get some overlap with other file
>> system with the information provided in superblock.
> 
> Unfortunately, as far as I know, none of the other file systems
> currently save the location where the file system was last mounted.
> And to be honest, the way we do it in ext4 is a horrible hack (get out
> your barf bags!):
> 
> static int ext4_file_open(struct inode * inode, struct file * filp)
> {
>    struct super_block *sb = inode->i_sb;
>    struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
>    struct vfsmount *mnt = filp->f_path.mnt;
>    struct path path;
>    char buf[64], *cp;
> 
>    if (unlikely(!(sbi->s_mount_flags & EXT4_MF_MNTDIR_SAMPLED) &&
>             !(sb->s_flags & MS_RDONLY))) {
>        sbi->s_mount_flags |= EXT4_MF_MNTDIR_SAMPLED;
>        /*
>         * Sample where the filesystem has been mounted and
>         * store it in the superblock for sysadmin convenience
>         * when trying to sort through large numbers of block
>         * devices or filesystem images.
>         */
>        memset(buf, 0, sizeof(buf));
>        path.mnt = mnt;
>        path.dentry = mnt->mnt_root;
>        cp = d_path(&path, buf, sizeof(buf));
>        ...
> 
> What we would need to do is file a feature request in the other file
> systems to save this information, and then add proper support for to
> pass this information from the VFS layer into the struct
> super_operations's mount function, which would be the proper, sane way
> to provide this functionality.
> 
>                        - 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
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 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     | 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);
+	if (ret >= 0)
+		return ret;
 
 #ifdef HAVE_LINUX_MAJOR_H
 #ifndef MAJOR
-- 
1.9.0

Re: [PATCH 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 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
> 

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

[Theodore Ts'o]

On Mon, May 05, 2014 at 03:52:05PM +0200, Lukáš Czerner wrote:
> > +	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.

The issue is that e2fsprogs gets compiled for systems other than just
Linux.  I don't want to be like the assholes who work on systemd and
GNOME that simply screw over *BSD systems.  This is why I keep our
internal version of blkid in e2fsprogs, even if I do plan to use the
system blkid by default for 1.43.

If I had infinite amounts of free time, I'd backport the the new
blkid_probe interfaces to our internal version of blkid, but since I
don't, I prefer use the old interfaces for blkid as much as possible,
since that's the path of least resistance in terms of continuing to
support non-Linux users of e2fsprogs.

> 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.

How do you disable probing for superblocks with the new blkid interface?

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 3/3] mke2fs: check for a partition table and warn if present

[Lukáš Czerner]

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

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

> Date: Mon, 5 May 2014 09:58:04 -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 3/3] mke2fs: check for a partition table and warn if
>     present
> 
> On Mon, May 05, 2014 at 03:52:05PM +0200, Lukáš Czerner wrote:
> > > +	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.
> 
> The issue is that e2fsprogs gets compiled for systems other than just
> Linux.  I don't want to be like the assholes who work on systemd and
> GNOME that simply screw over *BSD systems.  This is why I keep our
> internal version of blkid in e2fsprogs, even if I do plan to use the
> system blkid by default for 1.43.
> 
> If I had infinite amounts of free time, I'd backport the the new
> blkid_probe interfaces to our internal version of blkid, but since I
> don't, I prefer use the old interfaces for blkid as much as possible,
> since that's the path of least resistance in terms of continuing to
> support non-Linux users of e2fsprogs.

Fair enough. But we should still make the use of system libblkid by
default if you do not have any objections.

Also it'll be great to mention that in the commit description that
this is the reason why we still try to use the old approach.

> 
> > 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.
> 
> How do you disable probing for superblocks with the new blkid interface?

blkid_probe_enable_superblocks(pr, 0);

Thanks!
-Lukas

> 
> Cheers,
> 
> 					- Ted
> 

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

[Theodore Ts'o]

On Mon, May 05, 2014 at 04:11:41PM +0200, Lukáš Czerner wrote:
> 
> Fair enough. But we should still make the use of system libblkid by
> default if you do not have any objections.

What I want to do is test to see if there is a system libblkid
available at all, and if so, use it by default.  I want to be able to
use the internal libblkid for development testing purposes, but the
goal is that when Andreas builds on MacOS, it should use the internal
libblkid by default (since there is no system blkid), but on Linux
systems, we should use the system blkid by default in the 1.43 branch.

> Also it'll be great to mention that in the commit description that
> this is the reason why we still try to use the old approach.

Fair enough.  Actually, we should probably put it in the code
comments.

> blkid_probe_enable_superblocks(pr, 0);

Ok, thanks.

        				- 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 3/3] mke2fs: check for a partition table and warn if present

[Karel Zak]

On Mon, May 05, 2014 at 10:20:56AM -0400, Theodore Ts'o wrote:
> On Mon, May 05, 2014 at 04:11:41PM +0200, Lukáš Czerner wrote:
> > 
> > Fair enough. But we should still make the use of system libblkid by
> > default if you do not have any objections.
> 
> What I want to do is test to see if there is a system libblkid
> available at all, and if so, use it by default.  I want to be able to
> use the internal libblkid for development testing purposes, but the
> goal is that when Andreas builds on MacOS, it should use the internal
> libblkid by default (since there is no system blkid), but on Linux
> systems, we should use the system blkid by default in the 1.43 branch.

BTW, util-linux is not Linux only (although code portability is bonus
rather than a primary goal :-). We have BSD, Solaris and GNU Hurd users. 

The plan for the next release (v2.25) is to support libs-only builds
(for example build and install libblkid and libuuid, but nothing
else).

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com
--
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 3/3] mke2fs: check for a partition table and warn if present

[Lukáš Czerner]

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

On Tue, 6 May 2014, Karel Zak wrote:

> Date: Tue, 6 May 2014 14:20:31 +0200
> From: Karel Zak <kzak@redhat.com>
> To: Theodore Ts'o <tytso@mit.edu>
> Cc: Lukáš Czerner <lczerner@redhat.com>,
>     Ext4 Developers List <linux-ext4@vger.kernel.org>
> Subject: Re: [PATCH 3/3] mke2fs: check for a partition table and warn if
>     present
> 
> On Mon, May 05, 2014 at 10:20:56AM -0400, Theodore Ts'o wrote:
> > On Mon, May 05, 2014 at 04:11:41PM +0200, Lukáš Czerner wrote:
> > > 
> > > Fair enough. But we should still make the use of system libblkid by
> > > default if you do not have any objections.
> > 
> > What I want to do is test to see if there is a system libblkid
> > available at all, and if so, use it by default.  I want to be able to
> > use the internal libblkid for development testing purposes, but the
> > goal is that when Andreas builds on MacOS, it should use the internal
> > libblkid by default (since there is no system blkid), but on Linux
> > systems, we should use the system blkid by default in the 1.43 branch.
> 
> BTW, util-linux is not Linux only (although code portability is bonus
> rather than a primary goal :-). We have BSD, Solaris and GNU Hurd users. 
> 
> The plan for the next release (v2.25) is to support libs-only builds
> (for example build and install libblkid and libuuid, but nothing
> else).
> 
>     Karel

Nice, so we're a step further to actually get rid of the libblkid in
e2fsprogs tree ? :)

-Lukas

Re: [PATCH 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 09:04:02 -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 1/3] mke2fs: print a message when creating a regular file

Description is missing.

> 
> 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)) {

I wonder whether it's possible not to have dev_size set at this point ?

>  		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);

Ah, so you're building on a previous patches. It's a bit confusing.

> +	if ((fd < 0) && (errno == ENOENT) && (flags & CREATE_FILE)) {
> +		fl |= O_CREAT;
> +		fd = open(device, fl, 0666);
> +		if (fd >= 0 && (flags & VERBOSE_CREATE))

Do we have to use VERBOSE_CREATE ? quiet is global variable so we
can as well just test that here, or am I missing something ?

Thanks!
-Lukas

> +			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);
> 

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

[Theodore Ts'o]

On Mon, May 05, 2014 at 03:41:01PM +0200, Lukáš Czerner wrote:
> On Mon, 5 May 2014, Theodore Ts'o wrote:
> 
> > Date: Mon,  5 May 2014 09:04:02 -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 1/3] mke2fs: print a message when creating a regular file
> 
> Description is missing.

I didn't think more of a description was needed; can you suggest one?

> >  	/* Can't undo discard ... */
> > -	if (!noaction && discard && (io_ptr != undo_io_manager)) {
> > +	if (!noaction && discard && dev_size && (io_ptr != undo_io_manager)) {
> 
> I wonder whether it's possible not to have dev_size set at this point ?

I checked, and I don't believe so.  ext2fs_get_device_size2() never
returns EXT2_ET_UNIMPLEMENTED any more; and it hasn't for quite some
time, since it will use st_size for a regular file or do a binary
search trying to figure out the device size in the worst case.  So in
fact, there are some code paths we can eliminate in misc/mke2fs.c
which will simplify this analysis.

Even if there is some case in the future where dev_size could be left
unset, it will be initialized to zero, at which point we will fail
safe by skipping the mke2fs_discard_device() call.

> 
> > +	if ((fd < 0) && (errno == ENOENT) && (flags & CREATE_FILE)) {
> > +		fl |= O_CREAT;
> > +		fd = open(device, fl, 0666);
> > +		if (fd >= 0 && (flags & VERBOSE_CREATE))
> 
> Do we have to use VERBOSE_CREATE ? quiet is global variable so we
> can as well just test that here, or am I missing something ?

The problem is that util.c gets used by both mke2fs.c and tune2fs.c,
and it's only a global variable for mke2fs.c.

My long-term plans is to rename libquota to libsupport, which would
never be built as a shared library, and move some of our various
internal shared utility functions (i.e., e2fsck/profile.c,
misc/util.c, etc.) into that internal e2fsprogs insternal support
library.  So in general I'd prefer to minimize the number of global
functions which misc/util.c relies upon.

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 1/3] mke2fs: print a message when creating a regular file

[Lukáš Czerner]

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

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

> Date: Mon, 5 May 2014 09:52:39 -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 1/3] mke2fs: print a message when creating a regular file
> 
> On Mon, May 05, 2014 at 03:41:01PM +0200, Lukáš Czerner wrote:
> > On Mon, 5 May 2014, Theodore Ts'o wrote:
> > 
> > > Date: Mon,  5 May 2014 09:04:02 -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 1/3] mke2fs: print a message when creating a regular file
> > 
> > Description is missing.
> 
> I didn't think more of a description was needed; can you suggest one?

something along those lines ?

In the previous patch "..." we've added an ability to automatically
create file if it does not exist prior creating the file system. We
should at least notify the user that that's the case to avoid surprises
in the case of misspelled device/file names.

> 
> > >  	/* Can't undo discard ... */
> > > -	if (!noaction && discard && (io_ptr != undo_io_manager)) {
> > > +	if (!noaction && discard && dev_size && (io_ptr != undo_io_manager)) {
> > 
> > I wonder whether it's possible not to have dev_size set at this point ?
> 
> I checked, and I don't believe so.  ext2fs_get_device_size2() never
> returns EXT2_ET_UNIMPLEMENTED any more; and it hasn't for quite some
> time, since it will use st_size for a regular file or do a binary
> search trying to figure out the device size in the worst case.  So in
> fact, there are some code paths we can eliminate in misc/mke2fs.c
> which will simplify this analysis.
> 
> Even if there is some case in the future where dev_size could be left
> unset, it will be initialized to zero, at which point we will fail
> safe by skipping the mke2fs_discard_device() call.

ok.

> 
> > 
> > > +	if ((fd < 0) && (errno == ENOENT) && (flags & CREATE_FILE)) {
> > > +		fl |= O_CREAT;
> > > +		fd = open(device, fl, 0666);
> > > +		if (fd >= 0 && (flags & VERBOSE_CREATE))
> > 
> > Do we have to use VERBOSE_CREATE ? quiet is global variable so we
> > can as well just test that here, or am I missing something ?
> 
> The problem is that util.c gets used by both mke2fs.c and tune2fs.c,
> and it's only a global variable for mke2fs.c.
> 
> My long-term plans is to rename libquota to libsupport, which would
> never be built as a shared library, and move some of our various
> internal shared utility functions (i.e., e2fsck/profile.c,
> misc/util.c, etc.) into that internal e2fsprogs insternal support
> library.  So in general I'd prefer to minimize the number of global
> functions which misc/util.c relies upon.

Ah, it's in a different file, right. Agreed.

Thanks!
-Lukas

> 
> Cheers,
> 
> 					- Ted
> 

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

[Eric Sandeen]

On 5/5/14, 8:52 AM, Theodore Ts'o wrote:
> On Mon, May 05, 2014 at 03:41:01PM +0200, Lukáš Czerner wrote:
>> On Mon, 5 May 2014, Theodore Ts'o wrote:
>>
>>> Date: Mon,  5 May 2014 09:04:02 -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 1/3] mke2fs: print a message when creating a regular file
>>
>> Description is missing.
> 
> I didn't think more of a description was needed; can you suggest one?

Well, sometimes the summary is enough - if this patch just added a printf,
the summary covers the functional change.  However, I think it's useful
to see the rationale for a change, not just the description of the change.

"Close file descriptors before exit" doesn't really need any rationale,
but changing program behavior and/or output might.

AFAIK, nobody ever complained that a changelog was too descriptive or
informative.  ;)

>>>  	/* Can't undo discard ... */
>>> -	if (!noaction && discard && (io_ptr != undo_io_manager)) {
>>> +	if (!noaction && discard && dev_size && (io_ptr != undo_io_manager)) {
>>
>> I wonder whether it's possible not to have dev_size set at this point ?
> 
> I checked, and I don't believe so.  ext2fs_get_device_size2() never
> returns EXT2_ET_UNIMPLEMENTED any more; and it hasn't for quite some
> time, since it will use st_size for a regular file or do a binary
> search trying to figure out the device size in the worst case.  So in
> fact, there are some code paths we can eliminate in misc/mke2fs.c
> which will simplify this analysis.
> 
> Even if there is some case in the future where dev_size could be left
> unset, it will be initialized to zero, at which point we will fail
> safe by skipping the mke2fs_discard_device() call.

I kind of lost the thread here; if it is impossible to be unset, why
add the check?

And if (!dev_size) what would happen on this path?  I guess I need to get
all the pending patches applied to see what's changed in this area.

-Eric

--
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 1/3] mke2fs: print a message when creating a regular file

[Theodore Ts'o]

On Mon, May 05, 2014 at 09:46:29AM -0500, Eric Sandeen wrote:
> > Even if there is some case in the future where dev_size could be left
> > unset, it will be initialized to zero, at which point we will fail
> > safe by skipping the mke2fs_discard_device() call.
> 
> I kind of lost the thread here; if it is impossible to be unset, why
> add the check?
> 
> And if (!dev_size) what would happen on this path?  I guess I need to get
> all the pending patches applied to see what's changed in this area.

It's mostly cosmetic, to be honest.  It avoids this confusing line:

5% ./misc/mke2fs -t ext4 /tmp/foo.img 16M
mke2fs 1.42.9 (4-Feb-2014)
Creating regular file /tmp/foo.img
Discarding device: done         <========== this nonsense line
Creating filesystem with 16384 1k blocks and 4096 inodes
Filesystem UUID: 515a4e52-79f1-4317-ad61-a87c492c1371
Superblock backups stored on blocks: 
	   8193
   ...

				- Ted