repart: Place new partitions at beginning of free area rather than at end

When placing new partitions and there's space left because the new partitions
aren't occupying the whole free space, context_grow_partitions_on_free_area()
is supposed to distribute the free space between the new partitions. If still
no partition wants the free space, the free space ends up becoming padding.

Currently that padding is allocated to the partition preceding the FreeArea
(ie. a->after). This obviously means that the new partitions now end up at
the *end* of the free area rather than at the beginning, which is somewhat
unexpected given how partition placement usually is done.

Fix it by finding the last partition that belongs to the free area, and then
allocating the padding to that partition, so that the new partitions end up
getting aligned with the beginning of the free area, not the end.

Because the span might not be rounded to grain if there's a pre-allocated
a->after partition before the free area, we need to round it down ourselves
(otherwise the "left >= p->new_padding" assertion in context_place_partitions()
is going to fail).

Also ensure the fix works as expected by adding a test.

diff --git a/src/repart/repart.c b/src/repart/repart.c
index 92e1e6ef45e9334ccb6ec88443635abc003adb25..c7814798ad1e79de3660eb5a729aedbbfbebf4c3 100644 (file)
--- a/src/repart/repart.c
+++ b/src/repart/repart.c
@@ -1719,10 +1719,21 @@ static int context_grow_partitions_on_free_area(Context *context, FreeArea *a) {
                                 break;
                 }
 
-        /* Yuck, still no one? Then make it padding */
-        if (span > 0 && a->after) {
-                assert(a->after->new_padding != UINT64_MAX);
-                a->after->new_padding += span;
+        /* Partitions didn't want all the space? Then make it padding */
+        if (span > 0) {
+                Partition *last_partition = NULL;
+
+                LIST_FOREACH(partitions, p, context->partitions)
+                        if (p->allocated_to_area == a)
+                                last_partition = p;
+
+                if (last_partition) {
+                        assert(last_partition->new_padding != UINT64_MAX);
+                        last_partition->new_padding += round_down_size(span, context->grain_size);
+                } else if (a->after) {
+                        assert(a->after->new_padding != UINT64_MAX);
+                        a->after->new_padding += round_down_size(span, context->grain_size);
+                }
         }
 
         return 0;

diff --git a/test/units/TEST-58-REPART.sh b/test/units/TEST-58-REPART.sh
index e2530169ca3fb6b7d49583e9d26087b97c5f0d2d..bc0edbf206fe3b6f47fed534d41bc83b6d10287e 100755 (executable)
--- a/test/units/TEST-58-REPART.sh
+++ b/test/units/TEST-58-REPART.sh
@@ -500,8 +500,8 @@ EOF
                "raw_size" : 33554432,
                "size" : "-> 32M",
                "old_padding" : 0,
-               "raw_padding" : 0,
-               "padding" : "-> 0B",
+               "raw_padding" : 70234112,
+               "padding" : "-> 66.9M",
                "activity" : "create",
                "drop-in_files" : [
                        "$defs/root.conf.d/override1.conf",
@@ -577,8 +577,8 @@ EOF
                "raw_size" : 33554432,
                "size" : "-> 32M",
                "old_padding" : 0,
-               "raw_padding" : 0,
-               "padding" : "-> 0B",
+               "raw_padding" : 36679680,
+               "padding" : "-> 34.9M",
                "activity" : "create"
        }
 ]
@@ -2397,6 +2397,48 @@ EOF
     grep -q tada "${btrfs_mntpoint_encrypted}/magic-encrypted"
 }
 
+testcase_insert_into_gap() {
+    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 "*** Inserting a new partition into a gap between two existing partitions ***"
+
+    truncate -s 71M "$imgs/gap.img"
+    sfdisk "$imgs/gap.img" <<EOF
+label: gpt
+size=10M, type=${esp_guid}, name="part-a",
+start=60M, size=10M, type=${root_guid}, name="part-c",
+EOF
+
+    tee "$defs/new.conf" <<EOF
+[Partition]
+Type=usr
+Label=part-b
+SizeMinBytes=10M
+SizeMaxBytes=10M
+EOF
+
+    systemd-repart --offline="$OFFLINE" \
+                   --definitions="$defs" \
+                   --seed="$seed" \
+                   --dry-run=no \
+                   "$imgs/gap.img"
+
+    output=$(sfdisk --dump "$imgs/gap.img")
+
+    assert_in "$imgs/gap.img1 : start=        2048, size=       20480," "$output"
+    assert_in "$imgs/gap.img2 : start=      122880, size=       20480," "$output"
+
+    # New partition B must start at the beginning of the gap (after A), not at
+    # the end (before C).
+    assert_in "$imgs/gap.img3 : start=       22528, size=       20480, type=$usr_guid, uuid=$usr_uuid, name=\"part-b\"" "$output"
+}
+
 OFFLINE="yes"
 run_testcases