gadget: allow gadget struct with unspecified filesystem to match part with fs #11207
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
Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
… with fs This is primarily to enable devices where there are extra partitions on the disks that are not used with Linux and are instead just treated as binary blobs for other co-processors, etc. and the binary blob just so happens to be an actual image of a filesystem, which leads to discovering the on-disk device as having a filesystem, but the gadget saying there is no filesystem. Before this, we would fail to match these two structures and required instead specifying the filesystem in the gadget.yaml, but that would mean not specifying the content of the partition in the gadget.yaml with an image which is awkward and difficult. Also fundamentally re-organize the lambda function to check a gadget and on-disk structure to be more readable and straight-forward. Finally add a unit test for this case. Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
anonymouse64
added
the
Needs Samuele review
44 tasks
Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
…ates-33 Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
This was referenced Jan 11, 2022
Codecov Report
@@ Coverage Diff @@
## master #11207 +/- ##
==========================================
- Coverage 78.37% 78.36% -0.02%
==========================================
Files 923 923
Lines 105246 105342 +96
==========================================
+ Hits 82484 82548 +64
- Misses 17624 17655 +31
- Partials 5138 5139 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Thanks to Maciej for the suggestion here. Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
mardy
approved these changes
Jan 11, 2022
Thanks to Alberto for the suggestion Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
pedronis
added a commit
that referenced
this pull request
Jan 14, 2022
Merge pull request #11230 from anonymouse64/feature/uc20-multi-volume-gadget-asset-updates-36 I recently realized that there was another corner case to handle here, where ubuntu-image would create a system-data role partition on the disk at image build time when it was not specified in the gadet.yaml. This needs to be taken care of before we can merge #11084 for consistency's sake, so I have prepared some fixes here which are based on top of this PR: gadget: allow gadget struct with unspecified filesystem to match part with fs #11207 Additionally, as requested I broke out one particular change to a separate PR here: gadget: resolve index ambiguity between OnDiskStructure and LaidOutStructure #11234 You can also see how the helpers are used in #11084, which now depends on this specific PR. The main summary of changes: introduce OnDiskStructure.DiskIndex + LaidOutStructure.YamlIndex to fix ambiguity, issue with OnDiskStructure.Index vs LaidOutStructure.Index (as they mean different things and come from different data sources) add onDiskStructureIsLikelyImplicitSystemDataRole helper to identify the implicit system-data role where it may appear add AllowImplicitSystemData to EnsureLayoutCompatibleOptions to allow gadget.yaml's without system-data on UC16/UC18 to match with OnDiskVolume's that have a physical system-data partition The alternative to this is to instead make a hard u-turn from the direction I have been trying to go with this code and instead be very permissive with whatever is on the disk on UC16/UC18 gadgets, in which case we would probably just skip the final check in EnsureLayoutCompatibility entirely and instead with an option ignore all extra stuff on disk that isn't mentioned in the gadget.yaml, but this is a departure from what we are doing on UC20 where we are more strict. I have a reasonable expectation that this is the last case where I find a corner case in what we need to allow UC16/UC18 gadgets to do things that UC20 doesn't, because I don't expect there to be any devices out there that in addition to not having system-data declared in their gadget.yaml also happen to have other random partitions on the disk because that really goes against the whole point of the gadget.yaml's volumes key to declare the partition layout for an Ubuntu Core device.
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.
This is primarily to enable devices where there are extra partitions on the
disks that are not used with Linux and are instead just treated as binary blobs
for other co-processors, etc. and the binary blob just so happens to be an
actual image of a filesystem, which leads to discovering the on-disk device as
having a filesystem, but the gadget saying there is no filesystem.
Before this, we would fail to match these two structures and required instead
specifying the filesystem in the gadget.yaml, but that would mean not
specifying the content of the partition in the gadget.yaml with an image which
is awkward and difficult.
Also fundamentally re-organize the lambda function to check a gadget and
on-disk structure to be more readable and straight-forward.
Finally add a unit test for this case.
This is what we agreed to do to fix @kubiko's device(s) from before the break,
so now you can continue to just use
content: image: foo.imgand not specifythe filesystem of that partition, and we allow it even if on disk it ends up having
a filesystem.