[PATCH] expand-on-first-boot: Allow expanding extended MBR partitions

[Harald Seiler]

Extended partitions cannot be resized like primary partitions because
both the last logical partition and the EBR primary partition containing
it need to be expanded.

To do this, add a second SED directive for removing the parition size of
the EBR partition. This part is detected by having either type `f` (Win
95 Ext') or `5` (Extended).

Signed-off-by: Harald Seiler <h...@denx.de>
---

Notes:
I have tested this in an ongoing project and it seems to work
reliably. For some reason WIC creates an extended partition of
type `f` but this can't be kept because while fdisk can read it,
it is only happy about writing the EBR with type `5`.

While probably a very uncommon layout, I am not sure how this
would behave when the EBR is not the last partition, but
somewhere in between. I am a bit worried it would silently wreak
havoc ...

.../expand-on-first-boot/files/expand-last-partition.sh | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh b/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
index 08c69db30529..ddf1a089e87d 100755
--- a/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
+++ b/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
@@ -17,12 +17,13 @@ if [ "${ROOT_DEV}" = "${BOOT_DEV}" ]; then
exit 1
fi

-LAST_PART="$(sfdisk -d ${BOOT_DEV} 2>/dev/null | tail -1 | cut -d ' ' -f 1)"
+LAST_PART="$(sfdisk -d "${BOOT_DEV}" 2>/dev/null | tail -1 | cut -d ' ' -f 1)"

# Remove all hints to the current medium (last-lba) and last partition size,
# then ask sfdisk to recreate the partitioning
sfdisk -d "${BOOT_DEV}" 2>/dev/null | grep -v last-lba | \
- sed 's|\('"${LAST_PART}"' .*, \)size=[^,]*, |\1|' | \
+ sed 's|^\(.*, \)size=[^,]*, type=[f5]$|\1type=5|' | \
+ sed 's|^\('"${LAST_PART}"' .*, \)size=[^,]*, |\1|' | \
sfdisk --force "${BOOT_DEV}"

# Inform the kernel about the partitioning change
--
2.25.4

[Jan Kiszka]

On 10.06.20 10:02, Harald Seiler wrote:
> Extended partitions cannot be resized like primary partitions because
> both the last logical partition and the EBR primary partition containing
> it need to be expanded.
>
> To do this, add a second SED directive for removing the parition size of
> the EBR partition. This part is detected by having either type `f` (Win
> 95 Ext') or `5` (Extended).
>
> Signed-off-by: Harald Seiler <h...@denx.de>
> ---
>
> Notes:
> I have tested this in an ongoing project and it seems to work
> reliably. For some reason WIC creates an extended partition of
> type `f` but this can't be kept because while fdisk can read it,
> it is only happy about writing the EBR with type `5`.

Wikipedia says about 0x5:

"Extended partition with CHS addressing. It must reside within the first
physical 8 GB of disk, else use 0Fh instead"

So we likely need to account for that case as well.

>
> While probably a very uncommon layout, I am not sure how this
> would behave when the EBR is not the last partition, but
> somewhere in between. I am a bit worried it would silently wreak
> havoc ...

We should probably throw some test patterns at that. Could be done
against a disk image file as well, I suppose.

>
> .../expand-on-first-boot/files/expand-last-partition.sh | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh b/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
> index 08c69db30529..ddf1a089e87d 100755
> --- a/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
> +++ b/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
> @@ -17,12 +17,13 @@ if [ "${ROOT_DEV}" = "${BOOT_DEV}" ]; then
> exit 1
> fi
>
> -LAST_PART="$(sfdisk -d ${BOOT_DEV} 2>/dev/null | tail -1 | cut -d ' ' -f 1)"
> +LAST_PART="$(sfdisk -d "${BOOT_DEV}" 2>/dev/null | tail -1 | cut -d ' ' -f 1)"
>
> # Remove all hints to the current medium (last-lba) and last partition size,
> # then ask sfdisk to recreate the partitioning

The comment should likely be updated as well.

> sfdisk -d "${BOOT_DEV}" 2>/dev/null | grep -v last-lba | \
> - sed 's|\('"${LAST_PART}"' .*, \)size=[^,]*, |\1|' | \
> + sed 's|^\(.*, \)size=[^,]*, type=[f5]$|\1type=5|' | \
> + sed 's|^\('"${LAST_PART}"' .*, \)size=[^,]*, |\1|' | \

Would it make things worse or better to combine both sed expressions
into the same sed call?

> sfdisk --force "${BOOT_DEV}"
>
> # Inform the kernel about the partitioning change
>

Jan

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

[Baurzhan Ismagulov]

On Wed, Jun 10, 2020 at 02:50:40PM +0200, Jan Kiszka wrote:
> > While probably a very uncommon layout, I am not sure how this
> > would behave when the EBR is not the last partition, but
> > somewhere in between. I am a bit worried it would silently wreak
> > havoc ...
>
> We should probably throw some test patterns at that. Could be done
> against a disk image file as well, I suppose.

That would be great.

> > sfdisk -d "${BOOT_DEV}" 2>/dev/null | grep -v last-lba | \
> > - sed 's|\('"${LAST_PART}"' .*, \)size=[^,]*, |\1|' | \
> > + sed 's|^\(.*, \)size=[^,]*, type=[f5]$|\1type=5|' | \
> > + sed 's|^\('"${LAST_PART}"' .*, \)size=[^,]*, |\1|' | \
>
> Would it make things worse or better to combine both sed expressions
> into the same sed call?

From the performance PoV, yes. However, they tend to become less readable.
Perhaps we could assign them to three vars and use those... or anything else
without sacrificing any of both.


With kind regards,
Baurzhan.

[Harald Seiler]

Hello Jan,

On Wed, 2020-06-10 at 14:50 +0200, Jan Kiszka wrote:
> On 10.06.20 10:02, Harald Seiler wrote:
> > Extended partitions cannot be resized like primary partitions because
> > both the last logical partition and the EBR primary partition containing
> > it need to be expanded.
> >
> > To do this, add a second SED directive for removing the parition size of
> > the EBR partition. This part is detected by having either type `f` (Win
> > 95 Ext') or `5` (Extended).
> >
> > Signed-off-by: Harald Seiler <h...@denx.de>
> > ---
> >
> > Notes:
> > I have tested this in an ongoing project and it seems to work
> > reliably. For some reason WIC creates an extended partition of
> > type `f` but this can't be kept because while fdisk can read it,
> > it is only happy about writing the EBR with type `5`.
>
> Wikipedia says about 0x5:
>
> "Extended partition with CHS addressing. It must reside within the first
> physical 8 GB of disk, else use 0Fh instead"
>
> So we likely need to account for that case as well.
>

Hm, this is getting quite complicated ... And I suppose this is only one
of many corner cases. I'm not sure how much effort is worth here: This
change is only relevant to legacy systems that (have to) use MBR *and*
deal with more than 4 partitions which is most likely not too many.

What do you think? Is it likely that other projects would benefit from
a general solution or would it make more sense to keep this a) project
specific or b) detect one supported case and only do EBR expansion for
that one.

I don't think either option would make this prettier. I think the most
reasonable way forward is documenting what exactly is done with those
expressions.

> > sfdisk --force "${BOOT_DEV}"
> >
> > # Inform the kernel about the partitioning change
> >
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux
>

--
Harald

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de

[Jan Kiszka]

On 12.06.20 11:53, Harald Seiler wrote:
> Hello Jan,
>
> On Wed, 2020-06-10 at 14:50 +0200, Jan Kiszka wrote:
>> On 10.06.20 10:02, Harald Seiler wrote:
>>> Extended partitions cannot be resized like primary partitions because
>>> both the last logical partition and the EBR primary partition containing
>>> it need to be expanded.
>>>
>>> To do this, add a second SED directive for removing the parition size of
>>> the EBR partition. This part is detected by having either type `f` (Win
>>> 95 Ext') or `5` (Extended).
>>>
>>> Signed-off-by: Harald Seiler <h...@denx.de>
>>> ---
>>>
>>> Notes:
>>> I have tested this in an ongoing project and it seems to work
>>> reliably. For some reason WIC creates an extended partition of

That reason is "parted mkpart extended" which always creates a type-f
extended partition.

>>> type `f` but this can't be kept because while fdisk can read it,
>>> it is only happy about writing the EBR with type `5`.
>>
>> Wikipedia says about 0x5:
>>
>> "Extended partition with CHS addressing. It must reside within the first
>> physical 8 GB of disk, else use 0Fh instead"
>>
>> So we likely need to account for that case as well.
>>
>
> Hm, this is getting quite complicated ... And I suppose this is only one
> of many corner cases. I'm not sure how much effort is worth here: This
> change is only relevant to legacy systems that (have to) use MBR *and*
> deal with more than 4 partitions which is most likely not too many.
>
> What do you think? Is it likely that other projects would benefit from
> a general solution or would it make more sense to keep this a) project
> specific or b) detect one supported case and only do EBR expansion for
> that one.

How did you test this? I ran sfdisk on buster the way the script does,
just without type changing, and it happily created a type-f extended
partition AND expanded the last logical part inside that:

root@552c35371e6b:/# sfdisk -d disk | \
sed 's|^\(.*, \)size=[^,]*, \(type=[f5]\)$|\1\2|' | \
sed 's|^\(disk6 .*, \)size=[^,]*, |\1|' | \
sfdisk --force disk
Checking that no-one is using this disk right now ... OK

Disk disk: 4 GiB, 4294967296 bytes, 8388608 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: dos
Disk identifier: 0x7f8c0892

Old situation:

Device Boot Start End Sectors Size Id Type
disk1 2048 1953791 1951744 953M f W95 Ext'd (LBA)
disk5 4096 194559 190464 93M 83 Linux
disk6 196608 217087 20480 10M 83 Linux

>>> Script header accepted.
>>> Script header accepted.
>>> Script header accepted.
>>> Script header accepted.
>>> Created a new DOS disklabel with disk identifier 0x7f8c0892.
disk1: Created a new partition 1 of type 'W95 Ext'd (LBA)' and of size 4
GiB.
disk2: Created a new partition 5 of type 'Linux' and of size 93 MiB.
disk6: Created a new partition 6 of type 'Linux' and of size 3.9 GiB.
disk7: Done.

New situation:
Disklabel type: dos
Disk identifier: 0x7f8c0892

Device Boot Start End Sectors Size Id Type
disk1 2048 8388607 8386560 4G f W95 Ext'd (LBA)
disk5 4096 194559 190464 93M 83 Linux
disk6 196608 8388607 8192000 3.9G 83 Linux

The partition table has been altered.
Syncing disks.

[Harald Seiler]

Extended partitions cannot be resized like primary partitions because
both the last logical partition and the EBR primary partition containing
it need to be expanded.

To do this, add a second SED directive for removing the parition size of
the EBR partition. This part is detected by having either type `f` (Win
95 Ext') or `5` (Extended).

Signed-off-by: Harald Seiler <h...@denx.de>
---

Notes:

Changes in v2:
- Keep a type `f` EBR as type `f`.
- Update comment to better explain what is done.

I've built myself a little test-suite to see how this script
behaves when given different layouts. It seems to work just fine
now for the following cases:

- Expanding last partition of a GPT.
- Expanding last partition of an MBR without extended partitions.
- Expanding last (logical) partition of an MBR when the last
primary partition is the EBR.

Having the EBR somewhere in the middle is *not* supported. The
script will fail with an error message, without accidentally
ruining the layout. I think such a case is too rare to be worth
supporting.

.../files/expand-last-partition.sh | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh b/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
index 08c69db30529..7175dfd38b7e 100755
--- a/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
+++ b/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
@@ -17,12 +17,20 @@ if [ "${ROOT_DEV}" = "${BOOT_DEV}" ]; then

exit 1
fi

-LAST_PART="$(sfdisk -d ${BOOT_DEV} 2>/dev/null | tail -1 | cut -d ' ' -f 1)"
+LAST_PART="$(sfdisk -d "${BOOT_DEV}" 2>/dev/null | tail -1 | cut -d ' ' -f 1)"

-# Remove all hints to the current medium (last-lba) and last partition size,
-# then ask sfdisk to recreate the partitioning
-sfdisk -d "${BOOT_DEV}" 2>/dev/null | grep -v last-lba | \
- sed 's|\('"${LAST_PART}"' .*, \)size=[^,]*, |\1|' | \
+# Transform the partition table as follows:
+#
+# - Remove any 'last-lba' header so sfdisk uses the entire available space.
+# - If this partition table is MBR and an extended partition container (EBR)
+# exists, we assume this needs to be expanded as well; remove its size
+# field so sfdisk expands it.
+# - For the previously fetched last partition, also remove the size field so
+# sfdisk expands it.
+sfdisk -d "${BOOT_DEV}" 2>/dev/null | \
+ grep -v last-lba | \
+ sed 's|^\(.*, \)size=[^,]*, \(type=[f5]\)$|\1\2|' | \
+ sed 's|^\('"${LAST_PART}"' .*, \)size=[^,]*, |\1|' | \

sfdisk --force "${BOOT_DEV}"

# Inform the kernel about the partitioning change

--
2.25.4

[Jan Kiszka]

On 15.06.20 12:10, Harald Seiler wrote:
> Extended partitions cannot be resized like primary partitions because
> both the last logical partition and the EBR primary partition containing
> it need to be expanded.
>
> To do this, add a second SED directive for removing the parition size of
> the EBR partition. This part is detected by having either type `f` (Win
> 95 Ext') or `5` (Extended).
>
> Signed-off-by: Harald Seiler <h...@denx.de>
> ---
>
> Notes:
> Changes in v2:
> - Keep a type `f` EBR as type `f`.
> - Update comment to better explain what is done.
>
> I've built myself a little test-suite to see how this script
> behaves when given different layouts. It seems to work just fine
> now for the following cases:
>
> - Expanding last partition of a GPT.
> - Expanding last partition of an MBR without extended partitions.
> - Expanding last (logical) partition of an MBR when the last
> primary partition is the EBR.
>
> Having the EBR somewhere in the middle is *not* supported. The
> script will fail with an error message, without accidentally
> ruining the layout. I think such a case is too rare to be worth
> supporting.

Ack.

>
> .../files/expand-last-partition.sh | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh b/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
> index 08c69db30529..7175dfd38b7e 100755
> --- a/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
> +++ b/meta/recipes-support/expand-on-first-boot/files/expand-last-partition.sh
> @@ -17,12 +17,20 @@ if [ "${ROOT_DEV}" = "${BOOT_DEV}" ]; then
> exit 1
> fi
>
> -LAST_PART="$(sfdisk -d ${BOOT_DEV} 2>/dev/null | tail -1 | cut -d ' ' -f 1)"
> +LAST_PART="$(sfdisk -d "${BOOT_DEV}" 2>/dev/null | tail -1 | cut -d ' ' -f 1)"
>
> -# Remove all hints to the current medium (last-lba) and last partition size,
> -# then ask sfdisk to recreate the partitioning
> -sfdisk -d "${BOOT_DEV}" 2>/dev/null | grep -v last-lba | \
> - sed 's|\('"${LAST_PART}"' .*, \)size=[^,]*, |\1|' | \
> +# Transform the partition table as follows:
> +#
> +# - Remove any 'last-lba' header so sfdisk uses the entire available space.
> +# - If this partition table is MBR and an extended partition container (EBR)
> +# exists, we assume this needs to be expanded as well; remove its size
> +# field so sfdisk expands it.
> +# - For the previously fetched last partition, also remove the size field so
> +# sfdisk expands it.

Very nice.

> +sfdisk -d "${BOOT_DEV}" 2>/dev/null | \
> + grep -v last-lba | \
> + sed 's|^\(.*, \)size=[^,]*, \(type=[f5]\)$|\1\2|' | \
> + sed 's|^\('"${LAST_PART}"' .*, \)size=[^,]*, |\1|' | \
> sfdisk --force "${BOOT_DEV}"
>
> # Inform the kernel about the partitioning change
>

LGTM.

[Jan Kiszka]

Baurzhan, this is overdue.

Jan

--
Siemens AG, T RDA IOT

[Anton Mikanovich]

15.06.2020 13:10, Harald Seiler wrote:
> Extended partitions cannot be resized like primary partitions because
> both the last logical partition and the EBR primary partition containing
> it need to be expanded.
>
> To do this, add a second SED directive for removing the parition size of
> the EBR partition. This part is detected by having either type `f` (Win
> 95 Ext') or `5` (Extended).
>
> Signed-off-by: Harald Seiler <h...@denx.de>

Applied to next, thanks.

--
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov