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 <[email protected]>
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 <[email protected]>
---
 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
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Reply via email to