[PATCH v2] barebox update: add note after successful update

[PATCH v2] barebox update: add note after successful update

[Stefan Christ]

Hi Sascha,

On Fri, May 29, 2015 at 09:22:50AM +0200, Sascha Hauer wrote:
> > Oh, I didn't know that the "-z" argument exits. Nice.
> > 
> > Ok, but what about the situation when a user boots the barebox from sdcard and
> > uses the barebox_update handler to flash a barebox to NAND. In such case
> > 'saveenv -z' overwrites the environment on the sdcard.
> 
> I still have no good answer to this :/
> 
> I may also happen that the updated barebox comes with a different
> partition layout so that you can't properly erase the environment from
> the currently running barebox. At some point we thought about adding
> some version counter to the environment so that the updated barebox
> could either issue a warning when an outdated environment is detected
> or ignore it completely. This would of course mean we must not forget to
> increase the version when we do an incompatible change.
> 
> Sascha

Here is the second version of the patch. I added some further explanations for
the user, howto cleanup the environment. I think a mostly working advice is to
boot the new barebox, which is flashed by the barebox_update command, and then
to execute

   $ saveenv -z; loadenv

This should restore the default environment in the running barebox and update
the stored environment. Am I correct?

The advice works when the bootsource is different than the flash target and
when the partition layout changes.

Mit freundlichen Grüßen / Kind regards,
        Stefan Christ

Stefan Christ (1):
  barebox update: add note after successful update

 common/bbu.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

-- 
1.9.1


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

[PATCH v2] barebox update: add note after successful update

[Stefan Christ]

Some users of the barebox_update command forget to erase the external
barebox environment after updating the barebox. Using an old barebox
environment leads to various problems if there were major changes.

So add a gentle reminder after the successful update.

Signed-off-by: Stefan Christ <s.christ@phytec.de>
---
v2: Add explanation howto to clean up the environment
---
 common/bbu.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/common/bbu.c b/common/bbu.c
index 7fb154a..d23b864 100644
--- a/common/bbu.c
+++ b/common/bbu.c
@@ -113,8 +113,15 @@ int barebox_update(struct bbu_data *data)
 	if (ret == -EINTR)
 		printf("update aborted\n");
 
-	if (!ret)
+	if (!ret) {
 		printf("update succeeded\n");
+		if (IS_ENABLED(CONFIG_ENV_HANDLING)) {
+			printf("You maybe want to erase the barebox environment now.\n");
+			printf("After booting the flashed barebox you can use\n");
+			printf("    $ saveenv -z; loadenv\n");
+			printf("to do that.\n");
+		}
+	}
 
 	return ret;
 }
-- 
1.9.1


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

Re: [PATCH v2] barebox update: add note after successful update

[Eric Bénard]

Hi Stefan,

Le Fri, 5 Jun 2015 09:51:25 +0200,
Stefan Christ <s.christ@phytec.de> a écrit :

> Some users of the barebox_update command forget to erase the external
> barebox environment after updating the barebox. Using an old barebox
> environment leads to various problems if there were major changes.
> 
> So add a gentle reminder after the successful update.
> 
> Signed-off-by: Stefan Christ <s.christ@phytec.de>
> ---
> v2: Add explanation howto to clean up the environment
> ---
>  common/bbu.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/common/bbu.c b/common/bbu.c
> index 7fb154a..d23b864 100644
> --- a/common/bbu.c
> +++ b/common/bbu.c
> @@ -113,8 +113,15 @@ int barebox_update(struct bbu_data *data)
>  	if (ret == -EINTR)
>  		printf("update aborted\n");
>  
> -	if (!ret)
> +	if (!ret) {
>  		printf("update succeeded\n");
> +		if (IS_ENABLED(CONFIG_ENV_HANDLING)) {
> +			printf("You maybe want to erase the barebox environment now.\n");
> +			printf("After booting the flashed barebox you can use\n");
> +			printf("    $ saveenv -z; loadenv\n");
> +			printf("to do that.\n");
> +		}
> +	}

maybe loadenv -d; saveenv is less invasive than saveenv -z in order to
preserve nv ?

Eric

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

Re: [PATCH v2] barebox update: add note after successful update

[Stefan Christ]

Hi Eric,

> maybe loadenv -d; saveenv is less invasive than saveenv -z in order to
> preserve nv ?

Please correct me if I'm wrong. 'loadenv -d' will reset all nv variables which
are already defined in the default environment. Only variables which are not in
the default environment are left as is.

The same holds for all files in /env/. If they don't exist in the default
environment, they won't be overwritten. Right?

Hmm. I would prefer to cleanup the whole environment for the user, so no left
over files may interfere the boot process. That guarantees a consistent state
of the environment, which should work as-is.

What are your thoughts?

Mit freundlichen Grüßen / Kind regards,
	Stefan Christ

On Fri, Jun 05, 2015 at 09:59:19AM +0200, Eric Bénard wrote:
> Hi Stefan,
> 
> Le Fri, 5 Jun 2015 09:51:25 +0200,
> Stefan Christ <s.christ@phytec.de> a écrit :
> 
> > Some users of the barebox_update command forget to erase the external
> > barebox environment after updating the barebox. Using an old barebox
> > environment leads to various problems if there were major changes.
> > 
> > So add a gentle reminder after the successful update.
> > 
> > Signed-off-by: Stefan Christ <s.christ@phytec.de>
> > ---
> > v2: Add explanation howto to clean up the environment
> > ---
> >  common/bbu.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/common/bbu.c b/common/bbu.c
> > index 7fb154a..d23b864 100644
> > --- a/common/bbu.c
> > +++ b/common/bbu.c
> > @@ -113,8 +113,15 @@ int barebox_update(struct bbu_data *data)
> >  	if (ret == -EINTR)
> >  		printf("update aborted\n");
> >  
> > -	if (!ret)
> > +	if (!ret) {
> >  		printf("update succeeded\n");
> > +		if (IS_ENABLED(CONFIG_ENV_HANDLING)) {
> > +			printf("You maybe want to erase the barebox environment now.\n");
> > +			printf("After booting the flashed barebox you can use\n");
> > +			printf("    $ saveenv -z; loadenv\n");
> > +			printf("to do that.\n");
> > +		}
> > +	}
> 
> maybe loadenv -d; saveenv is less invasive than saveenv -z in order to
> preserve nv ?
> 
> Eric

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

Re: [PATCH v2] barebox update: add note after successful update

[Stefan Christ]

Hi Sascha,

*ping, is this patch acceptable?

Mit freundlichen Grüßen / Kind regards,
	Stefan Christ

On Fri, Jun 05, 2015 at 09:51:25AM +0200, Stefan Christ wrote:
> Some users of the barebox_update command forget to erase the external
> barebox environment after updating the barebox. Using an old barebox
> environment leads to various problems if there were major changes.
> 
> So add a gentle reminder after the successful update.
> 
> Signed-off-by: Stefan Christ <s.christ@phytec.de>
> ---
> v2: Add explanation howto to clean up the environment
> ---
>  common/bbu.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/common/bbu.c b/common/bbu.c
> index 7fb154a..d23b864 100644
> --- a/common/bbu.c
> +++ b/common/bbu.c
> @@ -113,8 +113,15 @@ int barebox_update(struct bbu_data *data)
>  	if (ret == -EINTR)
>  		printf("update aborted\n");
>  
> -	if (!ret)
> +	if (!ret) {
>  		printf("update succeeded\n");
> +		if (IS_ENABLED(CONFIG_ENV_HANDLING)) {
> +			printf("You maybe want to erase the barebox environment now.\n");
> +			printf("After booting the flashed barebox you can use\n");
> +			printf("    $ saveenv -z; loadenv\n");
> +			printf("to do that.\n");
> +		}
> +	}
>  
>  	return ret;
>  }
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

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

Re: [PATCH v2] barebox update: add note after successful update

[Sascha Hauer]

Hi Stefan,

On Mon, Jun 15, 2015 at 12:20:08PM +0200, Stefan Christ wrote:
> Hi Sascha,
> 
> *ping, is this patch acceptable?

I thought about this again.

I would find this message more useful from the new, updated barebox
rather than from the barebox that does the update. This way we could
also see the message with offline updates when for example a SD
card has been updated on an external host. Also we would have more
freedom to react on an outdated environment in the next steps. We could
for example make it configurable to completely ignore an outdated
environment or just to issue a warning message.
Doing this should be fairly simple, we could store the barebox version
in a nv variable and compare the variable with the current version
during startup.

What do you think?

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

Re: [PATCH v2] barebox update: add note after successful update

[Stefan Christ]

Hi Sascha,

> I would find this message more useful from the new, updated barebox
> rather than from the barebox that does the update. This way we could
> also see the message with offline updates when for example a SD
> card has been updated on an external host. Also we would have more
> freedom to react on an outdated environment in the next steps. We could
> for example make it configurable to completely ignore an outdated
> environment or just to issue a warning message.
> Doing this should be fairly simple, we could store the barebox version
> in a nv variable and compare the variable with the current version
> during startup.
> 
> What do you think?

This would solve the issue for our users.

If I understand you correctly, this approach is different from a separate
versioning of the environment data, since the barebox verison in UTS_RELEASE is
written to the nv variable. So nobody can forget to increase the version number
of the environment and a warning message is printing to the user as a new
barebox with a different version is flashed.

I looked at the code a bit. The first step would be to write the version of the
running barebox into environment in the function envfs_save() and compare the
version in the function envfs_load(). Correct? In envfs_load() the warning
message would be printed.

But we cannot interrupt the boot sequence and ask the user whether he/she wants
to use the old environment or use the default environment, because the default
boot process must be non-interactive. So only a warning can be printed, right?

Mit freundlichen Grüßen / Kind regards,
	Stefan Christ

On Tue, Jun 16, 2015 at 03:52:41PM +0200, Sascha Hauer wrote:
> Hi Stefan,
> 
> On Mon, Jun 15, 2015 at 12:20:08PM +0200, Stefan Christ wrote:
> > Hi Sascha,
> > 
> > *ping, is this patch acceptable?
> 
> I thought about this again.
> 
> I would find this message more useful from the new, updated barebox
> rather than from the barebox that does the update. This way we could
> also see the message with offline updates when for example a SD
> card has been updated on an external host. Also we would have more
> freedom to react on an outdated environment in the next steps. We could
> for example make it configurable to completely ignore an outdated
> environment or just to issue a warning message.
> Doing this should be fairly simple, we could store the barebox version
> in a nv variable and compare the variable with the current version
> during startup.
> 
> What do you think?
> 
> 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

Re: [PATCH v2] barebox update: add note after successful update

[Sascha Hauer]

On Thu, Jun 18, 2015 at 09:57:23AM +0200, Stefan Christ wrote:
> Hi Sascha,
> 
> > I would find this message more useful from the new, updated barebox
> > rather than from the barebox that does the update. This way we could
> > also see the message with offline updates when for example a SD
> > card has been updated on an external host. Also we would have more
> > freedom to react on an outdated environment in the next steps. We could
> > for example make it configurable to completely ignore an outdated
> > environment or just to issue a warning message.
> > Doing this should be fairly simple, we could store the barebox version
> > in a nv variable and compare the variable with the current version
> > during startup.
> > 
> > What do you think?
> 
> This would solve the issue for our users.
> 
> If I understand you correctly, this approach is different from a separate
> versioning of the environment data, since the barebox verison in UTS_RELEASE is
> written to the nv variable. So nobody can forget to increase the version number
> of the environment and a warning message is printing to the user as a new
> barebox with a different version is flashed.

Yes.

> 
> I looked at the code a bit. The first step would be to write the version of the
> running barebox into environment in the function envfs_save() and compare the
> version in the function envfs_load(). Correct? In envfs_load() the warning
> message would be printed.

Yes.

> 
> But we cannot interrupt the boot sequence and ask the user whether he/she wants
> to use the old environment or use the default environment, because the default
> boot process must be non-interactive. So only a warning can be printed, right?

Right. We can at some point make it configurable to a) use the default
env b) Use the saved env anyway or c) do something else in such a case.
For now I think being able to detect an outdated environment and to 
print a warning is a good first step.

BTW I think it's clear that an environment generated with another
barebox version doesn't necessarily mean that it's outdated. Also
detecting the same version doesn't necessarily mean it's compatible.
It's just a good indicator. The alternative would be to have some
counter which we update with each incompatible environment change, but
I think we are not always aware of incompatible changes so increasing
that counter would be forgotten. Also not all incompatible changes
would affect all boards. I think such a counter would not be feasible.

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