[U-Boot] [PATCH 4/4] UBI/UBIFS: Prevent UBI partition change while UBIFS is mounted

[U-Boot] [PATCH 4/4] UBI/UBIFS: Prevent UBI partition change while UBIFS is mounted

[Stefan Roese]

Only allow (re-)assignment to an UBI partition/device when UBIFS is
currently not mounted. Otherwise the following UBIFS commands will
crash.

Signed-off-by: Stefan Roese <sr@denx.de>
---
 common/cmd_ubi.c   |   13 +++++++++++++
 common/cmd_ubifs.c |    5 +++++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c
index 7692ac7..1e73f48 100644
--- a/common/cmd_ubi.c
+++ b/common/cmd_ubi.c
@@ -42,6 +42,8 @@ struct selected_dev {
 
 static struct selected_dev ubi_dev;
 
+int ubifs_is_mounted(void);
+
 static void ubi_dump_vol_info(const struct ubi_volume *vol)
 {
 	ubi_msg("volume information dump:");
@@ -472,6 +474,17 @@ static int do_ubi(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 		if (argc < 3)
 			return cmd_usage(cmdtp);
 
+		/*
+		 * Only allow (re-)assignment to an UBI partition/device
+		 * when UBIFS is currently not mounted. Otherwise
+		 * the following UBIFS commands will crash.
+		 */
+		if (ubifs_is_mounted()) {
+			printf("UBIFS is currently mounted!"
+			       " Unmount using ubifsumount first!\n");
+			return -EPERM;
+		}
+
 		/* todo: get dev number for NAND... */
 		ubi_dev.nr = 0;
 
diff --git a/common/cmd_ubifs.c b/common/cmd_ubifs.c
index 9526780..76fe057 100644
--- a/common/cmd_ubifs.c
+++ b/common/cmd_ubifs.c
@@ -92,6 +92,11 @@ int do_ubifs_umount(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return 0;
 }
 
+int ubifs_is_mounted(void)
+{
+	return ubifs_mounted;
+}
+
 int do_ubifs_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	char *filename = "/";
-- 
1.7.3.2

[U-Boot] [PATCH 4/4] UBI/UBIFS: Prevent UBI partition change while UBIFS is mounted

[Ben Gardiner]

On Thu, Oct 28, 2010 at 8:09 AM, Stefan Roese <sr@denx.de> wrote:
> Only allow (re-)assignment to an UBI partition/device when UBIFS is
> currently not mounted. Otherwise the following UBIFS commands will
> crash.
>
> Signed-off-by: Stefan Roese <sr@denx.de>

Applies to 908614f20f7f0f5df736eed21b88e81ebbf14e86 of
git://git.denx.de/u-boot.git.

One checkpatch warning:

WARNING: externs should be avoided in .c files
#89: FILE: common/cmd_ubi.c:45:
+int ubifs_is_mounted(void);

total: 0 errors, 1 warnings, 36 lines checked

Tested on da850evm with NAND, env.oob enabled. 'ubi part' failed while
a ubifs was mounted.

Tested-by: Ben Gardiner <bengardiner@nanometrics.ca>

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

[U-Boot] [PATCH 4/4] UBI/UBIFS: Prevent UBI partition change while UBIFS is mounted

[Wolfgang Denk]

Dear Stefan Roese,

In message <1288267776-1148-1-git-send-email-sr@denx.de> you wrote:
> Only allow (re-)assignment to an UBI partition/device when UBIFS is
> currently not mounted. Otherwise the following UBIFS commands will
> crash.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> ---
>  common/cmd_ubi.c   |   13 +++++++++++++
>  common/cmd_ubifs.c |    5 +++++
>  2 files changed, 18 insertions(+), 0 deletions(-)

I'm a bit biased here - from standard Unix command usage it seems
natural that you have to manually umount first, but then we have very
smple device handling in U-Boot, with always only one device in
access.  Would it not make sense to auto-unmount in case the user
switches the device?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In the beginning, there was nothing, which exploded.
                                - Terry Pratchett, _Lords and Ladies_

[U-Boot] [PATCH 4/4] UBI/UBIFS: Prevent UBI partition change while UBIFS is mounted

[Stefan Roese]

Hi Wolfgang,

On Friday 29 October 2010 23:06:12 Wolfgang Denk wrote:
> > Only allow (re-)assignment to an UBI partition/device when UBIFS is
> > currently not mounted. Otherwise the following UBIFS commands will
> > crash.
> > 
> > Signed-off-by: Stefan Roese <sr@denx.de>
> > ---
> > 
> >  common/cmd_ubi.c   |   13 +++++++++++++
> >  common/cmd_ubifs.c |    5 +++++
> >  2 files changed, 18 insertions(+), 0 deletions(-)
> 
> I'm a bit biased here - from standard Unix command usage it seems
> natural that you have to manually umount first, but then we have very
> smple device handling in U-Boot, with always only one device in
> access.  Would it not make sense to auto-unmount in case the user
> switches the device?

I can implement it this way if preferred. I'll prepare a new for this later.

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

[U-Boot] [PATCH 4/4] UBI/UBIFS: Prevent UBI partition change while UBIFS is mounted

[Wolfgang Denk]

Dear Stefan Roese,

In message <201011011420.35851.sr@denx.de> you wrote:
>
> > > Only allow (re-)assignment to an UBI partition/device when UBIFS is
> > > currently not mounted. Otherwise the following UBIFS commands will
> > > crash.
> > > 
> > > Signed-off-by: Stefan Roese <sr@denx.de>
> > > ---
> > > 
> > >  common/cmd_ubi.c   |   13 +++++++++++++
> > >  common/cmd_ubifs.c |    5 +++++
> > >  2 files changed, 18 insertions(+), 0 deletions(-)
> > 
> > I'm a bit biased here - from standard Unix command usage it seems
> > natural that you have to manually umount first, but then we have very
> > smple device handling in U-Boot, with always only one device in
> > access.  Would it not make sense to auto-unmount in case the user
> > switches the device?
> 
> I can implement it this way if preferred. I'll prepare a new for this later.

As mentioned - I am not sure what would be best here.

What is your own position?

And: anybody else to comment?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You might not be as stupid as you look. This is not hard. Let's think
about this. I mean ... I'll think about this, and  you  can  join  in
when you know the words.             - Terry Pratchett, _Men at Arms_

[U-Boot] [PATCH 4/4] UBI/UBIFS: Prevent UBI partition change while UBIFS is mounted

[Gray Remlin]

On 01/11/10 19:06, Wolfgang Denk wrote:
> Dear Stefan Roese,
> 
> In message <201011011420.35851.sr@denx.de> you wrote:
>>
>>>> Only allow (re-)assignment to an UBI partition/device when UBIFS is
>>>> currently not mounted. Otherwise the following UBIFS commands will
>>>> crash.
>>>>
>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>> ---
>>>>
>>>>  common/cmd_ubi.c   |   13 +++++++++++++
>>>>  common/cmd_ubifs.c |    5 +++++
>>>>  2 files changed, 18 insertions(+), 0 deletions(-)
>>>
>>> I'm a bit biased here - from standard Unix command usage it seems
>>> natural that you have to manually umount first, but then we have very
>>> smple device handling in U-Boot, with always only one device in
>>> access.  Would it not make sense to auto-unmount in case the user
>>> switches the device?
>>
>> I can implement it this way if preferred. I'll prepare a new for this later.
> 
> As mentioned - I am not sure what would be best here.
> 
> What is your own position?
> 
> And: anybody else to comment?
> 
> Best regards,
> 
> Wolfgang Denk
> 
As a 'User' what matters the most to me is command user interface
consistency
(yes I want the impossible) regardless of the filesystem\device type.

For example to load uImage, I want 'load <device>:<partition>:<file>'
whether
it be an ide device with ext3 filesystem or a mmc device with ubifs.

If a filesystem needs to be mounted for some devices\commands but not others
then it should 'just happen' when needed. As a 'User' my only interest
is the
desired end result with the minimum of brainpower input. I want all related
commands to take the same arguments in the same order. I want to be able to
use them without understanding (technically) what they do.

An acceptable alternative would be to 'select <device>:<partition>' then
have
all further commands refer to said '<device>:<partition>' without restating.
But I would want all commands to work that way.

And personally, I like terse commands, but I know others hate them....Oh
well.

As a developer I know this is unrealistic...but a 'User' what matters
the most
to me is command user interface consistency!

[U-Boot] [PATCH 4/4] UBI/UBIFS: Prevent UBI partition change while UBIFS is mounted

[Stefan Roese]

Hi Wolfgang,

On Monday 01 November 2010 20:06:31 Wolfgang Denk wrote:
> > > I'm a bit biased here - from standard Unix command usage it seems
> > > natural that you have to manually umount first, but then we have very
> > > smple device handling in U-Boot, with always only one device in
> > > access.  Would it not make sense to auto-unmount in case the user
> > > switches the device?
> > 
> > I can implement it this way if preferred. I'll prepare a new for this
> > later.
> 
> As mentioned - I am not sure what would be best here.
> 
> What is your own position?

I prefer the first approach, not automatically unmounting upon UBI device 
change. One plus for this version is that the user might have issued the 2nd 
"ubi part" by mistake and didn't really want to unmount the UBIFS filesystem 
in the first place.

But I have no strong feeling here. So I'm open to suggestions from others 
which version is the preferred one.

Thanks.

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

[U-Boot] [PATCH 4/4] UBI/UBIFS: Prevent UBI partition change while UBIFS is mounted

[Ben Gardiner]

On Tue, Nov 2, 2010 at 6:49 AM, Stefan Roese <sr@denx.de> wrote:
> Hi Wolfgang,
>
> On Monday 01 November 2010 20:06:31 Wolfgang Denk wrote:
>> > > I'm a bit biased here - from standard Unix command usage it seems
>> > > natural that you have to manually umount first, but then we have very
>> > > smple device handling in U-Boot, with always only one device in
>> > > access. ?Would it not make sense to auto-unmount in case the user
>> > > switches the device?
>> >
>> > I can implement it this way if preferred. I'll prepare a new for this
>> > later.
>>
>> As mentioned - I am not sure what would be best here.
>>
>> What is your own position?
>
> I prefer the first approach, not automatically unmounting upon UBI device
> change. One plus for this version is that the user might have issued the 2nd
> "ubi part" by mistake and didn't really want to unmount the UBIFS filesystem
> in the first place.
>
> But I have no strong feeling here. So I'm open to suggestions from others
> which version is the preferred one.

FWIW: I prefer the option where the UBIFS is automatically unmounted.
At first I was in favour of not automatically unmounting since it
emulates more closely the interface of a unix environment, to which I
am biased. However, automatically unmounting has an important
advantage: the state resulting from a 'ubi part' command is the same
regardless of the initial 'mounted state'. This make scripting boot
sequences simpler. The alternative would be to provide a 'test
mounted' command along with the option where ubi part does not
automatically unmount.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

[U-Boot] [PATCH 4/4] UBI/UBIFS: Prevent UBI partition change while UBIFS is mounted

[Wolfgang Denk]

Dear Stefan Roese,

In message <201011021149.24656.sr@denx.de> you wrote:
>
> I prefer the first approach, not automatically unmounting upon UBI device 

I don't see any advantages of this approach.

> change. One plus for this version is that the user might have issued the 2nd 
> "ubi part" by mistake and didn't really want to unmount the UBIFS filesystem 
> in the first place.

Well, if I type a command I expect it to be executed. Otherwise you
should add "Do you really want to..." dialogs everywhere (but don;t
expect that these get accepted for mainline ;-)

> But I have no strong feeling here. So I'm open to suggestions from others 
> which version is the preferred one.

Gray has spoken to the heart of mine.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
[Braddock:] Mr. Churchill, you are drunk.
[Churchill:] And you madam, are ugly.  But I shall be sober tomorrow.