On Tue, May 09, 2017 at 11:02:12AM +0100, Ian Abbott wrote:
> On 08/05/17 19:12, Sascha Hauer wrote:
> > On Mon, May 08, 2017 at 03:45:49PM +0100, Ian Abbott wrote:
> > > On 08/05/17 14:39, Ian Abbott wrote:
> > > > Hi,
> > > >
> > > > I'm not sure if this is a bug or whether I'm doing something wrong. In
> > > > barebox 2017.04.0 and earlier, I stored the Ethernet MAC address in a
> > > > non-volatile ('nv') variable dev.eth0.macaddr=xx:xx:xx:xx:xx:xx and that
> > > > got propagated to 'global' and the eth0 device on boot:
> > > >
> > > > barebox@xxxx:/ nv
> > > > allow_color: true
> > > > autoboot_timeout: 3
> > > > dev.eth0.ethaddr: xx:xx:xx:xx:xx:xx
> > > > user: none
> > > [snip]
> > > > However, in 2017.05.0, my 'dev.eth0.ethaddr' variable is no longer being
> > > > propagated to global on boot, and as a consequence, is no longer
> > > > propagated to eth0 (note that 'dev.eth0.ethaddr' is the only nv variable
> > > > that I set manually when setting up a new board):
> > > >
> > > > barebox@xxxx:/ devinfo eth0
> > > > Parameters:
> > > > ethaddr: 00:00:00:00:00:00
> > > > gateway: 0.0.0.0
> > > > ipaddr: 0.0.0.0
> > > > linux.bootargs:
> > > > netmask: 0.0.0.0
> > > > serverip: 0.0.0.0
> > > >
> > > [snip]
> > > > Is this the proper behaviour or a bug? Could it be related to commit
> > > > 35d8e858bea17ec4796069c9c27fd0b134125eaf ("nv: Do not create globalvars
> > > > from nvvars")?
> > >
> > > As a related follow-up, this code in globalvar_add_simple() looks a bit
> > > strange:
> > >
> > > if (value)
> > > dev_set_param(&global_device, name, value);
> > >
> > > globalvar_nv_sync(name);
> >
> > Ok, I think this should just be the other way round. Fixed in a patch I
> > just sent.
>
> Yes, I thought that might be the case about it being the wrong way round. I
> tested your patch and it works.
>
> While on the subject, what is the desired behaviour when removing an nv
> variable? Currently, removing an nv variable also deletes a global with the
> same name, even if the global has changed value in the meantime:
>
> barebox@xxxx:/ global -r quux
> barebox@xxxx:/ nv -r quux
> barebox@xxxx:/ nv quux=foo
> barebox@xxxx:/ global quux=bar
> barebox@xxxx:/ nv -r quux
> barebox@xxxx:/ global
> ... list of globals but 'quux' is missing ...
>
> In 2017-04-0, the global variable remained, although its value was cleared.
Are you sure the global variable is removed? I can't reproduce this
here. Here the globalvar is cleared as it used to be. However, this
behaviour is not very nice and could be changed with the following
patch.
Sacha
-----------------------8<-----------------------------------
>From 45a00b7e6518856fab24afd4b883a6f9c18f9b49 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
Date: Thu, 11 May 2017 08:11:39 +0200
Subject: [PATCH] nv: When a nv var is removed, do not clear the globalvar
When a nv var is removed, do not clear the corresponding global var.
Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
---
common/globalvar.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/common/globalvar.c b/common/globalvar.c
index c528b24062..4922346130 100644
--- a/common/globalvar.c
+++ b/common/globalvar.c
@@ -164,12 +164,15 @@ static int nvvar_device_set(const char *name, const char *val)
return 0;
}
-static int nv_set(struct param_d *p, const char *val)
+static int nv_set(struct param_d *p, const char *_val)
{
struct param_d *g;
int ret;
+ const char *val;
- if (!val)
+ if (_val)
+ val = _val;
+ else
val = "";
ret = nvvar_device_set(p->name, val);
@@ -177,7 +180,7 @@ static int nv_set(struct param_d *p, const char *val)
return ret;
g = get_param_by_name(&global_device, p->name);
- if (g) {
+ if (g && _val) {
ret = dev_set_param(&global_device, p->name, val);
if (ret)
return ret;
--
2.11.0
--
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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
