nv variable changes

nv variable changes

[Sascha Hauer]

This series has some changes for nv variables. The most significant change:
nv variables are now automatically saved when they are changed. This makes
an explicit 'saveenv' unnecessary. Since we do not want excessive write
accesses on the environment storage we save the variables only on shutdown.
The rest of the environment is still shall not automatically be saved, to
implement this the nv variable code reads the environment from storage,
changes the nv variables and writes it back to storage.

----------------------------------------------------------------
Sascha Hauer (6):
      nv: Do not save nv variables while loading
      nv: Save nv variables on shutdown
      nv: Add option to explicitly save nv variables
      nv: Allow to set/remove multiple variables with one command
      nv: Use dev_remove_param to delete nv variable
      nv: Allow wildcards when removing NV vars

 commands/nv.c        |  43 ++++++++++-------
 common/environment.c |   4 ++
 common/globalvar.c   | 134 ++++++++++++++++++++++++++++++++++++++++++++-------
 include/globalvar.h  |   3 ++
 4 files changed, 149 insertions(+), 35 deletions(-)

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

[PATCH 1/6] nv: Do not save nv variables while loading

[Sascha Hauer]

When reading nv variables from the storage in /env/nv we do
not need to write back the value to the file we just read from.
Optimize this a bit and make it unnecessary.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/globalvar.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/common/globalvar.c b/common/globalvar.c
index c0958e1..be08231 100644
--- a/common/globalvar.c
+++ b/common/globalvar.c
@@ -180,15 +180,26 @@ static int nv_set(struct device_d *dev, struct param_d *p, const char *val)
 	free(p->value);
 	p->value = xstrdup(val);
 
-	return nv_save(p->name, val);
+	return 0;
 }
 
-static const char *nv_get(struct device_d *dev, struct param_d *p)
+static const char *nv_param_get(struct device_d *dev, struct param_d *p)
 {
 	return p->value ? p->value : "";
 }
 
-int nvvar_add(const char *name, const char *value)
+static int nv_param_set(struct device_d *dev, struct param_d *p, const char *val)
+{
+	int ret;
+
+	ret = nv_set(dev, p, val);
+	if (ret)
+		return ret;
+
+	return nv_save(p->name, val);
+}
+
+static int __nvvar_add(const char *name, const char *value)
 {
 	struct param_d *p, *gp;
 	int ret;
@@ -226,7 +237,7 @@ int nvvar_add(const char *name, const char *value)
 			return ret;
 	}
 
-	p = dev_add_param(&nv_device, name, nv_set, nv_get, 0);
+	p = dev_add_param(&nv_device, name, nv_param_set, nv_param_get, 0);
 	if (IS_ERR(p))
 		return PTR_ERR(p);
 
@@ -242,6 +253,17 @@ int nvvar_add(const char *name, const char *value)
 	return nv_set(&nv_device, p, value);
 }
 
+int nvvar_add(const char *name, const char *value)
+{
+	int ret;
+
+	ret = __nvvar_add(name, value);
+	if (ret)
+		return ret;
+
+	return nv_save(name, value);
+}
+
 int nvvar_remove(const char *name)
 {
 	struct param_d *p;
@@ -288,7 +310,7 @@ int nvvar_load(void)
 		pr_debug("%s: Setting \"%s\" to \"%s\"\n",
 				__func__, d->d_name, val);
 
-		ret = nvvar_add(d->d_name, val);
+		ret = __nvvar_add(d->d_name, val);
 		if (ret)
 			pr_err("failed to create nv variable %s: %s\n",
 					d->d_name, strerror(-ret));
-- 
2.8.1


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

[PATCH 2/6] nv: Save nv variables on shutdown

[Sascha Hauer]

With this patch nv variables are automatically saved whenever barebox
shuts down (that is 'reset' is executed or a kernel is started). With
this the additional 'saveenv' step becomes unnecessary.

The nv variables are stored in the environment and the estasblished
behaviour is that files in the environment must be manually saved
using 'saveenv'. This behaviour shall be kept for now, so this patch
cannot just call 'saveenv' since that would save the modified
environment files aswell. Instead we read the environment from the
device, modifiy the nv variables and save the environment back.

Since this changes a long existing behaviour messages are printed the
first time a nv variable is modified and during shutdown when the
variables are actually saved.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/environment.c |  4 +++
 common/globalvar.c   | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 include/globalvar.h  |  3 ++
 3 files changed, 85 insertions(+), 3 deletions(-)

diff --git a/common/environment.c b/common/environment.c
index c3ad252..db127d7 100644
--- a/common/environment.c
+++ b/common/environment.c
@@ -364,6 +364,10 @@ int envfs_save(const char *filename, const char *dirname, unsigned flags)
 
 	ret = 0;
 
+#ifdef CONFIG_NVVAR
+	if (!strcmp(filename, default_environment_path_get()))
+	    nv_var_set_clean();
+#endif
 out:
 	close(envfd);
 out1:
diff --git a/common/globalvar.c b/common/globalvar.c
index be08231..4110a06 100644
--- a/common/globalvar.c
+++ b/common/globalvar.c
@@ -9,6 +9,9 @@
 #include <fcntl.h>
 #include <libfile.h>
 #include <generated/utsrelease.h>
+#include <envfs.h>
+
+static int nv_dirty;
 
 struct device_d global_device = {
 	.name = "global",
@@ -20,6 +23,11 @@ struct device_d nv_device = {
 	.id = DEVICE_ID_SINGLE,
 };
 
+void nv_var_set_clean(void)
+{
+	nv_dirty = 0;
+}
+
 int globalvar_add(const char *name,
 		int (*set)(struct device_d *dev, struct param_d *p, const char *val),
 		const char *(*get)(struct device_d *, struct param_d *p),
@@ -43,16 +51,16 @@ void globalvar_remove(const char *name)
 	dev_remove_param(param);
 }
 
-static int nv_save(const char *name, const char *val)
+static int __nv_save(const char *prefix, const char *name, const char *val)
 {
 	int fd, ret;
 	char *fname;
 
-	ret = make_directory("/env/nv");
+	ret = make_directory(prefix);
 	if (ret)
 		return ret;
 
-	fname = basprintf("/env/nv/%s", name);
+	fname = basprintf("%s/%s", prefix, name);
 
 	fd = open(fname, O_CREAT | O_WRONLY | O_TRUNC);
 
@@ -68,6 +76,25 @@ static int nv_save(const char *name, const char *val)
 	return 0;
 }
 
+static int nv_save(const char *name, const char *val)
+{
+	int ret;
+	static int once = 1;
+
+	ret = __nv_save("/env/nv", name, val);
+	if (ret)
+		return ret;
+
+	if (once) {
+		pr_info("nv variable modified, will save nv variables on shutdown\n");
+		once = 0;
+	}
+
+	nv_dirty = 1;
+
+	return 0;
+}
+
 /**
  * dev_param_init_from_nv - initialize a device parameter from nv variable
  * @dev: The device
@@ -423,3 +450,51 @@ static int globalvar_init(void)
 pure_initcall(globalvar_init);
 
 BAREBOX_MAGICVAR_NAMED(global_version, global.version, "The barebox version");
+
+/**
+ * nvvar_save - save NV variables to persistent environment
+ *
+ * This saves the NV variables to the persisitent environment without saving
+ * the other files in the environment that might be changed.
+ */
+int nvvar_save(void)
+{
+	struct param_d *param;
+	const char *env = default_environment_path_get();
+	int ret;
+#define TMPDIR "/.env.tmp"
+	if (!nv_dirty || !env)
+		return 0;
+
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT))
+		defaultenv_load(TMPDIR, 0);
+
+	envfs_load(env, TMPDIR, 0);
+
+	list_for_each_entry(param, &nv_device.parameters, list) {
+		ret = __nv_save(TMPDIR "/nv", param->name,
+				dev_get_param(&nv_device, param->name));
+		if (ret) {
+			pr_err("Cannot save NV var: %s\n", strerror(-ret));
+			goto out;
+		}
+	}
+
+	envfs_save(env, TMPDIR, 0);
+out:
+	unlink_recursive(TMPDIR, NULL);
+
+	if (!ret)
+		nv_dirty = 0;
+
+	return ret;
+}
+
+static void nv_exit(void)
+{
+	if (nv_dirty)
+		pr_info("nv variables modified, saving them\n");
+
+	nvvar_save();
+}
+predevshutdown_exitcall(nv_exit);
diff --git a/include/globalvar.h b/include/globalvar.h
index 67b97de..1cd8d21 100644
--- a/include/globalvar.h
+++ b/include/globalvar.h
@@ -173,4 +173,7 @@ static inline void dev_param_init_from_nv(struct device_d *dev, const char *name
 
 #endif
 
+void nv_var_set_clean(void);
+int nvvar_save(void);
+
 #endif /* __GLOBALVAR_H */
-- 
2.8.1


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

[PATCH 3/6] nv: Add option to explicitly save nv variables

[Sascha Hauer]

We now have code to save the nv variables without saving the
rest of the environment. This gives us the possibility to explicitly
save the nv variables. This patch adds an option to the 'nv' command
for this.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/nv.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/commands/nv.c b/commands/nv.c
index 8cebb85..e312368 100644
--- a/commands/nv.c
+++ b/commands/nv.c
@@ -26,20 +26,26 @@
 static int do_nv(int argc, char *argv[])
 {
 	int opt;
-	int do_remove = 0;
+	int do_remove = 0, do_save = 0;
 	int ret;
 	char *value;
 
-	while ((opt = getopt(argc, argv, "r")) > 0) {
+	while ((opt = getopt(argc, argv, "rs")) > 0) {
 		switch (opt) {
 		case 'r':
 			do_remove = 1;
 			break;
+		case 's':
+			do_save = 1;
+			break;
 		default:
 			return COMMAND_ERROR_USAGE;
 		}
 	}
 
+	if (do_save)
+		return nvvar_save();
+
 	if (argc == optind) {
 		nvvar_print();
 		return 0;
@@ -68,11 +74,12 @@ static int do_nv(int argc, char *argv[])
 BAREBOX_CMD_HELP_START(nv)
 BAREBOX_CMD_HELP_TEXT("Add a new non volatile variable named VAR, optionally set to VALUE.")
 BAREBOX_CMD_HELP_TEXT("non volatile variables are persistent variables that overwrite the")
-BAREBOX_CMD_HELP_TEXT("global variables of the same name. Their value is saved with")
-BAREBOX_CMD_HELP_TEXT("'saveenv'.")
+BAREBOX_CMD_HELP_TEXT("global variables of the same name. Their value is saved implicitly with")
+BAREBOX_CMD_HELP_TEXT("'saveenv' or explicitly with 'nv -s'")
 BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("Options:")
 BAREBOX_CMD_HELP_OPT("-r", "remove a non volatile variable")
+BAREBOX_CMD_HELP_OPT("-s", "Save NV variables")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(nv)
-- 
2.8.1


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

[PATCH 4/6] nv: Allow to set/remove multiple variables with one command

[Sascha Hauer]

It's convenient to set/remove multiple nv variables in one go. With
this patch we iterate over the remaining nonopts instead of expecting
exactly one nonopt.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/nv.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/commands/nv.c b/commands/nv.c
index e312368..a1fb095 100644
--- a/commands/nv.c
+++ b/commands/nv.c
@@ -27,7 +27,7 @@ static int do_nv(int argc, char *argv[])
 {
 	int opt;
 	int do_remove = 0, do_save = 0;
-	int ret;
+	int ret, i;
 	char *value;
 
 	while ((opt = getopt(argc, argv, "rs")) > 0) {
@@ -54,19 +54,21 @@ static int do_nv(int argc, char *argv[])
 	argc -= optind;
 	argv += optind;
 
-	if (argc != 1)
+	if (argc < 1)
 		return COMMAND_ERROR_USAGE;
 
-	value = strchr(argv[0], '=');
-	if (value) {
-		*value = 0;
-		value++;
-	}
+	for (i = 0; i < argc; i++) {
+		value = strchr(argv[0], '=');
+		if (value) {
+			*value = 0;
+			value++;
+		}
 
-	if (do_remove)
-		ret = nvvar_remove(argv[0]);
-	else
-		ret = nvvar_add(argv[0], value);
+		if (do_remove)
+			ret = nvvar_remove(argv[i]);
+		else
+			ret = nvvar_add(argv[i], value);
+	}
 
 	return ret;
 }
@@ -78,14 +80,14 @@ BAREBOX_CMD_HELP_TEXT("global variables of the same name. Their value is saved i
 BAREBOX_CMD_HELP_TEXT("'saveenv' or explicitly with 'nv -s'")
 BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("Options:")
-BAREBOX_CMD_HELP_OPT("-r", "remove a non volatile variable")
+BAREBOX_CMD_HELP_OPT("-r", "remove non volatile variables")
 BAREBOX_CMD_HELP_OPT("-s", "Save NV variables")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(nv)
 	.cmd		= do_nv,
 	BAREBOX_CMD_DESC("create or set non volatile variables")
-	BAREBOX_CMD_OPTS("[-r] VAR[=VALUE]")
+	BAREBOX_CMD_OPTS("[-r] VAR[=VALUE] ...")
 	BAREBOX_CMD_GROUP(CMD_GRP_ENV)
 	BAREBOX_CMD_HELP(cmd_nv_help)
 BAREBOX_CMD_END
-- 
2.8.1


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

[PATCH 5/6] nv: Use dev_remove_param to delete nv variable

[Sascha Hauer]

dev_remove_param() is exactly for the purpose of removing a device
parameter, so use this function instead of open coding the
functionality.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/globalvar.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/common/globalvar.c b/common/globalvar.c
index 4110a06..a2eaaa0 100644
--- a/common/globalvar.c
+++ b/common/globalvar.c
@@ -304,13 +304,12 @@ int nvvar_remove(const char *name)
 		return -ENOENT;
 
 	fname = basprintf("/env/nv/%s", p->name);
+
+	dev_remove_param(p);
+
 	unlink(fname);
 	free(fname);
 
-	list_del(&p->list);
-	free(p->name);
-	free(p);
-
 	return 0;
 }
 
-- 
2.8.1


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

[PATCH 6/6] nv: Allow wildcards when removing NV vars

[Sascha Hauer]

With this patch 'nv -r' can also take "*" and "?" wildcards for nv
variables. This makes it easier to remove multiple nv variables.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/globalvar.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/common/globalvar.c b/common/globalvar.c
index a2eaaa0..44e6528 100644
--- a/common/globalvar.c
+++ b/common/globalvar.c
@@ -10,6 +10,7 @@
 #include <libfile.h>
 #include <generated/utsrelease.h>
 #include <envfs.h>
+#include <fnmatch.h>
 
 static int nv_dirty;
 
@@ -293,22 +294,23 @@ int nvvar_add(const char *name, const char *value)
 
 int nvvar_remove(const char *name)
 {
-	struct param_d *p;
+	struct param_d *p, *tmp;
 	char *fname;
 
 	if (!IS_ENABLED(CONFIG_NVVAR))
 		return -ENOSYS;
 
-	p = get_param_by_name(&nv_device, name);
-	if (!p)
-		return -ENOENT;
+	list_for_each_entry_safe(p, tmp, &nv_device.parameters, list) {
+		if (fnmatch(name, p->name, 0))
+			continue;
 
-	fname = basprintf("/env/nv/%s", p->name);
+		fname = basprintf("/env/nv/%s", p->name);
 
-	dev_remove_param(p);
+		dev_remove_param(p);
 
-	unlink(fname);
-	free(fname);
+		unlink(fname);
+		free(fname);
+	}
 
 	return 0;
 }
-- 
2.8.1


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