when preserving existing partitions, size: -1 is calculated incorrectly

[ov2k]

When preserving existing partitions, specifying a partition size as -1 causes curtin to fail. /var/log/curtin/install.log indicates the expected and observed partition sizes do not match. The expected partition size is calculated by subiquity and recorded in /var/log/installer/subiquity-curtin-install.conf, and that expected size is incorrect.

A user-data file is attached that demonstrates the problem. For anybody trying to use it, it is designed to be run on an EFI system. Partitioning is done via an early command.

For the purposes of hard numbers, I tested with a 10GB disk. It has 20971520 sectors. The 512MB ESP partition has 1048576 sectors. The 2048 leading sectors and the 33 trailing sectors of the disk are reserved by GPT or otherwise unused. That leaves 20971520 - 1048576 - 2048 - 33 = 19920863 sectors, which is exactly what is reported by fdisk. For whatever reason, subiquity seems to calculate 19920863 - 2048 + 33 = 19918848 sectors as the size, which it then provides (in bytes) to curtin.

Things work when the partition size is explicitly provided, but this defeats the point of size: -1.

[ov2k (ov2k)]

The basic problem is that subiquity reserves 2MiB for GPT, but GPT only reserves 66 sectors (33KiB). When partitioning with sfdisk, as in the provided example, specifying that a partition's size should be "as much as possible" to "end-of-device" causes 33 sectors at the end of the disk to be reserved for GPT. However, the partition size calculated by subiquity effectively reserves 1MiB at the end of the disk.

Superficially, this is a disagreement between GPT, sfdisk, and subiquity over how much to reserve. However, there are other problems with the size calculation done by subiquity. That calculation is effectively:

GPT_OVERHEAD = 2 * (1 << 20)
self.size - self.used - GPT_OVERHEAD

This doesn't account for the actual partition arrangement. If the first partition does not start at sector 2048 or if there are holes in the partition table, then the end of the last partition will not be at 1MiB + self.used.

I think a better approach would do different things depending on whether the partition is to be preserved or not. If it is to be preserved, it would just use the actual size of the partition. If it is not to be preserved, then subiquity should probably account for the last sector of the previous partition.

Finally, when preserving partitions, I don't think the size needs to be required, as partition numbers apparently have to be given anyway:

https://bugs.launchpad.net/subiquity/+bug/1885638

So to summarize, I have a few suggestions:

1. Don't require size when preserving a partition.
2. Account for position of the end of the last partition.
3. Use the actual partition size when preserving a partition with size: -1.

[Michael Hudson-Doyle (mwhudson)]

I admit it had never occurred to me that you would use size: -1 on a preserved partition. I think it makes sense to not require size to be set when preserving a partition, but that requires curtin changes I think.

[ov2k (ov2k)]

It’s useful when same configuration should be applied to multiple
computers, but the disks containing the root filesystem differ in size.
It’s also useful when demonstrating bugs without requiring developers to
use a disk of a specific size.

Even if curtin stops requiring the size when partitions are preserved,
there are still issues with how subiquity calculates the size. A user might
still provide both a partition number and size: -1. Even if the partition
with size: -1 is not preserved, a preserved first partition starting after
sector 2048 or holes in the preserved portion of the partition table will
cause problems.

On Tue, Jul 14, 2020 at 10:50 PM Michael Hudson-Doyle <
<email address hidden>> wrote:

> I admit it had never occurred to me that you would use size: -1 on a
> preserved partition. I think it makes sense to not require size to be
> set when preserving a partition, but that requires curtin changes I
> think.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1885635
>
> Title:
> when preserving existing partitions, size: -1 is calculated
> incorrectly
>
> Status in subiquity:
> New
>
> Bug description:
> When preserving existing partitions, specifying a partition size as -1
> causes curtin to fail. /var/log/curtin/install.log indicates the
> expected and observed partition sizes do not match. The expected
> partition size is calculated by subiquity and recorded in
> /var/log/installer/subiquity-curtin-install.conf, and that expected
> size is incorrect.
>
> A user-data file is attached that demonstrates the problem. For
> anybody trying to use it, it is designed to be run on an EFI system.
> Partitioning is done via an early command.
>
> For the purposes of hard numbers, I tested with a 10GB disk. It has
> 20971520 sectors. The 512MB ESP partition has 1048576 sectors. The
> 2048 leading sectors and the 33 trailing sectors of the disk are
> reserved by GPT or otherwise unused. That leaves 20971520 - 1048576 -
> 2048 - 33 = 19920863 sectors, which is exactly what is reported by
> fdisk. For whatever reason, subiquity seems to calculate 19920863 -
> 2048 + 33 = 19918848 sectors as the size, which it then provides (in
> bytes) to curtin.
>
> Things work when the partition size is explicitly provided, but this
> defeats the point of size: -1.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/subiquity/+bug/1885635/+subscriptions
>