mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* envfs: provide an intentional way to ignore an existing external environment
@ 2014-07-30 10:20 Juergen Borleis
  2014-07-30 10:20 ` [PATCH 1/3] " Juergen Borleis
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Juergen Borleis @ 2014-07-30 10:20 UTC (permalink / raw)
  To: barebox

Some use cases are using the barebox's built-in environment only, but still
provide an external environment store to save a modified environment (for
development purposes for example).
In this case barebox works as intended even if the external store is empty
and thus invalid. But even if it is an intentional behavior, barebox emits an
error message due to an invalid content in the external store (CRC error).

Because this error message will confuse a new user (how to know if this error
message is important or can be ignored?) and it is a bad style to ship
intentionally working systems with error messages, the following change set
adds an "empty environment" feature to barebox.

This change set adds a new option to the saveenv command which will write an
empty environment without content. But it will be marked as a placeholder and
thus should be "ignored" and barebox falls back to its built-in default
environment.

With this feature we now get:

 - if the environment store is empty, we still see an error message and
   barebox still falls back to its built-in default environment
 - if the environment store contains the new placeholder environment, there
   will be no error message but barebox falls back to its built-in default
   environment as well ("intentional behaviour")
 - if the environment store contains a regular environment (modified compared
   to the built-in one) barebox will continue to use it and ignores its
   built-in default environment instead.

Comments are welcome.

Juergen


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] envfs: provide an intentional way to ignore an existing external environment
  2014-07-30 10:20 envfs: provide an intentional way to ignore an existing external environment Juergen Borleis
@ 2014-07-30 10:20 ` Juergen Borleis
  2014-07-30 10:20 ` [PATCH 2/3] saveenv: change API to be able to forward special flags into the envfs superblock Juergen Borleis
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Juergen Borleis @ 2014-07-30 10:20 UTC (permalink / raw)
  To: barebox

Add a simple flag to envfs to be able to mark an external environment as
"not to be used".
This change should not affect existing systems, because the current envfs
implementation ensures the 'flags' member in the envfs master block is always
zeroed.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 common/environment.c | 5 +++++
 include/envfs.h      | 1 +
 2 files changed, 6 insertions(+)

diff --git a/common/environment.c b/common/environment.c
index 2d1edf8..a38e966 100644
--- a/common/environment.c
+++ b/common/environment.c
@@ -272,6 +272,11 @@ static int envfs_check_super(struct envfs_super *super, size_t *size)
 		return -EIO;
 	}
 
+	if (super->flags & ENVFS_FLAGS_PLACEHOLDER) {
+		printf("envfs: content is to be ignored\n");
+		return -EIO;
+	}
+
 	if (super->major < ENVFS_MAJOR)
 		printf("envfs version %d.%d loaded into %d.%d\n",
 			super->major, super->minor,
diff --git a/include/envfs.h b/include/envfs.h
index 9b86398..edb559f 100644
--- a/include/envfs.h
+++ b/include/envfs.h
@@ -43,6 +43,7 @@ struct envfs_super {
 	uint8_t minor;			/* minor */
 	uint16_t future;		/* reserved for future use */
 	uint32_t flags;			/* feature flags */
+#define ENVFS_FLAGS_PLACEHOLDER	(1 << 0)
 	uint32_t sb_crc;		/* crc for the superblock */
 };
 
-- 
2.0.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/3] saveenv: change API to be able to forward special flags into the envfs superblock
  2014-07-30 10:20 envfs: provide an intentional way to ignore an existing external environment Juergen Borleis
  2014-07-30 10:20 ` [PATCH 1/3] " Juergen Borleis
@ 2014-07-30 10:20 ` Juergen Borleis
  2014-07-30 10:20 ` [PATCH 3/3] saveenv: provide a zeroed/empty/ignore environment Juergen Borleis
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Juergen Borleis @ 2014-07-30 10:20 UTC (permalink / raw)
  To: barebox

In order to be able to mark an stored envfs image with special features
(intentional ignore for example), we now can feed forward these flags.
By forwarding a '0' for the flags nothing changes because the envfs superblock
was already allocated with xzalloc.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 commands/saveenv.c   |  2 +-
 common/environment.c | 24 +++++++++++++++---------
 include/envfs.h      |  2 +-
 scripts/bareboxenv.c |  2 +-
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/commands/saveenv.c b/commands/saveenv.c
index 54b6fa1..41f111d 100644
--- a/commands/saveenv.c
+++ b/commands/saveenv.c
@@ -37,7 +37,7 @@ static int do_saveenv(int argc, char *argv[])
 	else
 		filename = argv[1];
 
-	ret = envfs_save(filename, dirname);
+	ret = envfs_save(filename, dirname, 0);
 
 	return ret;
 }
diff --git a/common/environment.c b/common/environment.c
index a38e966..e135b0b 100644
--- a/common/environment.c
+++ b/common/environment.c
@@ -167,26 +167,29 @@ out:
  * Make the current environment persistent
  * @param[in] filename where to store
  * @param[in] dirname what to store (all files in this dir)
+ * @param[in] flags superblock flags (refer ENVFS_FLAGS_* macros)
  * @return 0 on success, anything else in case of failure
  *
  * Note: This function will also be used on the host! See note in the header
  * of this file.
  */
-int envfs_save(const char *filename, const char *dirname)
+int envfs_save(const char *filename, const char *dirname, unsigned flags)
 {
 	struct envfs_super *super;
-	int envfd, size, ret;
+	int envfd, size = 0, ret;
 	struct action_data data;
 	void *buf = NULL, *wbuf;
 
 	data.writep = NULL;
 	data.base = dirname;
 
-	/* first pass: calculate size */
-	recursive_action(dirname, ACTION_RECURSE, file_size_action,
-			 NULL, &data, 0);
+	if (!(flags & ENVFS_FLAGS_PLACEHOLDER)) {
+		/* first pass: calculate size */
+		recursive_action(dirname, ACTION_RECURSE, file_size_action,
+				NULL, &data, 0);
 
-	size = (unsigned long)data.writep;
+		size = (unsigned long)data.writep;
+	}
 
 	buf = xzalloc(size + sizeof(struct envfs_super));
 	data.writep = buf + sizeof(struct envfs_super);
@@ -196,10 +199,13 @@ int envfs_save(const char *filename, const char *dirname)
 	super->major = ENVFS_MAJOR;
 	super->minor = ENVFS_MINOR;
 	super->size = ENVFS_32(size);
+	super->flags = ENVFS_32(flags);
 
-	/* second pass: copy files to buffer */
-	recursive_action(dirname, ACTION_RECURSE, file_save_action,
-			 NULL, &data, 0);
+	if (!(flags & ENVFS_FLAGS_PLACEHOLDER)) {
+		/* second pass: copy files to buffer */
+		recursive_action(dirname, ACTION_RECURSE, file_save_action,
+				NULL, &data, 0);
+	}
 
 	super->crc = ENVFS_32(crc32(0, buf + sizeof(struct envfs_super), size));
 	super->sb_crc = ENVFS_32(crc32(0, buf, sizeof(struct envfs_super) - 4));
diff --git a/include/envfs.h b/include/envfs.h
index edb559f..1bbf8b0 100644
--- a/include/envfs.h
+++ b/include/envfs.h
@@ -93,7 +93,7 @@ struct envfs_super {
 
 #define ENV_FLAG_NO_OVERWRITE	(1 << 0)
 int envfs_load(const char *filename, const char *dirname, unsigned flags);
-int envfs_save(const char *filename, const char *dirname);
+int envfs_save(const char *filename, const char *dirname, unsigned flags);
 int envfs_load_from_buf(void *buf, int len, const char *dir, unsigned flags);
 
 /* defaults to /dev/env0 */
diff --git a/scripts/bareboxenv.c b/scripts/bareboxenv.c
index da420db..ec6ccfe 100644
--- a/scripts/bareboxenv.c
+++ b/scripts/bareboxenv.c
@@ -181,7 +181,7 @@ int main(int argc, char *argv[])
 		if (verbose)
 			printf("saving contents of %s to file %s\n", dirname, filename);
 
-		err = envfs_save(filename, dirname);
+		err = envfs_save(filename, dirname, 0);
 
 		if (verbose && err)
 			printf("saving env failed: %d\n", err);
-- 
2.0.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/3] saveenv: provide a zeroed/empty/ignore environment
  2014-07-30 10:20 envfs: provide an intentional way to ignore an existing external environment Juergen Borleis
  2014-07-30 10:20 ` [PATCH 1/3] " Juergen Borleis
  2014-07-30 10:20 ` [PATCH 2/3] saveenv: change API to be able to forward special flags into the envfs superblock Juergen Borleis
@ 2014-07-30 10:20 ` Juergen Borleis
  2014-07-30 15:28 ` envfs: provide an intentional way to ignore an existing external environment Jan Lübbe
  2014-07-31  7:14 ` Uwe Kleine-König
  4 siblings, 0 replies; 14+ messages in thread
From: Juergen Borleis @ 2014-07-30 10:20 UTC (permalink / raw)
  To: barebox

If an external environment storage should be used in very rare and special cases,
the intentional behaviour should be to ignore the external environment and always
fall back to the built-in environment. By storing an empty "to be ignored" environment
into the external environment a confusing error message about invalid CRC sums will go
away and still the built-in environment is used.
With this new option we can force the intentional behaviour.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 commands/saveenv.c   | 30 ++++++++++++++++++++++--------
 scripts/bareboxenv.c | 10 ++++++++--
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/commands/saveenv.c b/commands/saveenv.c
index 41f111d..84ddfa7 100644
--- a/commands/saveenv.c
+++ b/commands/saveenv.c
@@ -18,26 +18,39 @@
 #include <common.h>
 #include <command.h>
 #include <errno.h>
+#include <getopt.h>
 #include <fs.h>
 #include <fcntl.h>
 #include <envfs.h>
 
 static int do_saveenv(int argc, char *argv[])
 {
-	int ret;
+	int ret, opt;
+	unsigned envfs_flags = 0;
 	char *filename, *dirname;
 
 	printf("saving environment\n");
-	if (argc < 3)
+	while ((opt = getopt(argc, argv, "z")) > 0) {
+		switch (opt) {
+		case 'z':
+			envfs_flags |= ENVFS_FLAGS_PLACEHOLDER;
+			break;
+		}
+	}
+
+	/* destination and source are given? */
+	if (argc == optind + 2)
+		dirname = argv[optind + 1];
+	else
 		dirname = "/env";
+
+	/* destination only given? */
+	if (argc == optind + 1)
+		filename = argv[optind];
 	else
-		dirname = argv[2];
-	if (argc < 2)
 		filename = default_environment_path_get();
-	else
-		filename = argv[1];
 
-	ret = envfs_save(filename, dirname, 0);
+	ret = envfs_save(filename, dirname, envfs_flags);
 
 	return ret;
 }
@@ -49,13 +62,14 @@ BAREBOX_CMD_HELP_TEXT("ENVFS is usually a block in flash but can be any other fi
 BAREBOX_CMD_HELP_TEXT("omitted, DIRECTORY defaults to /env and ENVFS defaults to")
 BAREBOX_CMD_HELP_TEXT("/dev/env0. Note that envfs can only handle files, directories are being")
 BAREBOX_CMD_HELP_TEXT("skipped silently.")
+BAREBOX_CMD_HELP_OPT ("-z",  "store a 'zeroed' environment to force the built-in default environment at startup")
 
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(saveenv)
 	.cmd		= do_saveenv,
 	BAREBOX_CMD_DESC("save environment to persistent storage")
-	BAREBOX_CMD_OPTS("[ENVFS] [DIRECTORY]")
+	BAREBOX_CMD_OPTS("[-z] [ENVFS [DIRECTORY]]")
 	BAREBOX_CMD_GROUP(CMD_GRP_ENV)
 	BAREBOX_CMD_HELP(cmd_saveenv_help)
 BAREBOX_CMD_END
diff --git a/scripts/bareboxenv.c b/scripts/bareboxenv.c
index ec6ccfe..1936155 100644
--- a/scripts/bareboxenv.c
+++ b/scripts/bareboxenv.c
@@ -109,6 +109,7 @@ static void usage(char *prgname)
 		"\n"
 		"options:\n"
 		"  -s        save (directory -> environment sector)\n"
+		"  -z        force the built-in default environment at startup\n"
 		"  -l        load (environment sector -> directory)\n"
 		"  -p <size> pad output file to given size\n"
 		"  -v        verbose\n",
@@ -120,9 +121,10 @@ int main(int argc, char *argv[])
 	int opt;
 	int save = 0, load = 0, pad = 0, err = 0, fd;
 	char *filename = NULL, *dirname = NULL;
+	unsigned envfs_flags = 0;
 	int verbose = 0;
 
-	while((opt = getopt(argc, argv, "slp:v")) != -1) {
+	while((opt = getopt(argc, argv, "slp:vz")) != -1) {
 		switch (opt) {
 		case 's':
 			save = 1;
@@ -133,6 +135,10 @@ int main(int argc, char *argv[])
 		case 'p':
 			pad = strtoul(optarg, NULL, 0);
 			break;
+		case 'z':
+			envfs_flags |= ENVFS_FLAGS_PLACEHOLDER;
+			save = 1;
+			break;
 		case 'v':
 			verbose = 1;
 			break;
@@ -181,7 +187,7 @@ int main(int argc, char *argv[])
 		if (verbose)
 			printf("saving contents of %s to file %s\n", dirname, filename);
 
-		err = envfs_save(filename, dirname, 0);
+		err = envfs_save(filename, dirname, envfs_flags);
 
 		if (verbose && err)
 			printf("saving env failed: %d\n", err);
-- 
2.0.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: envfs: provide an intentional way to ignore an existing external environment
  2014-07-30 10:20 envfs: provide an intentional way to ignore an existing external environment Juergen Borleis
                   ` (2 preceding siblings ...)
  2014-07-30 10:20 ` [PATCH 3/3] saveenv: provide a zeroed/empty/ignore environment Juergen Borleis
@ 2014-07-30 15:28 ` Jan Lübbe
  2014-07-30 21:09   ` Sascha Hauer
  2014-07-31  7:14 ` Uwe Kleine-König
  4 siblings, 1 reply; 14+ messages in thread
From: Jan Lübbe @ 2014-07-30 15:28 UTC (permalink / raw)
  To: barebox

Hi Jürgen!

This looks good so far.

On Wed, 2014-07-30 at 12:20 +0200, Juergen Borleis wrote:
> This change set adds a new option to the saveenv command which will
> write an empty environment without content. But it will be marked as a
> placeholder and thus should be "ignored" and barebox falls back to its
> built-in default environment.

I haven't found where the environment loading is changed to implement
this behavior. Is a patch missing?

Thanks,
Jan
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: envfs: provide an intentional way to ignore an existing external environment
  2014-07-30 15:28 ` envfs: provide an intentional way to ignore an existing external environment Jan Lübbe
@ 2014-07-30 21:09   ` Sascha Hauer
  0 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2014-07-30 21:09 UTC (permalink / raw)
  To: Jan Lübbe; +Cc: barebox

On Wed, Jul 30, 2014 at 05:28:13PM +0200, Jan Lübbe wrote:
> Hi Jürgen!
> 
> This looks good so far.
> 
> On Wed, 2014-07-30 at 12:20 +0200, Juergen Borleis wrote:
> > This change set adds a new option to the saveenv command which will
> > write an empty environment without content. But it will be marked as a
> > placeholder and thus should be "ignored" and barebox falls back to its
> > built-in default environment.
> 
> I haven't found where the environment loading is changed to implement
> this behavior. Is a patch missing?

It's in the first patch. envfs_check_super is changed to return an error
when the ENVFS_FLAGS_PLACEHOLDER flag is set. Then barebox will fall
back to the default environment.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: envfs: provide an intentional way to ignore an existing external environment
  2014-07-30 10:20 envfs: provide an intentional way to ignore an existing external environment Juergen Borleis
                   ` (3 preceding siblings ...)
  2014-07-30 15:28 ` envfs: provide an intentional way to ignore an existing external environment Jan Lübbe
@ 2014-07-31  7:14 ` Uwe Kleine-König
  2014-07-31  7:33   ` Juergen Borleis
  4 siblings, 1 reply; 14+ messages in thread
From: Uwe Kleine-König @ 2014-07-31  7:14 UTC (permalink / raw)
  To: Juergen Borleis; +Cc: barebox

Hello,

On Wed, Jul 30, 2014 at 12:20:03PM +0200, Juergen Borleis wrote:
> Some use cases are using the barebox's built-in environment only, but still
> provide an external environment store to save a modified environment (for
> development purposes for example).
> In this case barebox works as intended even if the external store is empty
> and thus invalid. But even if it is an intentional behavior, barebox emits an
> error message due to an invalid content in the external store (CRC error).
> 
> Because this error message will confuse a new user (how to know if this error
> message is important or can be ignored?) and it is a bad style to ship
> intentionally working systems with error messages, the following change set
> adds an "empty environment" feature to barebox.
> 
> This change set adds a new option to the saveenv command which will write an
> empty environment without content. But it will be marked as a placeholder and
> thus should be "ignored" and barebox falls back to its built-in default
> environment.
> 
> With this feature we now get:
> 
>  - if the environment store is empty, we still see an error message and
>    barebox still falls back to its built-in default environment
>  - if the environment store contains the new placeholder environment, there
>    will be no error message but barebox falls back to its built-in default
>    environment as well ("intentional behaviour")
>  - if the environment store contains a regular environment (modified compared
>    to the built-in one) barebox will continue to use it and ignores its
>    built-in default environment instead.

Compared with storing the default environment in the external store the
only difference is that you don't need to modify it if you change the
internal one, right?

I wonder what the targeted use case is. A rescue barebox to repair a
borken bootloader and/or environment? If so I'd implemented a different
solution: To make the rescue barebox completely independant from the
state of the hardware, introduce a config symbol (say NO_AUTOLOAD_ENV)
that makes barebox only use the default environment. With this you could
still explicitly load and save the external env.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: envfs: provide an intentional way to ignore an existing external environment
  2014-07-31  7:14 ` Uwe Kleine-König
@ 2014-07-31  7:33   ` Juergen Borleis
  2014-07-31  7:44     ` Uwe Kleine-König
  0 siblings, 1 reply; 14+ messages in thread
From: Juergen Borleis @ 2014-07-31  7:33 UTC (permalink / raw)
  To: barebox; +Cc: Uwe Kleine-König

Hi Uwe,

On Thursday 31 July 2014 09:14:25 Uwe Kleine-König wrote:
> [...]
> Compared with storing the default environment in the external store the
> only difference is that you don't need to modify it if you change the
> internal one, right?

This would also be an advantage of this new feature.

> I wonder what the targeted use case is.

To use an external stored environment *only* for development purposes or tests 
and to keep the possibility to do so.
The production system should always use a well defined built-in default 
environment (but without an error message due to an empty storage which always 
looks more like "it works by accident").

> A rescue barebox to repair a borken bootloader and/or environment?

No. If the environment is broken (but valid from the checksum point of view) 
this new feature wouldn't help.

Repairing a broken environment/barebox is a different issue.

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Borleis             |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: envfs: provide an intentional way to ignore an existing external environment
  2014-07-31  7:33   ` Juergen Borleis
@ 2014-07-31  7:44     ` Uwe Kleine-König
  2014-07-31  8:03       ` Juergen Borleis
                         ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2014-07-31  7:44 UTC (permalink / raw)
  To: Juergen Borleis; +Cc: barebox

On Thu, Jul 31, 2014 at 09:33:02AM +0200, Juergen Borleis wrote:
> Hi Uwe,
> 
> On Thursday 31 July 2014 09:14:25 Uwe Kleine-König wrote:
> > [...]
> > Compared with storing the default environment in the external store the
> > only difference is that you don't need to modify it if you change the
> > internal one, right?
> 
> This would also be an advantage of this new feature.
The only one even?

> > I wonder what the targeted use case is.
> 
> To use an external stored environment *only* for development purposes or tests 
> and to keep the possibility to do so.
Doesn't make a warm and cosy feeling. Isn't it easier and more robust to
just not tell barebox about the external storage at all and for the
testing/development procedure do an explicit

	loadenv /dev/tralala

?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: envfs: provide an intentional way to ignore an existing external environment
  2014-07-31  7:44     ` Uwe Kleine-König
@ 2014-07-31  8:03       ` Juergen Borleis
  2014-08-06  3:56       ` Jean-Christophe PLAGNIOL-VILLARD
  2014-08-06  7:04       ` Michael Olbrich
  2 siblings, 0 replies; 14+ messages in thread
From: Juergen Borleis @ 2014-07-31  8:03 UTC (permalink / raw)
  To: barebox; +Cc: Uwe Kleine-König

Hi Uwe,

On Thursday 31 July 2014 09:44:16 Uwe Kleine-König wrote:
> On Thu, Jul 31, 2014 at 09:33:02AM +0200, Juergen Borleis wrote:
> > Hi Uwe,
> >
> > On Thursday 31 July 2014 09:14:25 Uwe Kleine-König wrote:
> > > [...]
> > > Compared with storing the default environment in the external store the
> > > only difference is that you don't need to modify it if you change the
> > > internal one, right?
> >
> > This would also be an advantage of this new feature.
>
> The only one even?

No. Confusing error messages are gone and we can force the intended behaviour 
as well.

> > > I wonder what the targeted use case is.
> >
> > To use an external stored environment *only* for development purposes or
> > tests and to keep the possibility to do so.
>
> Doesn't make a warm and cosy feeling. Isn't it easier and more robust to
> just not tell barebox about the external storage at all and for the
> testing/development procedure do an explicit
>
> 	loadenv /dev/tralala

Developers are lazy... I'm sure they will not love you for this suggestion ;)

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Borleis             |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: envfs: provide an intentional way to ignore an existing external environment
  2014-07-31  7:44     ` Uwe Kleine-König
  2014-07-31  8:03       ` Juergen Borleis
@ 2014-08-06  3:56       ` Jean-Christophe PLAGNIOL-VILLARD
  2014-08-06  7:04       ` Michael Olbrich
  2 siblings, 0 replies; 14+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2014-08-06  3:56 UTC (permalink / raw)
  To: Uwe Kleine-K??nig; +Cc: barebox, Juergen Borleis

On 09:44 Thu 31 Jul     , Uwe Kleine-K??nig wrote:
> 
> On Thu, Jul 31, 2014 at 09:33:02AM +0200, Juergen Borleis wrote:
> > Hi Uwe,
> > 
> > On Thursday 31 July 2014 09:14:25 Uwe Kleine-König wrote:
> > > [...]
> > > Compared with storing the default environment in the external store the
> > > only difference is that you don't need to modify it if you change the
> > > internal one, right?
> > 
> > This would also be an advantage of this new feature.
> The only one even?
> 
> > > I wonder what the targeted use case is.
> > 
> > To use an external stored environment *only* for development purposes or tests 
> > and to keep the possibility to do so.
> Doesn't make a warm and cosy feeling. Isn't it easier and more robust to
> just not tell barebox about the external storage at all and for the
> testing/development procedure do an explicit
> 
> 	loadenv /dev/tralala
+1

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: envfs: provide an intentional way to ignore an existing external environment
  2014-07-31  7:44     ` Uwe Kleine-König
  2014-07-31  8:03       ` Juergen Borleis
  2014-08-06  3:56       ` Jean-Christophe PLAGNIOL-VILLARD
@ 2014-08-06  7:04       ` Michael Olbrich
  2014-08-06  7:28         ` Uwe Kleine-König
  2 siblings, 1 reply; 14+ messages in thread
From: Michael Olbrich @ 2014-08-06  7:04 UTC (permalink / raw)
  To: barebox

On Thu, Jul 31, 2014 at 09:44:16AM +0200, Uwe Kleine-König wrote:
> On Thu, Jul 31, 2014 at 09:33:02AM +0200, Juergen Borleis wrote:
> > On Thursday 31 July 2014 09:14:25 Uwe Kleine-König wrote:
> > > [...]
> > > Compared with storing the default environment in the external store the
> > > only difference is that you don't need to modify it if you change the
> > > internal one, right?
> > 
> > This would also be an advantage of this new feature.
> The only one even?
> 
> > > I wonder what the targeted use case is.
> > 
> > To use an external stored environment *only* for development purposes or tests 
> > and to keep the possibility to do so.
> Doesn't make a warm and cosy feeling. Isn't it easier and more robust to
> just not tell barebox about the external storage at all and for the
> testing/development procedure do an explicit
> 
> 	loadenv /dev/tralala

That doesn't help at all. There are several changes that I regularly use
that are required when init runs, so a manual loadenv is too late:
- global.autoboot_timeout=3 (the build-in value is 0 to boot faster by
  default).
- nfs automounts that contain '$user'

Also, this requires me to know _where_ the environment is. And that is not
easy to remember when I need to work with multiple devices a day and gets
worse, when it changes with the boot source (SD/eMMC). Mistakes are
guaranteed.

Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: envfs: provide an intentional way to ignore an existing external environment
  2014-08-06  7:04       ` Michael Olbrich
@ 2014-08-06  7:28         ` Uwe Kleine-König
  2014-08-06 10:41           ` Michael Olbrich
  0 siblings, 1 reply; 14+ messages in thread
From: Uwe Kleine-König @ 2014-08-06  7:28 UTC (permalink / raw)
  To: barebox

Hello Michael,

On Wed, Aug 06, 2014 at 09:04:13AM +0200, Michael Olbrich wrote:
> On Thu, Jul 31, 2014 at 09:44:16AM +0200, Uwe Kleine-König wrote:
> > On Thu, Jul 31, 2014 at 09:33:02AM +0200, Juergen Borleis wrote:
> > > On Thursday 31 July 2014 09:14:25 Uwe Kleine-König wrote:
> > > > [...]
> > > > Compared with storing the default environment in the external store the
> > > > only difference is that you don't need to modify it if you change the
> > > > internal one, right?
> > > 
> > > This would also be an advantage of this new feature.
> > The only one even?
> > 
> > > > I wonder what the targeted use case is.
> > > 
> > > To use an external stored environment *only* for development purposes or tests 
> > > and to keep the possibility to do so.
> > Doesn't make a warm and cosy feeling. Isn't it easier and more robust to
> > just not tell barebox about the external storage at all and for the
> > testing/development procedure do an explicit
> > 
> > 	loadenv /dev/tralala
> 
> That doesn't help at all. There are several changes that I regularly use
> that are required when init runs, so a manual loadenv is too late:
> - global.autoboot_timeout=3 (the build-in value is 0 to boot faster by
>   default).
Ok, that might be annoying, but I'd say this doesn't justify the
Jürgen's changes.

> - nfs automounts that contain '$user'
Don't get this one. The nice thing about automount is that they are done
on request, so isn't it early enough to set global.user when loading the
debug environment (probably resulting in another line to type)?

Even though Jürgen predicted that developers won't like me for the
suggestion, I still think it's wrong to change a production system for
debug purposes.

IMHO a righter thing would be to implement an extension to microcom that
catches the 0s prompt, interrupts it and types

	loadenv /dev/tralala
	global.user=mol

for you. If I didn't miss anything that would catch all problems without
the need to change barebox.

> Also, this requires me to know _where_ the environment is. And that is not
/dev/tralala could be the same for all configurations :-) 

> easy to remember when I need to work with multiple devices a day and gets
> worse, when it changes with the boot source (SD/eMMC). Mistakes are
> guaranteed.
I'd say, either you want a) an environment that is used always, or b)
you don't.

With a) you can do your modifications to increase boot time and set the
username and save it for development. If you want to reset it to
"production mode" just do: loadenv /dev/default_env; saveenv;

With b) either live with the decision and adapt your *development*
workflow to it, or change to a).

Just my 0.02€, feel free to ignore them.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: envfs: provide an intentional way to ignore an existing external environment
  2014-08-06  7:28         ` Uwe Kleine-König
@ 2014-08-06 10:41           ` Michael Olbrich
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Olbrich @ 2014-08-06 10:41 UTC (permalink / raw)
  To: barebox

On Wed, Aug 06, 2014 at 09:28:56AM +0200, Uwe Kleine-König wrote:
> On Wed, Aug 06, 2014 at 09:04:13AM +0200, Michael Olbrich wrote:
> > On Thu, Jul 31, 2014 at 09:44:16AM +0200, Uwe Kleine-König wrote:
> > > On Thu, Jul 31, 2014 at 09:33:02AM +0200, Juergen Borleis wrote:
> > > > On Thursday 31 July 2014 09:14:25 Uwe Kleine-König wrote:
> > > > > [...]
> > > > > Compared with storing the default environment in the external store the
> > > > > only difference is that you don't need to modify it if you change the
> > > > > internal one, right?
> > > > 
> > > > This would also be an advantage of this new feature.
> > > The only one even?
> > > 
> > > > > I wonder what the targeted use case is.
> > > > 
> > > > To use an external stored environment *only* for development purposes or tests 
> > > > and to keep the possibility to do so.
> > > Doesn't make a warm and cosy feeling. Isn't it easier and more robust to
> > > just not tell barebox about the external storage at all and for the
> > > testing/development procedure do an explicit
> > > 
> > > 	loadenv /dev/tralala
> > 
> > That doesn't help at all. There are several changes that I regularly use
> > that are required when init runs, so a manual loadenv is too late:
> > - global.autoboot_timeout=3 (the build-in value is 0 to boot faster by
> >   default).
> Ok, that might be annoying, but I'd say this doesn't justify the
> Jürgen's changes.

Some hardware cannot be interrupted reliably with global.autoboot_timeout=0
That is more than annoying.

> > - nfs automounts that contain '$user'
> Don't get this one. The nice thing about automount is that they are done
> on request, so isn't it early enough to set global.user when loading the
> debug environment (probably resulting in another line to type)?

Because it's my custom automount that doesn't exist in the default usecase.

> Even though Jürgen predicted that developers won't like me for the
> suggestion, I still think it's wrong to change a production system for
> debug purposes.
> 
> IMHO a righter thing would be to implement an extension to microcom that
> catches the 0s prompt, interrupts it and types
> 
> 	loadenv /dev/tralala
> 	global.user=mol

and setup my automount

> for you. If I didn't miss anything that would catch all problems without
> the need to change barebox.
> 
> > Also, this requires me to know _where_ the environment is. And that is not
> /dev/tralala could be the same for all configurations :-) 

So defining a not-really-default-environment device is a better solution
than these patches?

> > easy to remember when I need to work with multiple devices a day and gets
> > worse, when it changes with the boot source (SD/eMMC). Mistakes are
> > guaranteed.
> I'd say, either you want a) an environment that is used always, or b)
> you don't.
> 
> With a) you can do your modifications to increase boot time and set the
> username and save it for development. If you want to reset it to
> "production mode" just do: loadenv /dev/default_env; saveenv;
> 
> With b) either live with the decision and adapt your *development*
> workflow to it, or change to a).

and there is c):
I want to start with the default environment but change it later without
confusing the user. And right now that's not possible.
A clean setup means writing barebox and overwriting the environment with
something. Currently thats 'nothing' with confuses to user with an error
message. In the past I could use the default environment, but that no
longer exists as a single environment file.

Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-08-06 10:41 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-30 10:20 envfs: provide an intentional way to ignore an existing external environment Juergen Borleis
2014-07-30 10:20 ` [PATCH 1/3] " Juergen Borleis
2014-07-30 10:20 ` [PATCH 2/3] saveenv: change API to be able to forward special flags into the envfs superblock Juergen Borleis
2014-07-30 10:20 ` [PATCH 3/3] saveenv: provide a zeroed/empty/ignore environment Juergen Borleis
2014-07-30 15:28 ` envfs: provide an intentional way to ignore an existing external environment Jan Lübbe
2014-07-30 21:09   ` Sascha Hauer
2014-07-31  7:14 ` Uwe Kleine-König
2014-07-31  7:33   ` Juergen Borleis
2014-07-31  7:44     ` Uwe Kleine-König
2014-07-31  8:03       ` Juergen Borleis
2014-08-06  3:56       ` Jean-Christophe PLAGNIOL-VILLARD
2014-08-06  7:04       ` Michael Olbrich
2014-08-06  7:28         ` Uwe Kleine-König
2014-08-06 10:41           ` Michael Olbrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox