On 12.07.2018 09:20, Maxime Ripard wrote:
On Thu, Jul 12, 2018 at 09:02:26AM +0200, Simon Goldschmidt wrote:
On 11.07.2018 15:50, Maxime Ripard wrote:
On Wed, Jul 11, 2018 at 12:44:23PM +0200, Nicholas wrote:
> Maybe a solution could be to have an env_save() function which > acts in a similar way as proposed in my patch and an > env_save_prio() function, which acts like the env_load() > i.e. looking for the best working location instead of relying > on what has been stored into gd-> env_load_location.
I don't really see a use-case for overriding wherever the environment at the user-level actually. At the board level, for redundancy or transition, yes, definitely, but we can already do that.
Well, the use case I saw was that I wanted to test redundant environment storage. I admit this is not an end user use case but a developer use case, so I guess you're right.
So after fixing this endless loops I see two questions: - to which environment do we store if the one in 'env_load_location' fails to store?
Good question. My opinion is that it strongly depends on what we want to achieve with the implementation: do we want 1) to keep the consistency between load and save, or we want 2) to guarantee to be able to load/save from at least one location?
If 1) then a failure on env_save() simply fails and doesn't store anything. In other words we are multi-env when loading but single-env when storing. We are actually binding env_save() to the last env_load()'s result.
If 2) then a failure on env_save() will try all the available locations but we are open to misalignments. Here we are full multi-env since env_save() and env_load() are not bound together.
In that case, we don't want to store to a lower priority, but to a higher one. Otherwise, the environment will be saved just fine, but will not be read next time, which will be very confusing to the user.
And with a higher priority, you might end up overwriting something you weren't expecting to overwrite, so I'd vote 1.
I agree that 1 would be best. But from reading the code, unless I'm totally wrong, it seems that the patch sent by Nicholas does not suffice:
If no location contained a valid environment (e.g. no environment written yet), the lowest priority will be written as 'gd->env_load_location' is set to the lowest priority from iterating locations in 'env_load()'.
So we might have to reset 'gd->env_load_location' to highest prio if loading the environment fails.
That would make sense yes.
Nicholas has sent v4 of this patch meanwhile, but it's not in reply to v1.
Any comments on that?
Simon