VM: Firmware detection fixes (from Incus) #14050
Merged
Conversation
This file contains 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
tomponline
force-pushed
the
tp-vm-firmwares
branch
from
490d9e9 to
f8e1b91
Compare
Signed-off-by: Stéphane Graber <[email protected]> (cherry picked from commit 20e228fa146569b78a36320a4c808da0782e038f) Signed-off-by: Thomas Parrott <[email protected]> License: Apache-2.0
The NVRAM symlink was previously deleted thanks to a bug in the firmware lookup logic. That logic got recently corrected which in turn caused the symlink to no longer be deleted, causing it to occasionaly point to the incorrect target... Signed-off-by: Stéphane Graber <[email protected]> (cherry picked from commit 17cba6ecb8900c8fc462e8ab59a1cd509567a91d) Signed-off-by: Thomas Parrott <[email protected]> License: Apache-2.0
tomponline
force-pushed
the
tp-vm-firmwares
branch
from
f8e1b91 to
bdf1e44
Compare
tomponline
force-pushed
the
tp-vm-firmwares
branch
from
bdf1e44 to
db587d0
Compare
mihalicyn
previously approved these changes
Sep 6, 2024
…tAchitectureFirmwareVarsCandidates Now that GetArchitectureFirmwarePairsForUsage is checking for the existence of the firmware files (good) it means that calling GetAchitectureFirmwarePairs is also checking for the existence of the firmware files. This is inefficient because GetAchitectureFirmwarePairs is only during setupNvram() in order to build a list of candidate firmware vars names in order to see if the VM currently has any of those to be deleted. In this scenario we don't need to check whether the firmwares exist on the host, and actually it is undesirable to do this as if the firmwares were removed from the host then old vars files may be left behind in the VM's config volume. Additionally the current return value of GetAchitectureFirmwarePairs means that LXD will check if the same files exist several times (because the same vars file name can be used for different usages). So this commit reworks GetAchitectureFirmwarePairs into GetAchitectureFirmwareVarsCandidates which returns just a unqiue slice of candidate var names without checking if the associated firmware files exist on the host. The caller can then use this slice to try and remove the vars files from the VM's config volume. Because it is a unique slice, LXD will only try to remove each file once avoiding unnecessary syscalls. Signed-off-by: Thomas Parrott <[email protected]>
…s usage And error message improvement. Signed-off-by: Thomas Parrott <[email protected]>
…arePairsForUsage Signed-off-by: Thomas Parrott <[email protected]>
tomponline
force-pushed
the
tp-vm-firmwares
branch
from
db587d0 to
f92bde3
Compare
mihalicyn
approved these changes
Sep 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Based on:
Follows on from #14032
Also improves the removal of old firmware vars files by not checking the if the associated firmwares exist on the host and not trying to remove the same files multiple times.