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

[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 <hws@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

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

[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 <hws@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

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

[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.

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

[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 <hws@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.

> >     
> >     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?
> 

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: hws@denx.de

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

[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 <hws@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.


Jan

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