[PATCH v2] envfs: provide an intentional way to ignore an existing external environment

[PATCH v2] envfs: provide an intentional way to ignore an existing external environment

[Juergen Borleis]

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
zeroed 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 zeroed 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.

This is v2 of this change set.

 - using a forced built-in default environment is handled inside the
   envfs_load() function. Suggested by Sascha.
 - the new flag to force the built-in default environment uses a more
   descriptive name. Suggested by Sascha.

Comments are welcome.

Juergen


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

[PATCH 1/4] saveenv: make clear how to use the command's parameters

[Juergen Borleis]

Both parameters can be omitted and DIRECTORY can be omitted, but the
DIRECTORY parameter requires the ENVFS parameter as well.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 commands/saveenv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commands/saveenv.c b/commands/saveenv.c
index 54b6fa1..29a29c1 100644
--- a/commands/saveenv.c
+++ b/commands/saveenv.c
@@ -55,7 +55,7 @@ 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("[ENVFS [DIRECTORY]]")
 	BAREBOX_CMD_GROUP(CMD_GRP_ENV)
 	BAREBOX_CMD_HELP(cmd_saveenv_help)
 BAREBOX_CMD_END
-- 
2.0.1


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

[PATCH 2/4] envfs: provide an intentional way to ignore an existing external environment

[Juergen Borleis]

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 | 13 +++++++++++++
 include/envfs.h      |  1 +
 2 files changed, 14 insertions(+)

diff --git a/common/environment.c b/common/environment.c
index 2d1edf8..613342c 100644
--- a/common/environment.c
+++ b/common/environment.c
@@ -447,6 +447,19 @@ int envfs_load(const char *filename, const char *dir, unsigned flags)
 	if (ret)
 		goto out;
 
+	if (super.flags & ENVFS_FLAGS_FORCE_BUILT_IN) {
+#ifdef __BAREBOX__
+		if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT)) {
+			ret = defaultenv_load("/env", 0);
+			if (ret)
+				perror("envfs: forced built-in default environment");
+			else
+				printf("envfs: force built-in default environment\n");
+			goto out;
+		}
+#endif
+	}
+
 	buf = xmalloc(size);
 
 	rbuf = buf;
diff --git a/include/envfs.h b/include/envfs.h
index 9b86398..beae38a 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_FORCE_BUILT_IN	(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

[PATCH 3/4] envfs: change API to be able to forward special flags into the envfs superblock

[Juergen Borleis]

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, 19 insertions(+), 11 deletions(-)

diff --git a/commands/saveenv.c b/commands/saveenv.c
index 29a29c1..178b783 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 613342c..f85983a 100644
--- a/common/environment.c
+++ b/common/environment.c
@@ -167,12 +167,13 @@ 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;
@@ -182,11 +183,15 @@ int envfs_save(const char *filename, const char *dirname)
 	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_FORCE_BUILT_IN) {
+		size = 0; /* force no content */
+	} else {
+		/* 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 +201,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_FORCE_BUILT_IN)) {
+		/* 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 beae38a..fdcb8a8 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

[PATCH 4/4] saveenv: provide a zeroed/empty/ignore environment

[Juergen Borleis]

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   | 33 +++++++++++++++++++++++++--------
 scripts/bareboxenv.c | 10 ++++++++--
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/commands/saveenv.c b/commands/saveenv.c
index 178b783..23d9b58 100644
--- a/commands/saveenv.c
+++ b/commands/saveenv.c
@@ -18,26 +18,42 @@
 #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_FORCE_BUILT_IN;
+			if (!IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT))
+				printf("Warning: use of a non existing "
+					"built-in default environment forced!\n");
+			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 +65,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",  "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..249e65b 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_FORCE_BUILT_IN;
+			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