Message ID | 1398556834-31913-6-git-send-email-tytso@mit.edu |
---|---|
State | Accepted, archived |
Headers | show |
On Sat, 26 Apr 2014, Theodore Ts'o wrote: > Date: Sat, 26 Apr 2014 20:00:33 -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 6/7] mke2fs: check for pre-existing file system > > Warn the system administrator if there is an existing file system on > the block device, and give the administrator an opportunity to abort > the mkfs operation. > > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> > --- > misc/mke2fs.c | 4 +++- > misc/util.c | 29 +++++++++++++++++++++++++++++ > misc/util.h | 1 + > 3 files changed, 33 insertions(+), 1 deletion(-) > > diff --git a/misc/mke2fs.c b/misc/mke2fs.c > index 799132a..3694ce5 100644 > --- a/misc/mke2fs.c > +++ b/misc/mke2fs.c > @@ -1753,7 +1753,9 @@ profile_error: > profile_get_integer(profile, "options", "proceed_delay", 0, 5, > &proceed_delay); > > - if (!check_plausibility(device_name, CREATE_FILE, > + /* The isatty() test is so we don't break existing scripts */ > + if (!check_plausibility(device_name, CREATE_FILE | > + (isatty(0) ? CHECK_FS_EXIST : 0), > &is_device) && !force) > proceed_question(proceed_delay); > > diff --git a/misc/util.c b/misc/util.c > index afb0058..be16ebe 100644 > --- a/misc/util.c > +++ b/misc/util.c > @@ -113,6 +113,9 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev) > int fd, is_dev = 0; > ext2fs_struct_stat s; > int fl = O_RDONLY; > + blkid_cache cache = NULL; > + char *fs_type = NULL; > + char *fs_label = NULL; > > if (flags & CREATE_FILE) > fl |= O_CREAT; > @@ -148,6 +151,32 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev) > return 0; > } > > + if ((flags & CHECK_FS_EXIST) && blkid_get_cache(&cache, NULL) >= 0) { > + fs_type = blkid_get_tag_value(cache, "TYPE", device); > + if (fs_type) > + fs_label = blkid_get_tag_value(cache, "LABEL", device); > + blkid_put_cache(cache); > + } > + > + if (fs_type) { > + if (fs_label) > + printf(_("%s contains a %s file system " > + "labelled '%s'\n"), device, fs_type, fs_label); > + else > + printf(_("%s contains a %s file system\n"), device, > + fs_type); > + free(fs_type); > + free(fs_label); > + 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. > + */ That is not true. libblkid definitely can scan for partition or any other signature for that matter (lvm, mdraid, ...) and we should definitely utilize that. It is true that "our" blkid library can not do that, but we should actually stop using that and possibly remove it from e2fsprogs since it's really out-of-date comparing to upstream libblkid from util-linux. I have some patches to do that, I can revive and resent it. Thanks! -Lukas > + > #ifdef HAVE_LINUX_MAJOR_H > #ifndef MAJOR > #define MAJOR(dev) ((dev)>>8) > diff --git a/misc/util.h b/misc/util.h > index 9de3fbf..745568e 100644 > --- a/misc/util.h > +++ b/misc/util.h > @@ -20,6 +20,7 @@ extern char *journal_location_string; > */ > #define CHECK_BLOCK_DEV 0x0001 > #define CREATE_FILE 0x0002 > +#define CHECK_FS_EXIST 0x0004 > > #ifndef HAVE_STRCASECMP > extern int strcasecmp (char *s1, char *s2); > -- 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
On Wed, 30 Apr 2014, Lukáš Czerner wrote: > Date: Wed, 30 Apr 2014 13:50:13 +0200 (CEST) > From: Lukáš Czerner <lczerner@redhat.com> > To: Theodore Ts'o <tytso@mit.edu> > Cc: Ext4 Developers List <linux-ext4@vger.kernel.org> > Subject: Re: [PATCH 6/7] mke2fs: check for pre-existing file system > > On Sat, 26 Apr 2014, Theodore Ts'o wrote: > > > Date: Sat, 26 Apr 2014 20:00:33 -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 6/7] mke2fs: check for pre-existing file system > > > > Warn the system administrator if there is an existing file system on > > the block device, and give the administrator an opportunity to abort > > the mkfs operation. > > > > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> > > --- > > misc/mke2fs.c | 4 +++- > > misc/util.c | 29 +++++++++++++++++++++++++++++ > > misc/util.h | 1 + > > 3 files changed, 33 insertions(+), 1 deletion(-) > > > > diff --git a/misc/mke2fs.c b/misc/mke2fs.c > > index 799132a..3694ce5 100644 > > --- a/misc/mke2fs.c > > +++ b/misc/mke2fs.c > > @@ -1753,7 +1753,9 @@ profile_error: > > profile_get_integer(profile, "options", "proceed_delay", 0, 5, > > &proceed_delay); > > > > - if (!check_plausibility(device_name, CREATE_FILE, > > + /* The isatty() test is so we don't break existing scripts */ > > + if (!check_plausibility(device_name, CREATE_FILE | > > + (isatty(0) ? CHECK_FS_EXIST : 0), > > &is_device) && !force) > > proceed_question(proceed_delay); > > > > diff --git a/misc/util.c b/misc/util.c > > index afb0058..be16ebe 100644 > > --- a/misc/util.c > > +++ b/misc/util.c > > @@ -113,6 +113,9 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev) > > int fd, is_dev = 0; > > ext2fs_struct_stat s; > > int fl = O_RDONLY; > > + blkid_cache cache = NULL; > > + char *fs_type = NULL; > > + char *fs_label = NULL; > > > > if (flags & CREATE_FILE) > > fl |= O_CREAT; > > @@ -148,6 +151,32 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev) > > return 0; > > } > > > > + if ((flags & CHECK_FS_EXIST) && blkid_get_cache(&cache, NULL) >= 0) { > > + fs_type = blkid_get_tag_value(cache, "TYPE", device); > > + if (fs_type) > > + fs_label = blkid_get_tag_value(cache, "LABEL", device); > > + blkid_put_cache(cache); > > + } > > + > > + if (fs_type) { > > + if (fs_label) > > + printf(_("%s contains a %s file system " > > + "labelled '%s'\n"), device, fs_type, fs_label); > > + else > > + printf(_("%s contains a %s file system\n"), device, > > + fs_type); > > + free(fs_type); > > + free(fs_label); > > + 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. > > + */ > > That is not true. libblkid definitely can scan for partition or any > other signature for that matter (lvm, mdraid, ...) and we should > definitely utilize that. Take a look at how xfs is doing this: pr = blkid_new_probe_from_filename(device); if (!pr) goto out; ret = blkid_probe_enable_partitions(pr, 1); if (ret < 0) goto out; ret = blkid_do_fullprobe(pr); if (ret < 0) goto out; /* * Blkid returns 1 for nothing found and 0 when it finds a signature, * but we want the exact opposite, so reverse the return value here. * * In addition print some useful diagnostics about what actually is * on the device. */ if (ret) { ret = 0; goto out; } if (!blkid_probe_lookup_value(pr, "TYPE", &type, NULL)) { fprintf(stderr, _("%s: %s appears to contain an existing " "filesystem (%s).\n"), progname, device, type); } else if (!blkid_probe_lookup_value(pr, "PTTYPE", &type, NULL)) { fprintf(stderr, _("%s: %s appears to contain a partition " "table (%s).\n"), progname, device, type); } else { fprintf(stderr, _("%s: %s appears to contain something weird " "according to blkid\n"), progname, device); } ret = 1; we should probably also add blkid_probe_enable_superblocks(pr, 1) even though it is enabled by default but just to better readability. I think that we probably want to do something similar to this. Thanks! -Lukas > > It is true that "our" blkid library can not do that, but we should > actually stop using that and possibly remove it from e2fsprogs since > it's really out-of-date comparing to upstream libblkid from > util-linux. > > I have some patches to do that, I can revive and resent it. > > Thanks! > -Lukas > > > + > > #ifdef HAVE_LINUX_MAJOR_H > > #ifndef MAJOR > > #define MAJOR(dev) ((dev)>>8) > > diff --git a/misc/util.h b/misc/util.h > > index 9de3fbf..745568e 100644 > > --- a/misc/util.h > > +++ b/misc/util.h > > @@ -20,6 +20,7 @@ extern char *journal_location_string; > > */ > > #define CHECK_BLOCK_DEV 0x0001 > > #define CREATE_FILE 0x0002 > > +#define CHECK_FS_EXIST 0x0004 > > > > #ifndef HAVE_STRCASECMP > > extern int strcasecmp (char *s1, char *s2); > > > -- > 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 >
On Wed, Apr 30, 2014 at 01:50:13PM +0200, Lukáš Czerner wrote: > > + /* > > + * 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. > > + */ > > That is not true. libblkid definitely can scan for partition or any > other signature for that matter (lvm, mdraid, ...) and we should > definitely utilize that. libblkid scan scan for LVM and mdraid, sure. But it doesn't scan for GPT or MBR partition labels: % dpkg -S /sbin/blkid util-linux: /sbin/blkid % fdisk -l /dev/sda Disk /dev/sda: 1000.2 GB, 1000204886016 bytes 255 heads, 63 sectors/track, 121601 cylinders, total 1953525168 sectors Units = sectors of 1 * 512 = 512 bytes Sector size (logical/physical): 512 bytes / 512 bytes I/O size (minimum/optimal): 512 bytes / 512 bytes Disk identifier: 0xae7b6b2b Device Boot Start End Blocks Id System /dev/sda1 * 2048 206847 102400 7 HPFS/NTFS/exFAT /dev/sda2 206848 167772159 83782656 7 HPFS/NTFS/exFAT /dev/sda3 167772160 482344959 157286400 83 Linux /dev/sda4 482344960 1953525167 735590104 8e Linux LVM % sudo /sbin/blkid /dev/sda % Ohhh... this may be debian's fault for having a pre-historic util-linux. But anyway, if util-linux does detect partition labels, then this patch will catch the problem. - 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
diff --git a/misc/mke2fs.c b/misc/mke2fs.c index 799132a..3694ce5 100644 --- a/misc/mke2fs.c +++ b/misc/mke2fs.c @@ -1753,7 +1753,9 @@ profile_error: profile_get_integer(profile, "options", "proceed_delay", 0, 5, &proceed_delay); - if (!check_plausibility(device_name, CREATE_FILE, + /* The isatty() test is so we don't break existing scripts */ + if (!check_plausibility(device_name, CREATE_FILE | + (isatty(0) ? CHECK_FS_EXIST : 0), &is_device) && !force) proceed_question(proceed_delay); diff --git a/misc/util.c b/misc/util.c index afb0058..be16ebe 100644 --- a/misc/util.c +++ b/misc/util.c @@ -113,6 +113,9 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev) int fd, is_dev = 0; ext2fs_struct_stat s; int fl = O_RDONLY; + blkid_cache cache = NULL; + char *fs_type = NULL; + char *fs_label = NULL; if (flags & CREATE_FILE) fl |= O_CREAT; @@ -148,6 +151,32 @@ int check_plausibility(const char *device, int flags, int *ret_is_dev) return 0; } + if ((flags & CHECK_FS_EXIST) && blkid_get_cache(&cache, NULL) >= 0) { + fs_type = blkid_get_tag_value(cache, "TYPE", device); + if (fs_type) + fs_label = blkid_get_tag_value(cache, "LABEL", device); + blkid_put_cache(cache); + } + + if (fs_type) { + if (fs_label) + printf(_("%s contains a %s file system " + "labelled '%s'\n"), device, fs_type, fs_label); + else + printf(_("%s contains a %s file system\n"), device, + fs_type); + free(fs_type); + free(fs_label); + 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. + */ + #ifdef HAVE_LINUX_MAJOR_H #ifndef MAJOR #define MAJOR(dev) ((dev)>>8) diff --git a/misc/util.h b/misc/util.h index 9de3fbf..745568e 100644 --- a/misc/util.h +++ b/misc/util.h @@ -20,6 +20,7 @@ extern char *journal_location_string; */ #define CHECK_BLOCK_DEV 0x0001 #define CREATE_FILE 0x0002 +#define CHECK_FS_EXIST 0x0004 #ifndef HAVE_STRCASECMP extern int strcasecmp (char *s1, char *s2);
Warn the system administrator if there is an existing file system on the block device, and give the administrator an opportunity to abort the mkfs operation. Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> --- misc/mke2fs.c | 4 +++- misc/util.c | 29 +++++++++++++++++++++++++++++ misc/util.h | 1 + 3 files changed, 33 insertions(+), 1 deletion(-)