Patchwork [[mtd-utils] 3/3] mkfs.ubifs: allow reformatting of devices

login
register
mail settings
Submitter Mike Frysinger
Date May 8, 2013, 4:27 p.m.
Message ID <1368030446-343-3-git-send-email-vapier@gentoo.org>
Download mbox | patch
Permalink /patch/242609/
State New
Headers show

Comments

Mike Frysinger - May 8, 2013, 4:27 p.m.
Sometimes I want to re-initialize an existing ubifs, but the tool
currently bails out if the volume is already formatted.  Prompt the
user instead so they can decide.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 mkfs.ubifs/mkfs.ubifs.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
Ricard Wanderlof - May 10, 2013, 7:07 a.m.
On Wed, 8 May 2013, Mike Frysinger wrote:

> Sometimes I want to re-initialize an existing ubifs, but the tool
> currently bails out if the volume is already formatted.  Prompt the
> user instead so they can decide.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> ...
> 	{"root",               1, NULL, 'r'},
> @@ -142,6 +143,7 @@ static const struct option longopts[] = {
> 	{"max-leb-cnt",        1, NULL, 'c'},
> 	{"output",             1, NULL, 'o'},
> 	{"devtable",           1, NULL, 'D'},
> +	{"yes",                0, NULL, 'y'},
> 	{"help",               0, NULL, 'h'},
> 	{"verbose",            0, NULL, 'v'},
> 	{"version",            0, NULL, 'V'},
> @@ -191,6 +193,7 @@ static const char *helptext =
> "-U, --squash-uids        squash owners making all files owned by root\n"
> "-l, --log-lebs=COUNT     count of erase blocks for the log (used only for\n"
> "                         debugging)\n"
> +"-y, --yes                assume the answer is \"yes\" for all questions\n"
> ...

Wouldn't it be better to have a specific option for this specific case, 
rather than a general yes-to-everything option? The latter makes sense 
with programs such as fsck where the only prompt basically is 'I found a 
fault, shall I fix it?', but in this is case it can be difficult to 
predict the outcome should the option start to cover more potential 
questions in the future.

-f and -F are already taken, but I was thinking something along the lines of:

"-T, --reformat		format an existing ubifs even if already formatted"

/Ricard
Mike Frysinger - May 10, 2013, 3:26 p.m.
On Friday 10 May 2013 03:07:53 Ricard Wanderlof wrote:
> On Wed, 8 May 2013, Mike Frysinger wrote:
> > Sometimes I want to re-initialize an existing ubifs, but the tool
> > currently bails out if the volume is already formatted.  Prompt the
> > user instead so they can decide.
> > 
> > 	{"max-leb-cnt",        1, NULL, 'c'},
> > 	{"output",             1, NULL, 'o'},
> > 	{"devtable",           1, NULL, 'D'},
> > +	{"yes",                0, NULL, 'y'},
> > 	{"help",               0, NULL, 'h'},
> > 	{"verbose",            0, NULL, 'v'},
> > 	{"version",            0, NULL, 'V'},
> > 
> > @@ -191,6 +193,7 @@ static const char *helptext =
> > "-U, --squash-uids        squash owners making all files owned by root\n"
> > "-l, --log-lebs=COUNT     count of erase blocks for the log (used only
> > for\n" "                         debugging)\n"
> > +"-y, --yes                assume the answer is \"yes\" for all
> > questions\n" ...
> 
> Wouldn't it be better to have a specific option for this specific case,
> rather than a general yes-to-everything option?

this is the standard that the various mtd tools (including a bunch of UBI 
ones) follow

> The latter makes sense
> with programs such as fsck where the only prompt basically is 'I found a
> fault, shall I fix it?', but in this is case it can be difficult to
> predict the outcome should the option start to cover more potential
> questions in the future.

if you want to compare to standard tools, then the check would be dropped 
entirely.  when i run `mke2fs /dev/sda1`, it doesn't prompt me.
-mike
Artem Bityutskiy - May 13, 2013, 8:14 a.m.
On Fri, 2013-05-10 at 11:26 -0400, Mike Frysinger wrote:
> On Friday 10 May 2013 03:07:53 Ricard Wanderlof wrote:
> > On Wed, 8 May 2013, Mike Frysinger wrote:
> > > Sometimes I want to re-initialize an existing ubifs, but the tool
> > > currently bails out if the volume is already formatted.  Prompt the
> > > user instead so they can decide.
> > > 
> > >     {"max-leb-cnt",        1, NULL, 'c'},
> > >     {"output",             1, NULL, 'o'},
> > >     {"devtable",           1, NULL, 'D'},
> > > +   {"yes",                0, NULL, 'y'},
> > >     {"help",               0, NULL, 'h'},
> > >     {"verbose",            0, NULL, 'v'},
> > >     {"version",            0, NULL, 'V'},
> > > 
> > > @@ -191,6 +193,7 @@ static const char *helptext =
> > > "-U, --squash-uids        squash owners making all files owned by root\n"
> > > "-l, --log-lebs=COUNT     count of erase blocks for the log (used only
> > > for\n" "                         debugging)\n"
> > > +"-y, --yes                assume the answer is \"yes\" for all
> > > questions\n" ...
> > 
> > Wouldn't it be better to have a specific option for this specific case,
> > rather than a general yes-to-everything option?
> 
> this is the standard that the various mtd tools (including a bunch of UBI 
> ones) follow
> 
> > The latter makes sense
> > with programs such as fsck where the only prompt basically is 'I found a
> > fault, shall I fix it?', but in this is case it can be difficult to
> > predict the outcome should the option start to cover more potential
> > questions in the future.
> 
> if you want to compare to standard tools, then the check would be dropped 
> entirely.  when i run `mke2fs /dev/sda1`, it doesn't prompt me.

I admit I was not careful enough with the options. Feel free to change
the tools in the more direction of being more consistent with mainstream
filesystems' tools.
Mike Frysinger - May 13, 2013, 4:10 p.m.
On Monday 13 May 2013 04:14:08 Artem Bityutskiy wrote:
> On Fri, 2013-05-10 at 11:26 -0400, Mike Frysinger wrote:
> > On Friday 10 May 2013 03:07:53 Ricard Wanderlof wrote:
> > > On Wed, 8 May 2013, Mike Frysinger wrote:
> > > > Sometimes I want to re-initialize an existing ubifs, but the tool
> > > > currently bails out if the volume is already formatted.  Prompt the
> > > > user instead so they can decide.
> > > > 
> > > >     {"max-leb-cnt",        1, NULL, 'c'},
> > > >     {"output",             1, NULL, 'o'},
> > > >     {"devtable",           1, NULL, 'D'},
> > > > 
> > > > +   {"yes",                0, NULL, 'y'},
> > > > 
> > > >     {"help",               0, NULL, 'h'},
> > > >     {"verbose",            0, NULL, 'v'},
> > > >     {"version",            0, NULL, 'V'},
> > > > 
> > > > @@ -191,6 +193,7 @@ static const char *helptext =
> > > > "-U, --squash-uids        squash owners making all files owned by
> > > > root\n" "-l, --log-lebs=COUNT     count of erase blocks for the log
> > > > (used only for\n" "                         debugging)\n"
> > > > +"-y, --yes                assume the answer is \"yes\" for all
> > > > questions\n" ...
> > > 
> > > Wouldn't it be better to have a specific option for this specific case,
> > > rather than a general yes-to-everything option?
> > 
> > this is the standard that the various mtd tools (including a bunch of UBI
> > ones) follow
> > 
> > > The latter makes sense
> > > with programs such as fsck where the only prompt basically is 'I found
> > > a fault, shall I fix it?', but in this is case it can be difficult to
> > > predict the outcome should the option start to cover more potential
> > > questions in the future.
> > 
> > if you want to compare to standard tools, then the check would be dropped
> > entirely.  when i run `mke2fs /dev/sda1`, it doesn't prompt me.
> 
> I admit I was not careful enough with the options. Feel free to change
> the tools in the more direction of being more consistent with mainstream
> filesystems' tools.

i don't mind having it prompting first as long as there is a flag to override 
it.  i'd say let's take a survey of the community, but i think we both know 
that's doomed to go nowhere :).
-mike

Patch

diff --git a/mkfs.ubifs/mkfs.ubifs.c b/mkfs.ubifs/mkfs.ubifs.c
index 2bb819e..427e37d 100644
--- a/mkfs.ubifs/mkfs.ubifs.c
+++ b/mkfs.ubifs/mkfs.ubifs.c
@@ -103,6 +103,7 @@  static libubi_t ubi;
 /* Debug levels are: 0 (none), 1 (statistics), 2 (files) ,3 (more details) */
 int debug_level;
 int verbose;
+int yes;
 
 static char *root;
 static int root_len;
@@ -133,7 +134,7 @@  static struct inum_mapping **hash_table;
 /* Inode creation sequence number */
 static unsigned long long creat_sqnum;
 
-static const char *optstring = "d:r:m:o:D:h?vVe:c:g:f:Fp:k:x:X:j:R:l:j:UQq";
+static const char *optstring = "d:r:m:o:D:yh?vVe:c:g:f:Fp:k:x:X:j:R:l:j:UQq";
 
 static const struct option longopts[] = {
 	{"root",               1, NULL, 'r'},
@@ -142,6 +143,7 @@  static const struct option longopts[] = {
 	{"max-leb-cnt",        1, NULL, 'c'},
 	{"output",             1, NULL, 'o'},
 	{"devtable",           1, NULL, 'D'},
+	{"yes",                0, NULL, 'y'},
 	{"help",               0, NULL, 'h'},
 	{"verbose",            0, NULL, 'v'},
 	{"version",            0, NULL, 'V'},
@@ -191,6 +193,7 @@  static const char *helptext =
 "-U, --squash-uids        squash owners making all files owned by root\n"
 "-l, --log-lebs=COUNT     count of erase blocks for the log (used only for\n"
 "                         debugging)\n"
+"-y, --yes                assume the answer is \"yes\" for all questions\n"
 "-v, --verbose            verbose operation\n"
 "-V, --version            display version information\n"
 "-g, --debug=LEVEL        display debug information (0 - none, 1 - statistics,\n"
@@ -539,6 +542,9 @@  static int get_options(int argc, char**argv)
 				return sys_err_msg("bad device table file '%s'",
 						   tbl_file);
 			break;
+		case 'y':
+			yes = 1;
+			break;
 		case 'h':
 		case '?':
 			printf("%s", helptext);
@@ -2098,8 +2104,10 @@  static int open_target(void)
 		if (ubi_set_property(out_fd, UBI_VOL_PROP_DIRECT_WRITE, 1))
 			return sys_err_msg("ubi_set_property failed");
 
-		if (check_volume_empty())
-			return err_msg("UBI volume is not empty");
+		if (!yes && check_volume_empty()) {
+			if (!prompt("UBI volume is not empty.  Format anyways?", false))
+				return err_msg("UBI volume is not empty");
+		}
 	} else {
 		out_fd = open(output, O_CREAT | O_RDWR | O_TRUNC,
 			      S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH);