Multiple fixes for size: -1 partitions #1445
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
In the new tests recently added to test_filesystem.py, the initial disk object was marked having a msdos partition table. This is fine for computing offsets manually. However, what really needs to be done is to mark the partition table as msdos in the YAML itself. Otherwise, the disk is reformatted as GPT. This makes gap functions use the right alignment data when computing the list of gaps available. Sadly it breaks the tests because of other issues, so I added FIXME tags in the broken tests. Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
The available_for_partitions function wrongly assumed that the underlying disk had a GPT partition table. Updated the implementation to use the alignment data as expected. Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
When storage version 2 is used, partitions require an offset. When loading partitions via an autoinstall file, we now make sure to assign offsets automatically if the user did not specify offsets. https://bugs.launchpad.net/ubuntu/+source/subiquity/+bug/1991413 Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
When size: -1 is specified on a given partition, we attempt to determine the its right size by looping through the partitions and computing a list of available gaps. That said, when looping through the partitions, we also include the one which has an incomplete (i.e. = -1) size. This makes the computation rely on a negative value and this often leads to incorrect result. Fixed by excluding the current partition when determining its appropriate size. https://bugs.launchpad.net/ubuntu/+source/subiquity/+bug/1991929 Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
ogayot
force-pushed
the
LP1991413-LP1991929
branch
from
57c6475
to
01060fc
Compare
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
dbungert
approved these changes
Oct 28, 2022
…fsets Storage version 2 is a global setting. Having one disk use storage version 1 and another with storage version 2 should not be possible and should not be considered a valid use-case. Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
dbungert
approved these changes
Oct 30, 2022
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.
disk.available_for_partitionsthat always considered the disk had a GPT.size: -1. https://bugs.launchpad.net/ubuntu/+source/subiquity/+bug/1991413size: -1. The value-1(that is an valid integer) was used accidentally in the computations to determine the appropriate size for the partition. https://bugs.launchpad.net/ubuntu/+source/subiquity/+bug/1991929I am now able run a successful autoinstall with the data from https://bugs.launchpad.net/subiquity/+bug/1989977 without hard-coding any offset. The 1MiB that was missing on the final logical partition in the earlier implementation is now claimed properly.
I added this test case as a unit test:
test_partition_remaining_size_in_extended_and_logical_multiple