Patchwork [1/2] mke2fs: fix force option to create filesystem on mounted device

login
register
mail settings
Submitter Kazuya Mio
Date Feb. 4, 2013, 8:04 a.m.
Message ID <510F6B75.6090408@sx.jp.nec.com>
Download mbox | patch
Permalink /patch/217838/
State Superseded
Headers show

Comments

Kazuya Mio - Feb. 4, 2013, 8:04 a.m.
According to mke2fs man, we can create a filesystem on the mounted device
when -FF option is specified.
However, currently we have to specify -F option third to force mke2fs.
This patch fixes the problem.

Note that force mke2fs for mounted block device fails because mke2fs opens
a device with O_EXCL flag. This work is contradictory to mke2fs man page,
but I'm not sure it should be fixed or not. What do you think?

Signed-off-by: Kazuya Mio <k-mio@sx.jp.nec.com>
---
 misc/util.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

--
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
Kazuya Mio - April 25, 2013, 11:58 p.m.
2013/02/04 17:04, Kazuya Mio wrote:
> According to mke2fs man, we can create a filesystem on the mounted device
> when -FF option is specified.
> However, currently we have to specify -F option third to force mke2fs.
> This patch fixes the problem.
> 
> Note that force mke2fs for mounted block device fails because mke2fs opens
> a device with O_EXCL flag. This work is contradictory to mke2fs man page,
> but I'm not sure it should be fixed or not. What do you think?

Ted, please merge two patches if no problem is found.

http://patchwork.ozlabs.org/patch/217838/
http://patchwork.ozlabs.org/patch/217839/

Regards,
Kazuya Mio
--
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
Andreas Dilger - April 26, 2013, 9:45 p.m.
On 2013-04-25, at 5:58 PM, Kazuya Mio wrote:
> 2013/02/04 17:04, Kazuya Mio wrote:
>> According to mke2fs man, we can create a filesystem on the mounted device when -FF option is specified.
>> However, currently we have to specify -F option third to force mke2fs. This patch fixes the problem.
>> 
>> Note that force mke2fs for mounted block device fails because mke2fs opens a device with O_EXCL flag. This work is contradictory to mke2fs man page,  
>> but I'm not sure it should be fixed or not. What do you think?
> 
> Ted, please merge two patches if no problem is found.
> 
> http://patchwork.ozlabs.org/patch/217838/

Out of curiosity, is formatting a mounted filesystem something
that you are actually trying to do, or is this just a case of
trying to make the mke2fs usage consistent with the man page?

As for O_EXCL blocking mke2fs, this was changed by my patch in
commit 732e26b98e5c.  The only reason that this used to work
was because a filesystem might _appear_ to be mounted if /etc/mtab
was stale, but wasn't actually in use by the kernel.  I don't
think there is ANY good reason to allow mke2fs to succeed if the
block device is _actually_ in use.

Since 732e26b98e5c prevents mke2fs from actually formatting an
in-use block device (at least on any modern kernel), I'm OK with
this change, but wanted to understand the reasoning behind this.


> http://patchwork.ozlabs.org/patch/217839/

I don't understand the second patch.  If the user tries to format
the /dev/loop device, then it is a "block device" and not really
a "regular file" (i.e. the user probably knows what is going on).
I don't think it is necessary to include the extra code and checking
in this case.  Do I misunderstand what your patch is doing?

Cheers, Andreas





--
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
Kazuya Mio - May 7, 2013, 8:14 a.m.
2013/04/27 6:45, Andreas Dilger wrote:
>> http://patchwork.ozlabs.org/patch/217838/
>
> Out of curiosity, is formatting a mounted filesystem something
> that you are actually trying to do, or is this just a case of
> trying to make the mke2fs usage consistent with the man page?

The purpose of this patch is the latter.

>> http://patchwork.ozlabs.org/patch/217839/
>
> I don't understand the second patch.  If the user tries to format
> the /dev/loop device, then it is a "block device" and not really
> a "regular file" (i.e. the user probably knows what is going on).
> I don't think it is necessary to include the extra code and checking
> in this case.  Do I misunderstand what your patch is doing?

/dev/loop device is a block device as you know, but an image file is
a regular file and you can format it.
I think we have to prevent mke2fs to in-use image file by mistake.

Regards,
Kazuya Mio
--
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
Kazuya Mio - May 20, 2013, 4:50 a.m.
2013/05/07 17:14, Kazuya Mio wrote:
>>> http://patchwork.ozlabs.org/patch/217839/
>>
>> I don't understand the second patch.  If the user tries to format
>> the /dev/loop device, then it is a "block device" and not really
>> a "regular file" (i.e. the user probably knows what is going on).
>> I don't think it is necessary to include the extra code and checking
>> in this case.  Do I misunderstand what your patch is doing?
>
> /dev/loop device is a block device as you know, but an image file is
> a regular file and you can format it.
> I think we have to prevent mke2fs to in-use image file by mistake.

Does that answer your question?

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

Patch

diff --git a/misc/util.c b/misc/util.c
index 6c93e1c..40c8858 100644
--- a/misc/util.c
+++ b/misc/util.c
@@ -150,7 +150,7 @@  void check_mount(const char *device, int force, const char *type)
 	}
 	if (mount_flags & EXT2_MF_MOUNTED) {
 		fprintf(stderr, _("%s is mounted; "), device);
-		if (force > 2) {
+		if (force >= 2) {
 			fputs(_("mke2fs forced anyway.  Hope /etc/mtab is "
 				"incorrect.\n"), stderr);
 			return;
@@ -162,7 +162,7 @@  void check_mount(const char *device, int force, const char *type)
 	if (mount_flags & EXT2_MF_BUSY) {
 		fprintf(stderr, _("%s is apparently in use by the system; "),
 			device);
-		if (force > 2) {
+		if (force >= 2) {
 			fputs(_("mke2fs forced anyway.\n"), stderr);
 			return;
 		}