[U-Boot] [PATCH 4/4] UBI/UBIFS: Prevent UBI partition change while UBIFS is mounted
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 = "/";
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
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
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@denx.de
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
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!
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@denx.de
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
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
participants (4)
-
Ben Gardiner -
Gray Remlin -
Stefan Roese -
Wolfgang Denk