feat: Add support for creating multiple partitions #552
Conversation
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
Reviewer's GuideImplements full multi‐partition support by introducing a part_type parameter, enriching BlivetPartitionVolume with partition type mapping and lookup logic, refining device resolution and safe_mode checks, adjusting imports for parted integration, and updating documentation with comprehensive GPT/MSDOS tests. ER diagram for new part_type attribute in partition volumeserDiagram
VOLUME {
string name
string part_type
string fs_type
string size
string encryption
string mount_point
}
DISK {
string name
string label_type
}
VOLUME ||--|{ DISK : "on"
VOLUME ||--|{ PARTITION : "contains"
PARTITION {
int number
string part_type
string fs_type
}
Class diagram for updated BlivetPartitionVolume partition logicclassDiagram
class BlivetPartitionVolume {
+_update_from_device(param_name)
+_get_part_by_partnum(partnum)
+_get_device_id()
+_get_part_weight()
+_create()
-_type_check()
-_manage_cache()
}
BlivetPartitionVolume --|> BlivetVolume
BlivetPartitionVolume : +part_type
BlivetPartitionVolume : +fs_type
BlivetPartitionVolume : +name
BlivetPartitionVolume : +_blivet_pool
BlivetPartitionVolume : +_device
BlivetPartitionVolume : +_volume
BlivetPartitionVolume : +_get_format()
BlivetPartitionVolume : +_get_size()
BlivetPartitionVolume : +_blivet
BlivetPartitionVolume : +_get_part_weight()
BlivetPartitionVolume : +_get_part_by_partnum()
BlivetPartitionVolume : +_update_from_device()
BlivetPartitionVolume : +_create()
BlivetPartitionVolume : +_get_device_id()
BlivetPartitionVolume : +_manage_cache()
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #552 +/- ##
==========================================
- Coverage 16.54% 10.37% -6.18%
==========================================
Files 2 8 +6
Lines 284 2015 +1731
Branches 79 0 -79
==========================================
+ Hits 47 209 +162
- Misses 237 1806 +1569
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
vojtechtrefny
force-pushed
the
main_partitioning
branch
6 times, most recently
from
e1bb792 to
c9bf341
Compare
Collaborator
Author
|
[citest] |
vojtechtrefny
force-pushed
the
main_partitioning
branch
2 times, most recently
from
2674c29 to
fd4db30
Compare
Collaborator
Author
|
[citest] |
![sourcery-ai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `library/blivet.py:1000` </location>
<code_context>
- def _resize(self):
- pass
+ def _get_part_by_partnum(self, partnum):
+ return next((p for p in self._blivet_pool._disks[0].children if (p.parted_partition and p.parted_partition.number == partnum)),
+ None)
+
+ def _get_device_id(self):
</code_context>
<issue_to_address>
The _get_part_by_partnum method assumes the first disk is always present and partitioned.
Add a guard clause to handle cases where _blivet_pool._disks is empty or the first disk lacks partitions to prevent exceptions.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
def _get_part_by_partnum(self, partnum):
return next((p for p in self._blivet_pool._disks[0].children if (p.parted_partition and p.parted_partition.number == partnum)),
None)
=======
def _get_part_by_partnum(self, partnum):
# Guard clause: ensure disks exist and first disk has children
if not self._blivet_pool._disks:
return None
first_disk = self._blivet_pool._disks[0]
if not hasattr(first_disk, "children") or not first_disk.children:
return None
return next(
(p for p in first_disk.children if (p.parted_partition and p.parted_partition.number == partnum)),
None
)
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `library/blivet.py:1006` </location>
<code_context>
+
+ def _get_device_id(self):
+ name = self._volume['name']
+ if self._blivet_pool._disks[0].partitioned:
+ if name in (p.name for p in self._blivet_pool._disks[0].children):
+ # partition is specified by its name, e.g. "sda1"
+ return name
+ elif name and name.isdigit():
+ # partition is specified by its partition number, e.g. "1"
+ part = self._get_part_by_partnum(int(name))
+ if part:
+ return os.path.basename(part.path)
+ elif name and name[-1].isdigit():
+ # this might be something like "test1", not really supported but we used it previously
</code_context>
<issue_to_address>
Direct access to _blivet_pool._disks[0] may be unsafe if the disk list is empty.
Please add a check to ensure _blivet_pool._disks is not empty before accessing its first element to prevent IndexError.
</issue_to_address>
### Comment 3
<location> `README.md:383` </location>
<code_context>
+ Supported values include `primary`, `extended` and `logical`.
+ If not specified, first three partitions on the disk will be created as primary and fourth
+ one will be created as logical inside a newly created extended partition.
+ `part_type` is ignored on GPT partition table.
+
### `storage_safe_mode`
</code_context>
<issue_to_address>
Consider pluralizing 'partition table' to 'partition tables'.
Change to: '`part_type` is ignored on GPT partition tables.'
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
`part_type` is ignored on GPT partition table.
=======
`part_type` is ignored on GPT partition tables.
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1000
to
+1002
| def _get_part_by_partnum(self, partnum): | ||
| return next((p for p in self._blivet_pool._disks[0].children if (p.parted_partition and p.parted_partition.number == partnum)), | ||
| None) |
There was a problem hiding this comment.
suggestion: The _get_part_by_partnum method assumes the first disk is always present and partitioned.
Add a guard clause to handle cases where _blivet_pool._disks is empty or the first disk lacks partitions to prevent exceptions.
Suggested change
| def _get_part_by_partnum(self, partnum): | |
| return next((p for p in self._blivet_pool._disks[0].children if (p.parted_partition and p.parted_partition.number == partnum)), | |
| None) | |
| def _get_part_by_partnum(self, partnum): | |
| # Guard clause: ensure disks exist and first disk has children | |
| if not self._blivet_pool._disks: | |
| return None | |
| first_disk = self._blivet_pool._disks[0] | |
| if not hasattr(first_disk, "children") or not first_disk.children: | |
| return None | |
| return next( | |
| (p for p in first_disk.children if (p.parted_partition and p.parted_partition.number == partnum)), | |
| None | |
| ) |
| Supported values include `primary`, `extended` and `logical`. | ||
| If not specified, first three partitions on the disk will be created as primary and fourth | ||
| one will be created as logical inside a newly created extended partition. | ||
| `part_type` is ignored on GPT partition table. |
There was a problem hiding this comment.
suggestion (typo): Consider pluralizing 'partition table' to 'partition tables'.
Change to: 'part_type is ignored on GPT partition tables.'
Suggested change
| `part_type` is ignored on GPT partition table. | |
| `part_type` is ignored on GPT partition tables. |
This adds support for specifying size when creating a partition volume and adding multiple partition volumes in one partition pool. Fixes: linux-system-roles#437
vojtechtrefny
force-pushed
the
main_partitioning
branch
from
fd4db30 to
82f1c31
Compare
Contributor
|
[citest] |
Contributor
|
[citest_bad] |
richm
approved these changes
Sep 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Full partitioning support and one related fix found during testing in safe mode.
Summary by Sourcery
Enable full multi-partition support by introducing a part_type option, enhancing partition lookup and ordering, validating partition constraints, and bolstering tests and documentation.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: