Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

more resilient power faliure under flash write #5065

Closed
wants to merge 2 commits into from

Conversation

hhaim
Copy link

@hhaim hhaim commented Jan 29, 2019

No description provided.

Signed-off-by: Hanoch Haim <hhaim@cisco.com>
Signed-off-by: Hanoch Haim <hhaim@cisco.com>
@arendst
Copy link
Owner

arendst commented Jan 29, 2019

Thx.

What you might know this crc thing is only available since version 6. Before there was no crc and only the cfg_holder was kind of indication for valid flash.

To make it possible for pre-version 6 users to migrate there were some tricks needed to make it possible.

With that info in mind you understand that just looking for a valid crc does solve your issue but raises problems for the migrants.

I'll look at your solution and see how it fits the migrants.

@arendst arendst added the Awaiting feedback from Project Owner Awaiting feedback / input from Project Owner label Jan 29, 2019
@hhaim
Copy link
Author

hhaim commented Jan 29, 2019 via email

@andrethomas
Copy link
Contributor

In addition to @arendst concerns about migrating users this is a serious concern for me... making a PR which has not been tested.

image

You need to understand that we also have many users running open development binaries (some because they always hunt the new features, some because they are continiously testing and others because open development binaries are the only way to solve a current issue that has been fixed since the last release.)

So you'll need to define some form of scientific reproducible way to prove which scenario's this PR is actually good for as merging an untested or unproven PR could have catastrophic implications for such users.

arendst added a commit that referenced this pull request Jan 30, 2019
Add resiliency to saved Settings (#5065)
@arendst
Copy link
Owner

arendst commented Jan 30, 2019

And I did it my way again.

@arendst arendst closed this Jan 30, 2019
@andrethomas2 andrethomas2 added fixed Result - The work on the issue has ended and removed Awaiting feedback from Project Owner Awaiting feedback / input from Project Owner labels Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed Result - The work on the issue has ended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants