boot: introduce good recovery systems, provide compatibility handling#9943
Merged
mvo5 merged 6 commits intocanonical:masterfrom Feb 22, 2021
Merged
Conversation
Add a new modeenv entry to tracking recovery systems that have been verified to work. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
github-actions
bot
removed
the
Run nested
bboozzoo
added a commit
to bboozzoo/snapd
that referenced
this pull request
Feb 18, 2021
…being removed Auto labeler seems to be removing the labels unexpectedly, see: - canonical#9940 (comment) label was added manually when opening a PR and later removed by bot - canonical#9943 (comment) modified files match the label, but labeler removed 'Run nested' label Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
anonymouse64
approved these changes
Feb 18, 2021
Contributor
anonymouse64
left a comment
anonymouse64
left a comment
There was a problem hiding this comment.
lgtm, one naming question though that is probably irrelevant
boot/bootstate20.go
Outdated
| dev Device | ||
| } | ||
|
|
||
| func (ba20 *bootState20RecoverySystem) markSuccessful(update bootStateUpdate) (bootStateUpdate, error) { |
Contributor
There was a problem hiding this comment.
Suggested change
| func (ba20 *bootState20RecoverySystem) markSuccessful(update bootStateUpdate) (bootStateUpdate, error) { | |
| func (brs20 *bootState20RecoverySystem) markSuccessful(update bootStateUpdate) (bootStateUpdate, error) { |
?
but again the acronym short hands for all the bootstate things are getting a bit unreadable anyways so maybe it doesn't matter so much
Contributor
There was a problem hiding this comment.
I think ba20 comes from bootState20BootAssets. So yes brs20 here, and bcl20 for the commandline previously landed would be better.
boot/modeenv.go
Outdated
| Mode string `key:"mode"` | ||
| RecoverySystem string `key:"recovery_system"` | ||
| // CurrentRecoverySystems is a list of labels corresponding to recovery | ||
| // systems have been verified or are in the process of being tried, thus |
Contributor
There was a problem hiding this comment.
Suggested change
| // systems have been verified or are in the process of being tried, thus | |
| // systems that have been verified or are in the process of being tried, thus |
| // clear kernel command lines list | ||
| modeenv2.CurrentKernelCommandLines = nil | ||
| c.Assert(modeenv1.DeepEqual(modeenv2), Equals, false) | ||
|
|
Contributor
There was a problem hiding this comment.
thanks for updating this test
pedronis
approved these changes
Feb 18, 2021
Contributor
pedronis
left a comment
pedronis
left a comment
There was a problem hiding this comment.
thanks, some small comments, some bits that that were reused need some tweaks
boot/boot_test.go
Outdated
| b := bootloadertest.Mock("mock", s.bootdir) | ||
| s.forceBootloader(b) | ||
|
|
||
| // a pending kernel command line change |
boot/boot_test.go
Outdated
| b := bootloadertest.Mock("mock", s.bootdir) | ||
| s.forceBootloader(b) | ||
|
|
||
| // a pending kernel command line change |
| // check the modeenv | ||
| m2, err := boot.ReadModeenv("") | ||
| c.Assert(err, IsNil) | ||
| // good recovery systems has been populated |
boot/modeenv.go
Outdated
| Mode string `key:"mode"` | ||
| RecoverySystem string `key:"recovery_system"` | ||
| // CurrentRecoverySystems is a list of labels corresponding to recovery | ||
| // systems have been verified or are in the process of being tried, thus |
boot/modeenv.go
Outdated
| BrandID string `key:"model,secondary"` | ||
| Grade string `key:"grade"` | ||
| // GoodRecoverySystems is a list of labels corresponding to recovery | ||
| // systems were verified and we are prepared to use for recovering. The |
boot/bootstate20.go
Outdated
| dev Device | ||
| } | ||
|
|
||
| func (ba20 *bootState20RecoverySystem) markSuccessful(update bootStateUpdate) (bootStateUpdate, error) { |
Contributor
There was a problem hiding this comment.
I think ba20 comes from bootState20BootAssets. So yes brs20 here, and bcl20 for the commandline previously landed would be better.
…eenv-good-recovery-systems
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduce good_recover_systems into modeev. Provide basic compatibility handling.