add support for custom image sizes #1826
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
jpculp
reviewed
Nov 16, 2021
tjkirch
reviewed
Nov 16, 2021
There was a problem hiding this comment.
I like it, it's just clever enough to get the flexibility we need without the logic being too hard to read. The math involved, though, is tough and non-obvious - I think it'd help to have an ASCII/draw.io diagram of an example partition layout with an explanation of MB vs. MiB vs. GiB vs. sectors where appropriate. The old sgdisk call at least had the partitions laid out next to each other, which helped a bit, even though it was hiding some secrets.
LGTM if upgrade/downgrade testing comes out OK.
bcressey
force-pushed
the
custom-image-size
branch
from
59f7c9b to
0cb08d7
Compare
tjkirch
approved these changes
Nov 17, 2021
bcressey
force-pushed
the
custom-image-size
branch
from
0cb08d7 to
1c59956
Compare
|
Fix or selectively disable shellcheck warnings. |
tjkirch
approved these changes
Nov 17, 2021
Move all decisions about partition sizes, types, etc into a library so that it's easier to support layouts for different disk sizes. The layout is still essentially static, but can be scaled up as needed to support larger root filesystems. For the 2 GiB OS image layout, the end result is almost identical, except for fixing a longstanding bug where the last two partitions on each disk - PRIVATE and DATA - were (1 MiB + 1 sector) in size, and not 1 MiB as intended. Signed-off-by: Ben Cressey <bcressey@amazon.com>
Add a new structure to variant metadata so the OS and data disk sizes can be overridden. The previous values - 2 GiB for the OS disk, and 1 GIB for the data disk, are promoted to the default values so existing variant definitions continue working. References to "disk image" in `rpm2img` have been changed to refer to "os image" instead, to avoid confusion over which image is meant. The expected sizes for these images are now required parameters. Signed-off-by: Ben Cressey <bcressey@amazon.com>
bcressey
force-pushed
the
custom-image-size
branch
from
1c59956 to
0bd6242
Compare
tjkirch
approved these changes
Nov 17, 2021
jpculp
approved these changes
Nov 18, 2021
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.
Issue number:
Fixes #1821
Description of changes:
Refactor partition layout code into a helper library, and add support for custom OS and data image sizes in the variant specification.
Fix a bug that affected the last partition on each disk, where it was one sector larger than intended. (The corresponding filesystems were already the correct size.)
Testing done:
Confirmed that the "old" and "new" layouts only differ by one sector in the last partition.
Old layout
New layout
Confirmed that an undersized hash partition is rejected if I reduce
HASH_SCALE_FACTORto2.Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.