On Sat, Jun 30, 2018 at 12:35:49PM +0200, Wolfgang Denk wrote: > Dear Tom, > > In message <20180629181550.GG18596@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
signature.asc
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
