[U-Boot] [PATCH] fw_printenv: Don't bail out directly after one env read error

[Tom Rini]

On Sat, Jun 30, 2018 at 12:35:49PM +0200, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20180629181550.GG18596 at bill-the-cat> you wrote:
> > 
> > > This patch can lead to reading incorrect (old, no longer valid)
> > > values without any way for the user to see what is happening.
> > > 
> > > This must not be done!
> >
> > I'm not 100% sure, after reading all of the code, if there's a problem.
> 
> There is.
> 
> > What we indeed do not want to do is be silent in seeing that the first
> > environment location we read from failed.  But AFAICT if flash_io
> > returns non-zero we also output something useful to stderr, so it should
> > be visible to the user that something went wrong.
> 
> But there is no indication about the problem in the return code, so
> any tools using this have no way to determine there was a problem.

Ah, true, thanks!

> > The next question is,
> > if half of the redundant environment has failed, is the other half
> > considered valid (so long as the crc passes) or would only the built-in
> > be valid?  I would think the other half is the valid one.
> 
> It may be valid, but it is still not what we want.
> 
> redundant environment is used to provide a fall-back in case of
> poblems when _writing_ the environment, i. e. like a power-off while
> a "saveenv" is running.  The purpose is to make sute that even then
> we will always have a valid environment, with correct checksum.
> 
> But as soon as the saveenv has succesfully completed, we do not have
> to equal, exchangable copies - instead, we have one which holds the
> current data (and only this is what represents the "correct" state),
> and another copy, which holds the old state, i. e. which is
> obsolete.
> 
> Sielntly swapping the current and the obsolete data is a serious
> bug, which must never happen.  It may be OK as part of some recovery
> procedure to limit the damage, but it must never ever go unnoticed.
> 
> Assume you switch to another boot device / boot partion as part of a
> system update procedure.  Then booting the wrong system is
> definitely not what you want.
> 
> Please revert!

Joe, Ioan-Adrian, I've reverted this.  Please re-work the changes such
that the use case you need to support is handled in some manner (likely
a new flag being passed to fw_printenv/setenv) such that the current
behavior is still the default, given Wolfgang's explanations above,
thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180702/735ee1aa/attachment.sig>