Btrfs devel - Feb 2013 - [PATCH, RFC] btrfs-progs: require mkfs -f force option to overwrite filesystem or partition table

The core of this is shamelessly stolen from xfsprogs.

Use blkid to detect an existing filesystem or partition
table on any of the target devices.  If something is found,
require the ''-f'' option to overwrite it, hopefully avoiding
disaster due to mistyped devicenames, etc.

# mkfs.btrfs /dev/sda1

WARNING! - Btrfs v0.20-rc1-59-gd00279c-dirty IS EXPERIMENTAL
WARNING! - see http://btrfs.wiki.kernel.org before using

/dev/sda1 appears to contain an existing filesystem (xfs).
Use the -f option to force overwrite.
#

This does introduce a requirement on libblkid.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

Note: this depends on my earlier small series,
[PATCH 1/2] btrfs-progs: fix mkfs.btrfs -r option
[PATCH 2/2, RFC] btrfs-progs: overhaul mkfs.btrfs -r option

diff --git a/Makefile b/Makefile
index 4894903..bd0e8a6 100644
--- a/Makefile
+++ b/Makefile
@@ -17,7 +17,7 @@ DEPFLAGS = -Wp,-MMD,$(@D)/.$(@F).d,-MT,$@
 INSTALL = install
 prefix ?= /usr/local
 bindir = $(prefix)/bin
-LIBS=-luuid -lm
+LIBS=-luuid -lblkid -lm
 RESTORE_LIBS=-lz
 
 progs = btrfsctl mkfs.btrfs btrfs-debug-tree btrfs-show btrfs-vol btrfsck \
diff --git a/man/mkfs.btrfs.8.in b/man/mkfs.btrfs.8.in
index c9f9e4f..9188201 100644
--- a/man/mkfs.btrfs.8.in
+++ b/man/mkfs.btrfs.8.in
@@ -6,6 +6,7 @@ mkfs.btrfs \- create a btrfs filesystem
 [ \fB\-A\fP\fI alloc-start\fP ]
 [ \fB\-b\fP\fI byte-count\fP ]
 [ \fB\-d\fP\fI data-profile\fP ]
+[ \fB\-f\fP\fI ]
 [ \fB\-l\fP\fI leafsize\fP ]
 [ \fB\-L\fP\fI label\fP ]
 [ \fB\-m\fP\fI metadata profile\fP ]
@@ -38,6 +39,11 @@ mkfs.btrfs uses all the available storage for the filesystem.
 Specify how the data must be spanned across the devices specified. Valid
 values are raid0, raid1, raid10 or single.
 .TP
+\fB\-f\fR
+Force overwrite when an existing filesystem is detected on the device.
+By default, mkfs.btrfs will not write to the device if it suspects that 
+there is a filesystem or partition table on the device already.
+.TP
 \fB\-l\fR, \fB\-\-leafsize \fIsize\fR
 Specify the leaf size, the least data item in which btrfs stores data. The
 default value is the page size.
diff --git a/mkfs.c b/mkfs.c
index 129fae8..207066c 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -39,6 +39,7 @@
 #include <linux/fs.h>
 #include <ctype.h>
 #include <attr/xattr.h>
+#include <blkid/blkid.h>
 #include "ctree.h"
 #include "disk-io.h"
 #include "volumes.h"
@@ -1162,6 +1163,86 @@ static int check_leaf_or_node_size(u32 size, u32
sectorsize)
 	return 0;
 }
 
+/*
+ * Check for existing filesystem or partition table on device.
+ * Returns:
+ *	 1 for existing fs or partition
+ *	 0 for nothing found
+ *	-1 for internal error
+ */
+static int
+check_overwrite(
+	char		*device)
+{
+	const char	*type;
+	blkid_probe	pr = NULL;
+	int		ret;
+	blkid_loff_t	size;
+
+	if (!device || !*device)
+		return 0;
+
+	ret = -1; /* will reset on success of all setup calls */
+
+	pr = blkid_new_probe_from_filename(device);
+	if (!pr)
+		goto out;
+
+	size = blkid_probe_get_size(pr);
+	if (size < 0)
+		goto out;
+
+	/* nothing to overwrite on a 0-length device */
+	if (size == 0) {
+		ret = 0;
+		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 appears to contain an existing "
+			"filesystem (%s).\n", device, type);
+	} else if (!blkid_probe_lookup_value(pr, "PTTYPE", &type, NULL))
{
+		fprintf(stderr,
+			"%s appears to contain a partition "
+			"table (%s).\n", device, type);
+	} else {
+		fprintf(stderr,
+			"%s appears to contain something weird "
+			"according to blkid\n", device);
+	}
+	ret = 1;
+
+out:
+	if (pr)
+		blkid_free_probe(pr);
+	if (ret == -1)
+		fprintf(stderr,
+			"probe of %s failed, cannot detect "
+			  "existing filesystem.\n", device);
+	return ret;
+}
+
 int main(int ac, char **av)
 {
 	char *file;
@@ -1188,6 +1269,7 @@ int main(int ac, char **av)
 	int data_profile_opt = 0;
 	int metadata_profile_opt = 0;
 	int nodiscard = 0;
+	int force_overwrite = 0;
 
 	char *source_dir = NULL;
 	int source_dir_set = 0;
@@ -1198,7 +1280,7 @@ int main(int ac, char **av)
 
 	while(1) {
 		int c;
-		c = getopt_long(ac, av, "A:b:l:n:s:m:d:L:r:VMK", long_options,
+		c = getopt_long(ac, av, "A:b:fl:n:s:m:d:L:r:VMK", long_options,
 				&option_index);
 		if (c < 0)
 			break;
@@ -1206,6 +1288,9 @@ int main(int ac, char **av)
 			case ''A'':
 				alloc_start = parse_size(optarg);
 				break;
+			case ''f'':
+				force_overwrite = 1;
+				break;
 			case ''d'':
 				data_profile = parse_profile(optarg);
 				data_profile_opt = 1;
@@ -1272,6 +1357,13 @@ int main(int ac, char **av)
 	file = av[optind++];
 	ac--; /* used that arg */
 
+	if (!force_overwrite) {
+		if (check_overwrite(file)) {
+			fprintf(stderr, "Use the -f option to force overwrite.\n");
+			exit(1);
+		}
+	}
+
 	ret = check_mounted(file);
 	if (ret < 0) {
 		fprintf(stderr, "error checking %s mount status\n", file);
@@ -1375,6 +1467,13 @@ int main(int ac, char **av)
 		int old_mixed = mixed;
 
 		file = av[optind++];
+		if (!force_overwrite) {
+			if (check_overwrite(file)) {
+				fprintf(stderr, "Use the -f option to force overwrite.\n");
+				exit(1);
+			}
+		}
+
 		ret = check_mounted(file);
 		if (ret < 0) {
 			fprintf(stderr, "error checking %s mount status\n",


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

On Thu, Feb 14, 2013 at 11:30:03AM -0700, Eric Sandeen
wrote:
> The core of this is shamelessly stolen from xfsprogs.
> 
> Use blkid to detect an existing filesystem or partition
> table on any of the target devices.  If something is found,
> require the ''-f'' option to overwrite it, hopefully
avoiding
> disaster due to mistyped devicenames, etc.
So, this has always been the one thing I didn''t want to steal from XFS.
I don''t have a good reason though, and it seems like it is time to
bring
this in.

Just to get back at you though, I''ll turn on an incandescent light bulb
every time I have to use -f.

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

On 2/14/13 2:23 PM, Chris Mason wrote:
> On Thu, Feb 14, 2013 at 11:30:03AM -0700, Eric Sandeen wrote:
>> The core of this is shamelessly stolen from xfsprogs.
>>
>> Use blkid to detect an existing filesystem or partition
>> table on any of the target devices.  If something is found,
>> require the ''-f'' option to overwrite it, hopefully
avoiding
>> disaster due to mistyped devicenames, etc.
> 
> So, this has always been the one thing I didn''t want to steal from
XFS.
> I don''t have a good reason though, and it seems like it is time to
bring
> this in.
If you really don''t want to, I''m not wedded to it.  It just
came up on the
other thread, and it''s easy enough to throw it out there for
discussion.

I don''t like a million "are you really sure?!"''s
from tools, but now
and then it''s nice to have a little barrier to screwup.

So, do as you like with it, I won''t be offended if you don''t
want it.
> Just to get back at you though, I''ll turn on an incandescent light
bulb
> every time I have to use -f.
NOOOO that seals the deal.  I retract the patch.   ;)

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

On Thu, 14 Feb 2013 12:30:03 -0600, Eric Sandeen wrote:
> The core of this is shamelessly stolen from xfsprogs.
> 
> Use blkid to detect an existing filesystem or partition
> table on any of the target devices.  If something is found,
> require the ''-f'' option to overwrite it, hopefully
avoiding
> disaster due to mistyped devicenames, etc.
> 
> # mkfs.btrfs /dev/sda1
> 
> WARNING! - Btrfs v0.20-rc1-59-gd00279c-dirty IS EXPERIMENTAL
> WARNING! - see http://btrfs.wiki.kernel.org before using
> 
> /dev/sda1 appears to contain an existing filesystem (xfs).
> Use the -f option to force overwrite.
> #
> 
> This does introduce a requirement on libblkid.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
This means that it is now required to change all occurrences of
"mkfs.btrfs" to "mkfs.btrfs -f" everywhere. Can''t
we first establish a
time period of 100 years where the -f option is tolerated and ignored,
and then in 2113 we require that the users add the -f option?

(Just had to do this string replacement everywhere, and had to add -f to
xfstest''s _scratch_mkfs in common.rc as well). Sigh.

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

On 2013/02/21 0:37, Stefan Behrens wrote:
> On Thu, 14 Feb 2013 12:30:03 -0600, Eric Sandeen wrote:
>> The core of this is shamelessly stolen from xfsprogs.
>>
>> Use blkid to detect an existing filesystem or partition
>> table on any of the target devices.  If something is found,
>> require the ''-f'' option to overwrite it, hopefully
avoiding
>> disaster due to mistyped devicenames, etc.
>>
>> # mkfs.btrfs /dev/sda1
>>
>> WARNING! - Btrfs v0.20-rc1-59-gd00279c-dirty IS EXPERIMENTAL
>> WARNING! - see http://btrfs.wiki.kernel.org before using
>>
>> /dev/sda1 appears to contain an existing filesystem (xfs).
>> Use the -f option to force overwrite.
>> #
>>
>> This does introduce a requirement on libblkid.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>
> This means that it is now required to change all occurrences of
> "mkfs.btrfs" to "mkfs.btrfs -f" everywhere.
Can''t we first establish a
I also think so.
It means -f is not significant to me, I think.
(Most of my test scripts fails without -f. So I''ll always type
"mkfs.btrfs -f")

Therefore I want you to revert commit:2a2d8e1962e8b6cda7b0a7584f6d2fb95d442cb6.
   btrfs-progs: require mkfs -f force option to overwrite filesystem or
partition table

How do you think about it?

Thanks,
Tsutomu
> time period of 100 years where the -f option is tolerated and ignored,
> and then in 2113 we require that the users add the -f option?
>
> (Just had to do this string replacement everywhere, and had to add -f to
> xfstest''s _scratch_mkfs in common.rc as well). Sigh.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 2/25/13 5:39 PM, Tsutomu Itoh wrote:
> On 2013/02/21 0:37, Stefan Behrens wrote:
>> On Thu, 14 Feb 2013 12:30:03 -0600, Eric Sandeen wrote:
>>> The core of this is shamelessly stolen from xfsprogs.
>>>
>>> Use blkid to detect an existing filesystem or partition
>>> table on any of the target devices.  If something is found,
>>> require the ''-f'' option to overwrite it,
hopefully avoiding
>>> disaster due to mistyped devicenames, etc.
>>>
>>> # mkfs.btrfs /dev/sda1
>>>
>>> WARNING! - Btrfs v0.20-rc1-59-gd00279c-dirty IS EXPERIMENTAL
>>> WARNING! - see http://btrfs.wiki.kernel.org before using
>>>
>>> /dev/sda1 appears to contain an existing filesystem (xfs).
>>> Use the -f option to force overwrite.
>>> #
>>>
>>> This does introduce a requirement on libblkid.
>>>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>
>> This means that it is now required to change all occurrences of
>> "mkfs.btrfs" to "mkfs.btrfs -f" everywhere.
Can''t we first establish a
> 
> I also think so.
> It means -f is not significant to me, I think.
> (Most of my test scripts fails without -f. So I''ll always type
"mkfs.btrfs -f")
> 
> Therefore I want you to revert
commit:2a2d8e1962e8b6cda7b0a7584f6d2fb95d442cb6.
>   btrfs-progs: require mkfs -f force option to overwrite filesystem or
partition table
> 
> How do you think about it?
What if you submit a patch to look at an environment variable,
BTRFS_CLOBBERS_ALL=1 which causes it to not require -f to overwrite?
Then you can just set it once at the top of your test environment,
and not change every instance?

Otherwise, I guess I think:

WARNING! - Btrfs v0.20-rc1-212-gf6ef8b5 IS EXPERIMENTAL

and we need to expect that things might change ...

-Eric
> Thanks,
> Tsutomu
> 
>> time period of 100 years where the -f option is tolerated and ignored,
>> and then in 2113 we require that the users add the -f option?
>>
>> (Just had to do this string replacement everywhere, and had to add -f
to
>> xfstest''s _scratch_mkfs in common.rc as well). Sigh.
>>
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 2013/02/26 9:07, Eric Sandeen wrote:
> On 2/25/13 5:39 PM, Tsutomu Itoh wrote:
>> On 2013/02/21 0:37, Stefan Behrens wrote:
>>> On Thu, 14 Feb 2013 12:30:03 -0600, Eric Sandeen wrote:
>>>> The core of this is shamelessly stolen from xfsprogs.
>>>>
>>>> Use blkid to detect an existing filesystem or partition
>>>> table on any of the target devices.  If something is found,
>>>> require the ''-f'' option to overwrite it,
hopefully avoiding
>>>> disaster due to mistyped devicenames, etc.
>>>>
>>>> # mkfs.btrfs /dev/sda1
>>>>
>>>> WARNING! - Btrfs v0.20-rc1-59-gd00279c-dirty IS EXPERIMENTAL
>>>> WARNING! - see http://btrfs.wiki.kernel.org before using
>>>>
>>>> /dev/sda1 appears to contain an existing filesystem (xfs).
>>>> Use the -f option to force overwrite.
>>>> #
>>>>
>>>> This does introduce a requirement on libblkid.
>>>>
>>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>>
>>> This means that it is now required to change all occurrences of
>>> "mkfs.btrfs" to "mkfs.btrfs -f" everywhere.
Can''t we first establish a
>>
>> I also think so.
>> It means -f is not significant to me, I think.
>> (Most of my test scripts fails without -f. So I''ll always type
"mkfs.btrfs -f")
>>
>> Therefore I want you to revert
commit:2a2d8e1962e8b6cda7b0a7584f6d2fb95d442cb6.
>>    btrfs-progs: require mkfs -f force option to overwrite filesystem or
partition table
>>
>> How do you think about it?
>
> What if you submit a patch to look at an environment variable,
> BTRFS_CLOBBERS_ALL=1 which causes it to not require -f to overwrite?
> Then you can just set it once at the top of your test environment,
> and not change every instance?
Yes. But,
 >> (Most of my test scripts fails without -f. So I''ll always
type "mkfs.btrfs -f")
is one example.

Almost everyone types "mkfs.btrfs -f" (or BTRFS_CLOBBERS_ALL=1 :)
unconditionally, I think.
So, I think -f option is almost meaningless.
> Otherwise, I guess I think:
>
> WARNING! - Btrfs v0.20-rc1-212-gf6ef8b5 IS EXPERIMENTAL
>
> and we need to expect that things might change ...
EXPERIMENTAL... It''s certainly so.
However, I think that we should not add the option that it troubles
a lot of people.

Thanks,
Tsutomu
>
> -Eric
>
>> Thanks,
>> Tsutomu
>>
>>> time period of 100 years where the -f option is tolerated and
ignored,
>>> and then in 2113 we require that the users add the -f option?
>>>
>>> (Just had to do this string replacement everywhere, and had to add
-f to
>>> xfstest''s _scratch_mkfs in common.rc as well). Sigh.
>>>
>>
>

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

On 2/25/13 9:55 PM, Tsutomu Itoh wrote:
> On 2013/02/26 9:07, Eric Sandeen wrote:
>> On 2/25/13 5:39 PM, Tsutomu Itoh wrote:
>>> On 2013/02/21 0:37, Stefan Behrens wrote:
>>>> On Thu, 14 Feb 2013 12:30:03 -0600, Eric Sandeen wrote:
>>>>> The core of this is shamelessly stolen from xfsprogs.
>>>>>
>>>>> Use blkid to detect an existing filesystem or partition
>>>>> table on any of the target devices.  If something is found,
>>>>> require the ''-f'' option to overwrite it,
hopefully avoiding
>>>>> disaster due to mistyped devicenames, etc.
>>>>>
>>>>> # mkfs.btrfs /dev/sda1
>>>>>
>>>>> WARNING! - Btrfs v0.20-rc1-59-gd00279c-dirty IS
EXPERIMENTAL
>>>>> WARNING! - see http://btrfs.wiki.kernel.org before using
>>>>>
>>>>> /dev/sda1 appears to contain an existing filesystem (xfs).
>>>>> Use the -f option to force overwrite.
>>>>> #
>>>>>
>>>>> This does introduce a requirement on libblkid.
>>>>>
>>>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>>>
>>>> This means that it is now required to change all occurrences of
>>>> "mkfs.btrfs" to "mkfs.btrfs -f" everywhere.
Can''t we first establish a
>>>
>>> I also think so.
>>> It means -f is not significant to me, I think.
>>> (Most of my test scripts fails without -f. So I''ll always
type "mkfs.btrfs -f")
>>>
>>> Therefore I want you to revert
commit:2a2d8e1962e8b6cda7b0a7584f6d2fb95d442cb6.
>>>    btrfs-progs: require mkfs -f force option to overwrite
filesystem or partition table
>>>
>>> How do you think about it?
>>
>> What if you submit a patch to look at an environment variable,
>> BTRFS_CLOBBERS_ALL=1 which causes it to not require -f to overwrite?
>> Then you can just set it once at the top of your test environment,
>> and not change every instance?
> 
> Yes. But,
>>> (Most of my test scripts fails without -f. So I''ll always
type "mkfs.btrfs -f")
> is one example.
> 
> Almost everyone types "mkfs.btrfs -f" (or BTRFS_CLOBBERS_ALL=1 :)
> unconditionally, I think.
> So, I think -f option is almost meaningless.
> 
>> Otherwise, I guess I think:
>>
>> WARNING! - Btrfs v0.20-rc1-212-gf6ef8b5 IS EXPERIMENTAL
>>
>> and we need to expect that things might change ...
> 
> EXPERIMENTAL... It''s certainly so.
> However, I think that we should not add the option that it troubles
> a lot of people.
Well, I sent it as an RFC.  Chris merged it; I''ll defer to his
judgement.

Thanks,
-Eric
> Thanks,
> Tsutomu
> 
>>
>> -Eric
>>
>>> Thanks,
>>> Tsutomu
>>>
>>>> time period of 100 years where the -f option is tolerated and
ignored,
>>>> and then in 2113 we require that the users add the -f option?
>>>>
>>>> (Just had to do this string replacement everywhere, and had to
add -f to
>>>> xfstest''s _scratch_mkfs in common.rc as well). Sigh.
>>>>
>>>
>>
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 2013/02/26 13:06, Eric Sandeen wrote:
> On 2/25/13 9:55 PM, Tsutomu Itoh wrote:
>> On 2013/02/26 9:07, Eric Sandeen wrote:
>>> On 2/25/13 5:39 PM, Tsutomu Itoh wrote:
>>>> On 2013/02/21 0:37, Stefan Behrens wrote:
>>>>> On Thu, 14 Feb 2013 12:30:03 -0600, Eric Sandeen wrote:
>>>>>> The core of this is shamelessly stolen from xfsprogs.
>>>>>>
>>>>>> Use blkid to detect an existing filesystem or partition
>>>>>> table on any of the target devices.  If something is
found,
>>>>>> require the ''-f'' option to overwrite
it, hopefully avoiding
>>>>>> disaster due to mistyped devicenames, etc.
>>>>>>
>>>>>> # mkfs.btrfs /dev/sda1
>>>>>>
>>>>>> WARNING! - Btrfs v0.20-rc1-59-gd00279c-dirty IS
EXPERIMENTAL
>>>>>> WARNING! - see http://btrfs.wiki.kernel.org before
using
>>>>>>
>>>>>> /dev/sda1 appears to contain an existing filesystem
(xfs).
>>>>>> Use the -f option to force overwrite.
>>>>>> #
>>>>>>
>>>>>> This does introduce a requirement on libblkid.
>>>>>>
>>>>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>>>>
>>>>> This means that it is now required to change all
occurrences of
>>>>> "mkfs.btrfs" to "mkfs.btrfs -f"
everywhere. Can''t we first establish a
>>>>
>>>> I also think so.
>>>> It means -f is not significant to me, I think.
>>>> (Most of my test scripts fails without -f. So I''ll
always type "mkfs.btrfs -f")
>>>>
>>>> Therefore I want you to revert
commit:2a2d8e1962e8b6cda7b0a7584f6d2fb95d442cb6.
>>>>     btrfs-progs: require mkfs -f force option to overwrite
filesystem or partition table
>>>>
>>>> How do you think about it?
>>>
>>> What if you submit a patch to look at an environment variable,
>>> BTRFS_CLOBBERS_ALL=1 which causes it to not require -f to
overwrite?
>>> Then you can just set it once at the top of your test environment,
>>> and not change every instance?
>>
>> Yes. But,
>>>> (Most of my test scripts fails without -f. So I''ll
always type "mkfs.btrfs -f")
>> is one example.
>>
>> Almost everyone types "mkfs.btrfs -f" (or
BTRFS_CLOBBERS_ALL=1 :)
>> unconditionally, I think.
>> So, I think -f option is almost meaningless.
>>
>>> Otherwise, I guess I think:
>>>
>>> WARNING! - Btrfs v0.20-rc1-212-gf6ef8b5 IS EXPERIMENTAL
>>>
>>> and we need to expect that things might change ...
>>
>> EXPERIMENTAL... It''s certainly so.
>> However, I think that we should not add the option that it troubles
>> a lot of people.
>
> Well, I sent it as an RFC.  Chris merged it; I''ll defer to his
judgement.
Agreed. So, I sent revert request to Chris :)

Thanks,
Tsutomu
>
> Thanks,
> -Eric
>
>> Thanks,
>> Tsutomu
>>
>>>
>>> -Eric
>>>
>>>> Thanks,
>>>> Tsutomu
>>>>
>>>>> time period of 100 years where the -f option is tolerated
and ignored,
>>>>> and then in 2113 we require that the users add the -f
option?
>>>>>
>>>>> (Just had to do this string replacement everywhere, and had
to add -f to
>>>>> xfstest''s _scratch_mkfs in common.rc as well).
Sigh.
>>>>>

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

On Tue, Feb 26, 2013 at 01:25:11PM +0900, Tsutomu Itoh
wrote:
> On 2013/02/26 13:06, Eric Sandeen wrote:
> >On 2/25/13 9:55 PM, Tsutomu Itoh wrote:
> >>EXPERIMENTAL... It''s certainly so.
> >>However, I think that we should not add the option that it troubles
> >>a lot of people.
> >
> >Well, I sent it as an RFC.  Chris merged it; I''ll defer to his
judgement.
> 
> Agreed. So, I sent revert request to Chris :)
Where? I want to NACK the revert - this is not a matter to joke
about.

You''re all smart enough to know how to use shell aliases and script
variables, so this "need to type -f all the time" argument holds
absolutely no weight at all. Further, most of the time you''re
working on systems that don''t hold any data you care about and so
the consequences of a mistake are very minor.

However, users often make mistakes and we have to take that into
account when deciding on the default behaviour of our tools.
Tools that destroy data *must* err on the side of conservative
default behaviour simply because of the fact that destroying the
wrong data can have catastrophic consequences. It''s not your data
that is being destroyed, but it is data that you have a
*responsibility to safeguard* as a filesystem developer.

Think about it this way: how happy would an important customer be if
they decided that *you* were directly responsible for a major data
loss incident because they found it would have been prevented by the
"-f" patch? I don''t think that the explanation of "it
was
inconvenient to me" would be an acceptable answer.....

Cheers,

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

On 2013/02/26 16:05, Dave Chinner wrote:
> On Tue, Feb 26, 2013 at 01:25:11PM +0900, Tsutomu Itoh wrote:
>> On 2013/02/26 13:06, Eric Sandeen wrote:
>>> On 2/25/13 9:55 PM, Tsutomu Itoh wrote:
>>>> EXPERIMENTAL... It''s certainly so.
>>>> However, I think that we should not add the option that it
troubles
>>>> a lot of people.
>>>
>>> Well, I sent it as an RFC.  Chris merged it; I''ll defer to
his judgement.
>>
>> Agreed. So, I sent revert request to Chris :)
>
> Where? I want to NACK the revert - this is not a matter to joke
> about.
I apologize for my childish expression.
I''ll also defer to Chris''s judgement.

Thanks,
Tsutomu
>
> You''re all smart enough to know how to use shell aliases and
script
> variables, so this "need to type -f all the time" argument holds
> absolutely no weight at all. Further, most of the time you''re
> working on systems that donMy childish expression is mistaken. ''t
hold any data you care about and so
> the consequences of a mistake are very minor.
>
> However, users often make mistakes and we have to take that into
> account when deciding on the default behaviour of our tools.
> Tools that destroy data *must* err on the side of conservative
> default behaviour simply because of the fact that destroying the
> wrong data can have catastrophic consequences. It''s not your data
> that is being destroyed, but it is data that you have a
> *responsibility to safeguard* as a filesystem developer.
>
> Think about it this way: how happy would an important customer be if
> they decided that *you* were directly responsible for a major data
> loss incident because they found it would have been prevented by the
> "-f" patch? I don''t think that the explanation of
"it was
> inconvenient to me" would be an acceptable answer.....
>
> Cheers,
>
> Dave.
>

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

Am Dienstag, 26. Februar 2013 schrieb Tsutomu Itoh:
> >> Therefore I want you to revert
> >> commit:2a2d8e1962e8b6cda7b0a7584f6d2fb95d442cb6.
> >>
> >>    btrfs-progs: require mkfs -f force option to overwrite
filesystem
> >>or partition table
> >>
> >> How do you think about it?
> > 
> > What if you submit a patch to look at an environment variable,
> > BTRFS_CLOBBERS_ALL=1 which causes it to not require -f to overwrite?
> > Then you can just set it once at the top of your test environment,
> > and not change every instance?
> 
> Yes. But,
>  >> (Most of my test scripts fails without -f. So I''ll
always type
> "mkfs.btrfs -f") is one example.
> 
> Almost everyone types "mkfs.btrfs -f" (or BTRFS_CLOBBERS_ALL=1 :)
> unconditionally, I think.
> So, I think -f option is almost meaningless.
No.

I don´t.

And I teach not to in my trainings as well.

Everyone who uses rm -rf by default even just for deleting a single file does 
it as long as he or she deleted his / her home directory or something.

Ciao,
-- 
Martin ''Helios'' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, Feb 26, 2013 at 08:39:52AM +0900, Tsutomu Itoh
wrote:
> >This means that it is now required to change all occurrences of
> >"mkfs.btrfs" to "mkfs.btrfs -f" everywhere.
Can''t we first establish a
> 
> I also think so.
> It means -f is not significant to me, I think.
> (Most of my test scripts fails without -f. So I''ll always type
"mkfs.btrfs -f")
> 
> Therefore I want you to revert
commit:2a2d8e1962e8b6cda7b0a7584f6d2fb95d442cb6.
>   btrfs-progs: require mkfs -f force option to overwrite filesystem or
partition table
I personally don''t want to see it reverted, but also very much
understand the
pain to tweak all existing test scripts that rely on direct success of plain
mkfs. To resolve this I''m going to use the attached script, that
intsalls
itself in place of mkfs.btrfs and act''s as we were used to. This is
intended to
help developers and is not for the end user.

david

first use: mkfs.btrfs.wrapper install

---
$ cat mkfs.btrfs.wrapper
#!/bin/sh

if [ $# = 1 -a $1 = ''install'' ]; then
	echo "Install mode"
	mkfs=`type -p mkfs.btrfs`
	if [ $? != 0 ]; then
		echo "Cannot find mkfs.btrfs"
		exit 1
	fi
	if file $mkfs | grep -q ''ELF .* executable''; then
		echo "Moving original mkfs to ${mkfs}.real"
		mv $mkfs ${mkfs}.real
		echo "Copying myself to $mkfs"
		cp $0 $mkfs
		chmod 755 $mkfs
		echo "Have a nice day"
		exit 0
	else
		echo "$mkfs is not a binary, will not overwrite"
		exit 1
	fi
fi

mkfs=`type -p mkfs.btrfs.real`
if [ $? != 0 ]; then
	echo "Cannot find mkfs.btrfs.real, install the wrapper properly"
	exit 1
fi
force
if grep -q ''Use the -f option to force overwrite'' $mkfs; then
	force=''-f''
fi

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

On 02/26/2013 11:37 AM, Martin Steigerwald wrote:
> Am Dienstag, 26. Februar 2013 schrieb Tsutomu Itoh:
>>>> Therefore I want you to revert
>>>> commit:2a2d8e1962e8b6cda7b0a7584f6d2fb95d442cb6.
>>>>
>>>>    btrfs-progs: require mkfs -f force option to overwrite
filesystem
>>>> or partition table
>>>>
>>>> How do you think about it?
>>>
>>> What if you submit a patch to look at an environment variable,
>>> BTRFS_CLOBBERS_ALL=1 which causes it to not require -f to
overwrite?
>>> Then you can just set it once at the top of your test environment,
>>> and not change every instance?
>>
>> Yes. But,
>>  >> (Most of my test scripts fails without -f. So I''ll
always type
>> "mkfs.btrfs -f") is one example.
>>
>> Almost everyone types "mkfs.btrfs -f" (or
BTRFS_CLOBBERS_ALL=1 :)
>> unconditionally, I think.
>> So, I think -f option is almost meaningless.
> 
> No.
> 
> I don´t.
me too
> 
> And I teach not to in my trainings as well.
> 
> Everyone who uses rm -rf by default even just for deleting a single file
does
> it as long as he or she deleted his / her home directory or something.
Unfortunately the "rm -rf" is a different case. Removing a directory
is
a common case. We should not be forced to use the -f for common case. A
''-f'' flag should be used only in "uncommon" case
(like *re*format a disk
or a test-suite)...

However I think that ''-f'' is good for mkfs.btrfs.

> 
> Ciao,

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 2/20/13 9:37 AM, Stefan Behrens wrote:

...
> This means that it is now required to change all occurrences of
> "mkfs.btrfs" to "mkfs.btrfs -f" everywhere.
Can''t we first establish a
> time period of 100 years where the -f option is tolerated and ignored,
> and then in 2113 we require that the users add the -f option?
> 
> (Just had to do this string replacement everywhere, and had to add -f to
> xfstest''s _scratch_mkfs in common.rc as well). Sigh.
I''ll send a patch today to make xfstests handle both variants cleanly,
I should have done that earlier, sorry.

-Eric

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

Am Dienstag, 26. Februar 2013 schrieb Goffredo
Baroncelli:
> > And I teach not to in my trainings as well.
> >
> > 
> >
> > Everyone who uses rm -rf by default even just for deleting a single
> > file does  it as long as he or she deleted his / her home directory or
> > something.
> 
> Unfortunately the "rm -rf" is a different case. Removing a
directory is
> a common case. We should not be forced to use the -f for common case. A
> ''-f'' flag should be used only in "uncommon"
case (like *re*format a disk
> or a test-suite)...
> 
> However I think that ''-f'' is good for mkfs.btrfs.
I don´t get you on this one:

artin@merkaba:~/Zeit> export LANG=C
martin@merkaba:~/Zeit> mkdir test
martin@merkaba:~/Zeit> rm test
rm: cannot remove ''test'': Is a directory
martin@merkaba:~/Zeit#1> rm -f test
rm: cannot remove ''test'': Is a directory
martin@merkaba:~/Zeit#1> rm -r test
martin@merkaba:~/Zeit> mkdir test
martin@merkaba:~/Zeit> rmdir test
martin@merkaba:~/Zeit>

So you do not need -f to remove a directory, but either rm -r or rmdir (if 
its empty).

I think that special casing deleting empty directories with rmdir command 
doesn´t make much sense. The most important distinction there IMHO is to 
recurse or not to recurse. Thus I would let rm also delete empty directories 
and remove rmdir altogether or alias it to rm (without -r!). Thats how it 
was done on AmigaOS delete command. Delete would delete files and empty 
dirs, and recurse only if ALL option was given.

rm -f doesn´t work anyway if parent directory has write permission removed:

martin@merkaba:~/Zeit> mkdir -p test/test
martin@merkaba:~/Zeit> chmod a-w test
martin@merkaba:~/Zeit> cd test
martin@merkaba:~/Zeit> rm -r test
rm: descend into write-protected directory ''test''? y
rm: cannot remove ''test/test'': Permission denied
martin@merkaba:~/Zeit> rm -rf test
rm: cannot remove ''test/test'': Permission denied

       -f, --force
              ignore nonexistent files and arguments, never prompt



Only in the case the directory itself is write-protected rm -f makes the 
prompt go away:

martin@merkaba:~/Zeit> mkdir test
martin@merkaba:~/Zeit> chmod a-w test
martin@merkaba:~/Zeit> rm -r test
rm: remove write-protected directory ''test''? y
martin@merkaba:~/Zeit> mkdir -m a-w test
martin@merkaba:~/Zeit> ls -ld test
dr-xr-xr-x 1 martin martin 0 Feb 26 22:12 test
martin@merkaba:~/Zeit> rm -rf test

But on skipping write protection -f is justified I think.

But more so since this is a empty directory and the parent directory has 
write protection, rmdir will remove it without -f (which it doesn´t support 
anyway):

martin@merkaba:~/Zeit> mkdir -m a-w test
martin@merkaba:~/Zeit> rmdir test
martin@merkaba:~/Zeit> 

Thanks,
-- 
Martin ''Helios'' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi,

writing from a user perspective :

The "-f" option would be highly appreciated. I agree with Dave Chinner
and
I guess most people who operate maschines do as well.

Cheers,
Markus

On Tue, 26 Feb 2013, Dave Chinner wrote:
> On Tue, Feb 26, 2013 at 01:25:11PM +0900, Tsutomu Itoh wrote:
>> On 2013/02/26 13:06, Eric Sandeen wrote:
>>> On 2/25/13 9:55 PM, Tsutomu Itoh wrote:
>>>> EXPERIMENTAL... It''s certainly so.
>>>> However, I think that we should not add the option that it
troubles
>>>> a lot of people.
>>>
>>> Well, I sent it as an RFC.  Chris merged it; I''ll defer to
his judgement.
>>
>> Agreed. So, I sent revert request to Chris :)
>
> Where? I want to NACK the revert - this is not a matter to joke
> about.
>
> You''re all smart enough to know how to use shell aliases and
script
> variables, so this "need to type -f all the time" argument holds
> absolutely no weight at all. Further, most of the time you''re
> working on systems that don''t hold any data you care about and so
> the consequences of a mistake are very minor.
>
> However, users often make mistakes and we have to take that into
> account when deciding on the default behaviour of our tools.
> Tools that destroy data *must* err on the side of conservative
> default behaviour simply because of the fact that destroying the
> wrong data can have catastrophic consequences. It''s not your data
> that is being destroyed, but it is data that you have a
> *responsibility to safeguard* as a filesystem developer.
>
> Think about it this way: how happy would an important customer be if
> they decided that *you* were directly responsible for a major data
> loss incident because they found it would have been prevented by the
> "-f" patch? I don''t think that the explanation of
"it was
> inconvenient to me" would be an acceptable answer.....
>
> Cheers,
>
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe
linux-btrfs" 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-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html