repart: Sort the partition list by partition offset
Currently the partition list is ordered like this: First come the partitions that exist as definition files (could be pre-existing partitions or could be new ones), then come the pre-existing partitions that aren't matched to a definition file. This ordering is visible to the user when we print our partition table, and it doesn't really make sense from a UX perspective: Partition tables are usually either presented in order of the partition indices, or in order of the partition offsets. Arguably the latter would be nicer here, since the visualization below is already ordered by physical offsets. So reorder the list after we assigned the new partitions to their respective free areas, according to the physical offset (or, for partitions to newly create, the order that we will allocate them in). Another potential upside of this is that we could rely on the partition order in the code now more, too. To ensure it keeps working, also add a test in the integration tests for it.
This commit is contained in:
Jonas Dreßler
committed by
Luca Boccassi
Luca Boccassi
parent
cc74f8bf36
commit
9c57f703d2
@@ -11266,6 +11266,57 @@ static int determine_auto_size(
|
||||
return 0;
|
||||
}
|
||||
|
||||
static void context_sort_partitions(Context *context) {
|
||||
assert(context);
|
||||
|
||||
Partition *p;
|
||||
LIST_HEAD(Partition, new_partitions) = NULL;
|
||||
LIST_HEAD(Partition, existing_partitions) = NULL;
|
||||
LIST_HEAD(Partition, dropped_partitions) = NULL;
|
||||
|
||||
while ((p = LIST_POP(partitions, context->partitions))) {
|
||||
if (p->allocated_to_area)
|
||||
LIST_APPEND(partitions, new_partitions, p);
|
||||
else if (p->dropped)
|
||||
LIST_APPEND(partitions, dropped_partitions, p);
|
||||
else
|
||||
LIST_APPEND(partitions, existing_partitions, p);
|
||||
}
|
||||
|
||||
/* First sort existing partitions by their offset */
|
||||
while ((p = LIST_POP(partitions, existing_partitions))) {
|
||||
Partition *cursor = NULL;
|
||||
|
||||
assert(p->offset != UINT64_MAX);
|
||||
|
||||
LIST_FOREACH(partitions, q, context->partitions)
|
||||
if (p->offset > q->offset)
|
||||
cursor = q;
|
||||
|
||||
LIST_INSERT_AFTER(partitions, context->partitions, cursor, p);
|
||||
}
|
||||
|
||||
/* Then insert the partitions we'll newly create in the free areas */
|
||||
while ((p = LIST_POP(partitions, new_partitions))) {
|
||||
FreeArea *a = p->allocated_to_area;
|
||||
Partition *cursor = a->after;
|
||||
|
||||
/* Advance past partitions of this area that we already inserted */
|
||||
LIST_FOREACH(partitions, q, context->partitions)
|
||||
if (q->allocated_to_area == a)
|
||||
cursor = q;
|
||||
|
||||
LIST_INSERT_AFTER(partitions, context->partitions, cursor, p);
|
||||
}
|
||||
|
||||
/* Finally append any dropped partitions to the end of the list */
|
||||
while ((p = LIST_POP(partitions, dropped_partitions))) {
|
||||
assert(p->offset == UINT64_MAX);
|
||||
|
||||
LIST_APPEND(partitions, context->partitions, p);
|
||||
}
|
||||
}
|
||||
|
||||
static int context_ponder(Context *context) {
|
||||
int r;
|
||||
|
||||
@@ -11310,6 +11361,10 @@ static int context_ponder(Context *context) {
|
||||
p->definition_path, p->supplement_for->definition_path);
|
||||
}
|
||||
|
||||
/* Now that we know which new partition goes into which free area, reorder
|
||||
* the partitions list so that the list is in the right order. */
|
||||
context_sort_partitions(context);
|
||||
|
||||
/* Now assign free space according to the weight logic */
|
||||
r = context_grow_partitions(context);
|
||||
if (r < 0)
|
||||
|
||||
@@ -706,6 +706,149 @@ EOF
|
||||
assert_in "$imgs/unaligned3 : start= 3662944, size= 17308536, type=${root_guid}, uuid=${root_uuid}, name=\"root-${architecture}\", attrs=\"GUID:59\"" "$output"
|
||||
}
|
||||
|
||||
testcase_output_order() {
|
||||
local defs imgs output
|
||||
|
||||
defs="$(mktemp --directory "/tmp/test-repart.defs.XXXXXXXXXX")"
|
||||
imgs="$(mktemp --directory "/var/tmp/test-repart.imgs.XXXXXXXXXX")"
|
||||
# shellcheck disable=SC2064
|
||||
trap "rm -rf '$defs' '$imgs'" RETURN
|
||||
chmod 0755 "$defs"
|
||||
|
||||
echo "*** Ensure the order of the partition list is correct ***"
|
||||
|
||||
# make this one min size 20MiB so that it has to be assigned slot 4
|
||||
tee "$defs/01-home.conf" <<EOF
|
||||
[Partition]
|
||||
Type=home
|
||||
SizeMinBytes=20971520
|
||||
EOF
|
||||
|
||||
tee "$defs/02-swap.conf" <<EOF
|
||||
[Partition]
|
||||
Type=swap
|
||||
EOF
|
||||
|
||||
tee "$defs/03-esp.conf" <<EOF
|
||||
[Partition]
|
||||
Type=esp
|
||||
EOF
|
||||
|
||||
tee "$defs/04-root.conf" <<EOF
|
||||
[Partition]
|
||||
Type=root-${architecture}
|
||||
EOF
|
||||
|
||||
truncate -s 80MiB "$imgs/order"
|
||||
sfdisk "$imgs/order" <<EOF
|
||||
label: gpt
|
||||
|
||||
size=10M, type=${root_guid}, uuid=837c3d67-21b3-478e-be82-7e7f83bf96d3
|
||||
size=5M, type=${xbootldr_guid}, uuid=4985c03e-eecb-4fe0-9f65-3f6345782214
|
||||
start=30M, size=10M, type=${esp_guid}, uuid=91c30bc9-0187-4db6-81a2-c648294197f8
|
||||
EOF
|
||||
|
||||
output=$(systemd-repart --offline="$OFFLINE" \
|
||||
--definitions="$defs" \
|
||||
--seed="$seed" \
|
||||
--dry-run=no \
|
||||
--json=pretty \
|
||||
"$imgs/order")
|
||||
|
||||
diff -u - <<EOF <(echo "$output")
|
||||
[
|
||||
{
|
||||
"type" : "root-$architecture",
|
||||
"label" : "root-$architecture",
|
||||
"uuid" : "837c3d67-21b3-478e-be82-7e7f83bf96d3",
|
||||
"partno" : 0,
|
||||
"file" : "$defs/04-root.conf",
|
||||
"node" : "$imgs/order1",
|
||||
"offset" : 1048576,
|
||||
"old_size" : 10485760,
|
||||
"raw_size" : 10485760,
|
||||
"size" : "10M",
|
||||
"old_padding" : 0,
|
||||
"raw_padding" : 0,
|
||||
"padding" : "0B",
|
||||
"activity" : "unchanged"
|
||||
},
|
||||
{
|
||||
"type" : "xbootldr",
|
||||
"label" : "xbootldr",
|
||||
"uuid" : "4985c03e-eecb-4fe0-9f65-3f6345782214",
|
||||
"partno" : 1,
|
||||
"file" : null,
|
||||
"node" : "$imgs/order2",
|
||||
"offset" : 11534336,
|
||||
"old_size" : 5242880,
|
||||
"raw_size" : 5242880,
|
||||
"size" : "5M",
|
||||
"old_padding" : 14680064,
|
||||
"raw_padding" : 0,
|
||||
"padding" : "14M -> 0B",
|
||||
"activity" : "unchanged"
|
||||
},
|
||||
{
|
||||
"type" : "swap",
|
||||
"label" : "swap",
|
||||
"uuid" : "78c92db8-3d2b-4823-b0dc-792b78f66f1e",
|
||||
"partno" : 3,
|
||||
"file" : "$defs/02-swap.conf",
|
||||
"node" : "$imgs/order4",
|
||||
"offset" : 16777216,
|
||||
"old_size" : 0,
|
||||
"raw_size" : 14680064,
|
||||
"size" : "-> 14M",
|
||||
"old_padding" : 0,
|
||||
"raw_padding" : 0,
|
||||
"padding" : "-> 0B",
|
||||
"activity" : "create"
|
||||
},
|
||||
{
|
||||
"type" : "esp",
|
||||
"label" : "esp",
|
||||
"uuid" : "91c30bc9-0187-4db6-81a2-c648294197f8",
|
||||
"partno" : 2,
|
||||
"file" : "$defs/03-esp.conf",
|
||||
"node" : "$imgs/order3",
|
||||
"offset" : 31457280,
|
||||
"old_size" : 10485760,
|
||||
"raw_size" : 26202112,
|
||||
"size" : "10M -> 24.9M",
|
||||
"old_padding" : 41922560,
|
||||
"raw_padding" : 0,
|
||||
"padding" : "39.9M -> 0B",
|
||||
"activity" : "resize"
|
||||
},
|
||||
{
|
||||
"type" : "home",
|
||||
"label" : "home",
|
||||
"uuid" : "4980595d-d74a-483a-aa9e-9903879a0ee5",
|
||||
"partno" : 4,
|
||||
"file" : "$defs/01-home.conf",
|
||||
"node" : "$imgs/order5",
|
||||
"offset" : 57659392,
|
||||
"old_size" : 0,
|
||||
"raw_size" : 26206208,
|
||||
"size" : "-> 24.9M",
|
||||
"old_padding" : 0,
|
||||
"raw_padding" : 0,
|
||||
"padding" : "-> 0B",
|
||||
"activity" : "create"
|
||||
}
|
||||
]
|
||||
EOF
|
||||
|
||||
output=$(sfdisk --dump "$imgs/order")
|
||||
|
||||
assert_in "$imgs/order1 : start= 2048, size= 20480, type=${root_guid}," "$output"
|
||||
assert_in "$imgs/order2 : start= 22528, size= 10240, type=${xbootldr_guid}," "$output"
|
||||
assert_in "$imgs/order3 : start= 61440, size= 51176, type=${esp_guid}," "$output"
|
||||
assert_in "$imgs/order4 : start= 32768, size= 28672, type=0657FD6D-A4AB-43C4-84E5-0933C84B4F4F," "$output"
|
||||
assert_in "$imgs/order5 : start= 112616, size= 51184, type=933AC7E1-2EB4-4F13-B844-0E14E2AEF915," "$output"
|
||||
}
|
||||
|
||||
testcase_issue_21817() {
|
||||
local defs imgs output
|
||||
|
||||
|
||||
Reference in New Issue
Block a user