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.

diff --git a/src/repart/repart.c b/src/repart/repart.c
index 6fc48754b18709b2afa247fb4a79d670b7fe1289..2ba3ab2e83981b63d91700e10409002dab8f388f 100644 (file)
--- a/src/repart/repart.c
+++ b/src/repart/repart.c
@@ -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)

diff --git a/test/units/TEST-58-REPART.sh b/test/units/TEST-58-REPART.sh
index bc0edbf206fe3b6f47fed534d41bc83b6d10287e..152b72bb9cdd1e041dc2ec92554a82d88b90707b 100755 (executable)
--- a/test/units/TEST-58-REPART.sh
+++ b/test/units/TEST-58-REPART.sh
@@ -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