public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
* [PATCH 0/2] expand-on-first-boot: try fs resize on expanded partitions
@ 2022-11-08 11:28 T. Schaffner
  2022-11-08 11:28 ` [PATCH 1/2] Check if last partition ends at GPT backup header T. Schaffner
  2022-11-08 11:28 ` [PATCH 2/2] Always try resizing the fs in expand on first boot T. Schaffner
  0 siblings, 2 replies; 8+ messages in thread
From: T. Schaffner @ 2022-11-08 11:28 UTC (permalink / raw)
  To: isar-users; +Cc: henning.schild, Tobias Schaffner

From: Tobias Schaffner <tobias.schaffner@siemens.com>

If the filesystem resize on first boot fails or gets interrupted the service
will not be disabled. But when the script is called on the next boot it will
not try to expand the filesystem as the partition is already expanded.

Check if the partition has to be expanded before expanding it but always try
to resize the filesystem. Leave the decision if the filesystem has to be resized
to the used tool.

Improve the check if we have to expand the partition by calculating the last
usabe block in the GUID table.

Tobias Schaffner (2):
  Check if last partition ends at GPT backup header
  Always try resizing the fs in expand on first boot

 .../files/expand-last-partition.sh            | 54 +++++++++----------
 1 file changed, 25 insertions(+), 29 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] Check if last partition ends at GPT backup header
  2022-11-08 11:28 [PATCH 0/2] expand-on-first-boot: try fs resize on expanded partitions T. Schaffner
@ 2022-11-08 11:28 ` T. Schaffner
  2022-11-09  8:26   ` Henning Schild
  2022-11-08 11:28 ` [PATCH 2/2] Always try resizing the fs in expand on first boot T. Schaffner
  1 sibling, 1 reply; 8+ messages in thread
From: T. Schaffner @ 2022-11-08 11:28 UTC (permalink / raw)
  To: isar-users; +Cc: henning.schild, Tobias Schaffner

From: Tobias Schaffner <tobias.schaffner@siemens.com>

The GPT backup header has a fixed size of 33 512 byte blocks. Check if the last
partition ends at the block before the start of the GUID partition table backup
header. If so the partition is fully expanded.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 .../files/expand-last-partition.sh            | 21 +++++++++----------
 1 file changed, 10 insertions(+), 11 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 57055cc..0d662cc 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
@@ -22,24 +22,23 @@ if [ "${ROOT_DEV}" = "${BOOT_DEV}" ]; then
 	exit 1
 fi
 
-# this value is in blocks. Normally a block has 512 bytes.
-BUFFER_SIZE=32768
 BOOT_DEV_NAME=${BOOT_DEV##*/}
+LAST_PART_NAME="$(lsblk -l -o NAME "${BOOT_DEV}" | tail -1)"
+LAST_PART="/dev/${LAST_PART_NAME}"
+
 DISK_SIZE="$(cat /sys/class/block/"${BOOT_DEV_NAME}"/size)"
-ALL_PARTS_SIZE=0
-for PARTITION in /sys/class/block/"${BOOT_DEV_NAME}"/"${BOOT_DEV_NAME}"*; do
-	PART_SIZE=$(cat "${PARTITION}"/size)
-	ALL_PARTS_SIZE=$((ALL_PARTS_SIZE + PART_SIZE))
-done
+LAST_PART_SIZE="$(cat /sys/class/block/"${LAST_PART_NAME}"/size)"
+LAST_PART_START="$(cat /sys/class/block/"${LAST_PART_NAME}"/start)"
+
+# The GUID partition table stores its backup in the last 33 blocks of the table.
+# Therefore the last partition ends 33 before the end of the disk if expanded.
+GPT_BACKUP_SIZE=33
 
-MINIMAL_SIZE=$((ALL_PARTS_SIZE + BUFFER_SIZE))
-if [ "$DISK_SIZE" -lt "$MINIMAL_SIZE" ]; then
+if [ $((LAST_PART_START + LAST_PART_SIZE + GPT_BACKUP_SIZE)) -lt "${DISK_SIZE}" ]; then
 	echo "Disk is practically already full, doing nothing." >&2
 	exit 0
 fi
 
-LAST_PART="$(sfdisk -d "${BOOT_DEV}" 2>/dev/null | tail -1 | cut -d ' ' -f 1)"
-
 # Transform the partition table as follows:
 #
 # - Remove any 'last-lba' header so sfdisk uses the entire available space.
-- 
2.34.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] Always try resizing the fs in expand on first boot
  2022-11-08 11:28 [PATCH 0/2] expand-on-first-boot: try fs resize on expanded partitions T. Schaffner
  2022-11-08 11:28 ` [PATCH 1/2] Check if last partition ends at GPT backup header T. Schaffner
@ 2022-11-08 11:28 ` T. Schaffner
  2022-11-09  8:52   ` Henning Schild
  1 sibling, 1 reply; 8+ messages in thread
From: T. Schaffner @ 2022-11-08 11:28 UTC (permalink / raw)
  To: isar-users; +Cc: henning.schild, Tobias Schaffner

From: Tobias Schaffner <tobias.schaffner@siemens.com>

If the filesystem resize fails or gets interrupted we have no way to recover
from this as the script always exits if the partition was already resized.

Check if we have to resize the partition but alway run the chosen fs resize
tool. Leave the decision if the filesystem has to be resized to resize2fs /
systemd-growfs.
If the filesystem was already expanded the resize2fs / systemd-growfs call is a
noop.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 .../files/expand-last-partition.sh            | 35 +++++++++----------
 1 file changed, 16 insertions(+), 19 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 0d662cc..b21b958 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
@@ -35,26 +35,23 @@ LAST_PART_START="$(cat /sys/class/block/"${LAST_PART_NAME}"/start)"
 GPT_BACKUP_SIZE=33
 
 if [ $((LAST_PART_START + LAST_PART_SIZE + GPT_BACKUP_SIZE)) -lt "${DISK_SIZE}" ]; then
-	echo "Disk is practically already full, doing nothing." >&2
-	exit 0
-fi
+	# 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}"
 
-# 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
-partx -u "${LAST_PART}"
+	# Inform the kernel about the partitioning change
+	partx -u "${LAST_PART}"
+fi
 
 # this is for debian stretch or systemd < 236
 if [ ! -x /lib/systemd/systemd-growfs ]; then
-- 
2.34.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] Check if last partition ends at GPT backup header
  2022-11-08 11:28 ` [PATCH 1/2] Check if last partition ends at GPT backup header T. Schaffner
@ 2022-11-09  8:26   ` Henning Schild
  2022-11-09  9:40     ` Schaffner, Tobias
  0 siblings, 1 reply; 8+ messages in thread
From: Henning Schild @ 2022-11-09  8:26 UTC (permalink / raw)
  To: T. Schaffner; +Cc: isar-users

Am Tue, 8 Nov 2022 12:28:36 +0100
schrieb "T. Schaffner" <tobias.schaffner@siemens.com>:

> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> The GPT backup header has a fixed size of 33 512 byte blocks. Check
> if the last partition ends at the block before the start of the GUID
> partition table backup header. If so the partition is fully expanded.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  .../files/expand-last-partition.sh            | 21
> +++++++++---------- 1 file changed, 10 insertions(+), 11 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 57055cc..0d662cc 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
> @@ -22,24 +22,23 @@ if [ "${ROOT_DEV}" = "${BOOT_DEV}" ]; then exit 1
> fi -# this value is in blocks. Normally a block has 512 bytes.
> -BUFFER_SIZE=32768
>  BOOT_DEV_NAME=${BOOT_DEV##*/}
> +LAST_PART_NAME="$(lsblk -l -o NAME "${BOOT_DEV}" | tail -1)"
> +LAST_PART="/dev/${LAST_PART_NAME}"
> +
>  DISK_SIZE="$(cat /sys/class/block/"${BOOT_DEV_NAME}"/size)"
> -ALL_PARTS_SIZE=0
> -for PARTITION in
> /sys/class/block/"${BOOT_DEV_NAME}"/"${BOOT_DEV_NAME}"*; do
> -	PART_SIZE=$(cat "${PARTITION}"/size)
> -	ALL_PARTS_SIZE=$((ALL_PARTS_SIZE + PART_SIZE))
> -done
> +LAST_PART_SIZE="$(cat /sys/class/block/"${LAST_PART_NAME}"/size)"
> +LAST_PART_START="$(cat /sys/class/block/"${LAST_PART_NAME}"/start)"
> +
> +# The GUID partition table stores its backup in the last 33 blocks
> of the table. +# Therefore the last partition ends 33 before the end
> of the disk if expanded. +GPT_BACKUP_SIZE=33
>  
> -MINIMAL_SIZE=$((ALL_PARTS_SIZE + BUFFER_SIZE))
> -if [ "$DISK_SIZE" -lt "$MINIMAL_SIZE" ]; then
> +if [ $((LAST_PART_START + LAST_PART_SIZE + GPT_BACKUP_SIZE)) -lt
> "${DISK_SIZE}" ]; then echo "Disk is practically already full, doing
> nothing." >&2 exit 0

There was a good reason for that. I think when for alignment reasons or
so it looks like there is a little bit of space left. But the resize
would then not work or we would in fact try a shrink and fail.
Here you seem the switch the logic from "practically full" to "really
full" and remove that margin where a resize would be tried but would
fail.

The trick with last_part start + size looks nicer than summing up the
whole size.

Henning

>  fi
>  
> -LAST_PART="$(sfdisk -d "${BOOT_DEV}" 2>/dev/null | tail -1 | cut -d
> ' ' -f 1)" -
>  # Transform the partition table as follows:
>  #
>  # - Remove any 'last-lba' header so sfdisk uses the entire available
> space.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] Always try resizing the fs in expand on first boot
  2022-11-08 11:28 ` [PATCH 2/2] Always try resizing the fs in expand on first boot T. Schaffner
@ 2022-11-09  8:52   ` Henning Schild
  2022-11-09 10:11     ` Schaffner, Tobias
  0 siblings, 1 reply; 8+ messages in thread
From: Henning Schild @ 2022-11-09  8:52 UTC (permalink / raw)
  To: T. Schaffner; +Cc: isar-users

Hi,

if the scripts runs into a problem that is likely severe and should not
have happened in the first place. Making it more robust and so that it
can be run multiple times still does not hurt.

But really what one would need to do is make a copy of the original
partition table and restore that when the fs resize fails, in some exit
hook.

What you are doing here would only skip the partition resize on a
second call and maybe run into the same error again later on. Whatever
that error might have been. So we better understand the nature of such
errors instead of making that script so that we try over and over ...
and maybe always fail.

In fact the longer term plan was to move this to use systemds
first-boot semantics instead of that self-disabling trick.

Henning

Am Tue, 8 Nov 2022 12:28:37 +0100
schrieb "T. Schaffner" <tobias.schaffner@siemens.com>:

> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> If the filesystem resize fails or gets interrupted we have no way to
> recover from this as the script always exits if the partition was
> already resized.
> 
> Check if we have to resize the partition but alway run the chosen fs
> resize tool. Leave the decision if the filesystem has to be resized
> to resize2fs / systemd-growfs.
> If the filesystem was already expanded the resize2fs / systemd-growfs
> call is a noop.
> 
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> ---
>  .../files/expand-last-partition.sh            | 35
> +++++++++---------- 1 file changed, 16 insertions(+), 19 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 0d662cc..b21b958 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
> @@ -35,26 +35,23 @@ LAST_PART_START="$(cat
> /sys/class/block/"${LAST_PART_NAME}"/start)" GPT_BACKUP_SIZE=33 if [
> $((LAST_PART_START + LAST_PART_SIZE + GPT_BACKUP_SIZE)) -lt
> "${DISK_SIZE}" ]; then
> -	echo "Disk is practically already full, doing nothing." >&2
> -	exit 0
> -fi
> +	# 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}"
>  
> -# 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
> -partx -u "${LAST_PART}"
> +	# Inform the kernel about the partitioning change
> +	partx -u "${LAST_PART}"
> +fi
>  
>  # this is for debian stretch or systemd < 236
>  if [ ! -x /lib/systemd/systemd-growfs ]; then


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] Check if last partition ends at GPT backup header
  2022-11-09  8:26   ` Henning Schild
@ 2022-11-09  9:40     ` Schaffner, Tobias
  2022-11-09 14:54       ` Henning Schild
  0 siblings, 1 reply; 8+ messages in thread
From: Schaffner, Tobias @ 2022-11-09  9:40 UTC (permalink / raw)
  To: Schild, Henning; +Cc: isar-users

On 09.11.22 09:26, Schild, Henning (T CED SES-DE) wrote:
> Am Tue, 8 Nov 2022 12:28:36 +0100
> schrieb "T. Schaffner" <tobias.schaffner@siemens.com>:
> 
>> From: Tobias Schaffner <tobias.schaffner@siemens.com>
>>
>> The GPT backup header has a fixed size of 33 512 byte blocks. Check
>> if the last partition ends at the block before the start of the GUID
>> partition table backup header. If so the partition is fully expanded.
>>
>> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
>> ---
>>   .../files/expand-last-partition.sh            | 21
>> +++++++++---------- 1 file changed, 10 insertions(+), 11 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 57055cc..0d662cc 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
>> @@ -22,24 +22,23 @@ if [ "${ROOT_DEV}" = "${BOOT_DEV}" ]; then exit 1
>> fi -# this value is in blocks. Normally a block has 512 bytes.
>> -BUFFER_SIZE=32768
>>   BOOT_DEV_NAME=${BOOT_DEV##*/}
>> +LAST_PART_NAME="$(lsblk -l -o NAME "${BOOT_DEV}" | tail -1)"
>> +LAST_PART="/dev/${LAST_PART_NAME}"
>> +
>>   DISK_SIZE="$(cat /sys/class/block/"${BOOT_DEV_NAME}"/size)"
>> -ALL_PARTS_SIZE=0
>> -for PARTITION in
>> /sys/class/block/"${BOOT_DEV_NAME}"/"${BOOT_DEV_NAME}"*; do
>> -	PART_SIZE=$(cat "${PARTITION}"/size)
>> -	ALL_PARTS_SIZE=$((ALL_PARTS_SIZE + PART_SIZE))
>> -done
>> +LAST_PART_SIZE="$(cat /sys/class/block/"${LAST_PART_NAME}"/size)"
>> +LAST_PART_START="$(cat /sys/class/block/"${LAST_PART_NAME}"/start)"
>> +
>> +# The GUID partition table stores its backup in the last 33 blocks
>> of the table. +# Therefore the last partition ends 33 before the end
>> of the disk if expanded. +GPT_BACKUP_SIZE=33
>>   
>> -MINIMAL_SIZE=$((ALL_PARTS_SIZE + BUFFER_SIZE))
>> -if [ "$DISK_SIZE" -lt "$MINIMAL_SIZE" ]; then
>> +if [ $((LAST_PART_START + LAST_PART_SIZE + GPT_BACKUP_SIZE)) -lt
>> "${DISK_SIZE}" ]; then echo "Disk is practically already full, doing
>> nothing." >&2 exit 0
> 
> There was a good reason for that. I think when for alignment reasons or
> so it looks like there is a little bit of space left. But the resize
> would then not work or we would in fact try a shrink and fail.
> Here you seem the switch the logic from "practically full" to "really
> full" and remove that margin where a resize would be tried but would
> fail.
> 
> The trick with last_part start + size looks nicer than summing up the
> whole size.
> 
> Henning

Any ideas how I could reproduce such a case?

I have not seen any partition alignment in my tests so far. The only 
thing that has to be respected is the GUID backup header in the end of 
the partition.

What I have seen is a alignment of the filesystem inside of the 
partition. Ext4 is aligned to a minimum of 4096 bytes / 8 sysfs blocks. 
But this is not relevant in the check if the partition was expanded.


All the best

>>   fi
>>   
>> -LAST_PART="$(sfdisk -d "${BOOT_DEV}" 2>/dev/null | tail -1 | cut -d
>> ' ' -f 1)" -
>>   # Transform the partition table as follows:
>>   #
>>   # - Remove any 'last-lba' header so sfdisk uses the entire available
>> space.
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] Always try resizing the fs in expand on first boot
  2022-11-09  8:52   ` Henning Schild
@ 2022-11-09 10:11     ` Schaffner, Tobias
  0 siblings, 0 replies; 8+ messages in thread
From: Schaffner, Tobias @ 2022-11-09 10:11 UTC (permalink / raw)
  To: Schild, Henning; +Cc: isar-users

On 09.11.22 09:52, Schild, Henning (T CED SES-DE) wrote:
> Hi,
> 
> if the scripts runs into a problem that is likely severe and should not
> have happened in the first place. Making it more robust and so that it
> can be run multiple times still does not hurt.
> 
> But really what one would need to do is make a copy of the original
> partition table and restore that when the fs resize fails, in some exit
> hook.
> 
> What you are doing here would only skip the partition resize on a
> second call and maybe run into the same error again later on. Whatever
> that error might have been. So we better understand the nature of such
> errors instead of making that script so that we try over and over ...
> and maybe always fail.
> 
> In fact the longer term plan was to move this to use systemds
> first-boot semantics instead of that self-disabling trick.
> 
> Henning

I like the idea of restoring the partition table after a failed resize. 
That covers the usecase that sfdisk did something wrong which I have not 
considered yet.

What this patch tried to fix in the first place where these two scenarios:
* The script gets interrupted for e.g. because of power cycle after 
repartitioning.
* The uevent after the partx -u takes longer than expected and the 
resize resizes to the old partition size or fails because of that.

Rare cases but may happen with many devices in the field.

> Am Tue, 8 Nov 2022 12:28:37 +0100
> schrieb "T. Schaffner" <tobias.schaffner@siemens.com>:
> 
>> From: Tobias Schaffner <tobias.schaffner@siemens.com>
>>
>> If the filesystem resize fails or gets interrupted we have no way to
>> recover from this as the script always exits if the partition was
>> already resized.
>>
>> Check if we have to resize the partition but alway run the chosen fs
>> resize tool. Leave the decision if the filesystem has to be resized
>> to resize2fs / systemd-growfs.
>> If the filesystem was already expanded the resize2fs / systemd-growfs
>> call is a noop.
>>
>> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
>> ---
>>   .../files/expand-last-partition.sh            | 35
>> +++++++++---------- 1 file changed, 16 insertions(+), 19 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 0d662cc..b21b958 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
>> @@ -35,26 +35,23 @@ LAST_PART_START="$(cat
>> /sys/class/block/"${LAST_PART_NAME}"/start)" GPT_BACKUP_SIZE=33 if [
>> $((LAST_PART_START + LAST_PART_SIZE + GPT_BACKUP_SIZE)) -lt
>> "${DISK_SIZE}" ]; then
>> -	echo "Disk is practically already full, doing nothing." >&2
>> -	exit 0
>> -fi
>> +	# 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}"
>>   
>> -# 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
>> -partx -u "${LAST_PART}"
>> +	# Inform the kernel about the partitioning change
>> +	partx -u "${LAST_PART}"
>> +fi
>>   
>>   # this is for debian stretch or systemd < 236
>>   if [ ! -x /lib/systemd/systemd-growfs ]; then
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] Check if last partition ends at GPT backup header
  2022-11-09  9:40     ` Schaffner, Tobias
@ 2022-11-09 14:54       ` Henning Schild
  0 siblings, 0 replies; 8+ messages in thread
From: Henning Schild @ 2022-11-09 14:54 UTC (permalink / raw)
  To: Schaffner, Tobias (T CED SES-DE); +Cc: isar-users, Tobias Schmidl

Am Wed, 9 Nov 2022 10:40:20 +0100
schrieb "Schaffner, Tobias (T CED SES-DE)"
<tobias.schaffner@siemens.com>:

> On 09.11.22 09:26, Schild, Henning (T CED SES-DE) wrote:
> > Am Tue, 8 Nov 2022 12:28:36 +0100
> > schrieb "T. Schaffner" <tobias.schaffner@siemens.com>:
> >   
> >> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> >>
> >> The GPT backup header has a fixed size of 33 512 byte blocks. Check
> >> if the last partition ends at the block before the start of the
> >> GUID partition table backup header. If so the partition is fully
> >> expanded.
> >>
> >> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> >> ---
> >>   .../files/expand-last-partition.sh            | 21
> >> +++++++++---------- 1 file changed, 10 insertions(+), 11
> >> 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 57055cc..0d662cc 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
> >> @@ -22,24 +22,23 @@ if [ "${ROOT_DEV}" = "${BOOT_DEV}" ]; then
> >> exit 1 fi -# this value is in blocks. Normally a block has 512
> >> bytes. -BUFFER_SIZE=32768
> >>   BOOT_DEV_NAME=${BOOT_DEV##*/}
> >> +LAST_PART_NAME="$(lsblk -l -o NAME "${BOOT_DEV}" | tail -1)"
> >> +LAST_PART="/dev/${LAST_PART_NAME}"
> >> +
> >>   DISK_SIZE="$(cat /sys/class/block/"${BOOT_DEV_NAME}"/size)"
> >> -ALL_PARTS_SIZE=0
> >> -for PARTITION in
> >> /sys/class/block/"${BOOT_DEV_NAME}"/"${BOOT_DEV_NAME}"*; do
> >> -	PART_SIZE=$(cat "${PARTITION}"/size)
> >> -	ALL_PARTS_SIZE=$((ALL_PARTS_SIZE + PART_SIZE))
> >> -done
> >> +LAST_PART_SIZE="$(cat /sys/class/block/"${LAST_PART_NAME}"/size)"
> >> +LAST_PART_START="$(cat
> >> /sys/class/block/"${LAST_PART_NAME}"/start)" +
> >> +# The GUID partition table stores its backup in the last 33 blocks
> >> of the table. +# Therefore the last partition ends 33 before the
> >> end of the disk if expanded. +GPT_BACKUP_SIZE=33
> >>   
> >> -MINIMAL_SIZE=$((ALL_PARTS_SIZE + BUFFER_SIZE))
> >> -if [ "$DISK_SIZE" -lt "$MINIMAL_SIZE" ]; then
> >> +if [ $((LAST_PART_START + LAST_PART_SIZE + GPT_BACKUP_SIZE)) -lt
> >> "${DISK_SIZE}" ]; then echo "Disk is practically already full,
> >> doing nothing." >&2 exit 0  
> > 
> > There was a good reason for that. I think when for alignment
> > reasons or so it looks like there is a little bit of space left.
> > But the resize would then not work or we would in fact try a shrink
> > and fail. Here you seem the switch the logic from "practically
> > full" to "really full" and remove that margin where a resize would
> > be tried but would fail.
> > 
> > The trick with last_part start + size looks nicer than summing up
> > the whole size.
> > 
> > Henning  
> 
> Any ideas how I could reproduce such a case?
> 
> I have not seen any partition alignment in my tests so far. The only 
> thing that has to be respected is the GUID backup header in the end
> of the partition.
> 
> What I have seen is a alignment of the filesystem inside of the 
> partition. Ext4 is aligned to a minimum of 4096 bytes / 8 sysfs
> blocks. But this is not relevant in the check if the partition was
> expanded.

I think the repro was with an example image booted in qemu without
growing that image with qemu-img convert. In which case the partition
table resize was a noop but the fs resize wanted to shrink for alignment
reasons. And that failed because the fs was just very full. Might be
hard to repro but also not too smart to basically revert the patch that
introduced that "close to full ... better not touch".

Likely not all versions/distros affected in the same way. And it was
with resize2fs, not sure how systemd would react but i assume it will
in effect also call resize2fs at the end of the day.

https://github.com/ilbers/isar/commit/bd9bd34c978829ffc13d61c36af90363030c2349

The commit does not explain in detail which combination was "broken".
May be spread somewhere in the discussions that have been going on
around that.

I took Tobias Schmidl on Cc, maybe he will answer. But we might be on
our own.

I have sent a patch to CI test expand-on-first-boot. While that still
needs a rewrite, it can already be used to test this series. Maybe you
find your repro that way.

Henning

> 
> All the best
> 
> >>   fi
> >>   
> >> -LAST_PART="$(sfdisk -d "${BOOT_DEV}" 2>/dev/null | tail -1 | cut
> >> -d ' ' -f 1)" -
> >>   # Transform the partition table as follows:
> >>   #
> >>   # - Remove any 'last-lba' header so sfdisk uses the entire
> >> available space.  
> >   


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-11-09 14:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-08 11:28 [PATCH 0/2] expand-on-first-boot: try fs resize on expanded partitions T. Schaffner
2022-11-08 11:28 ` [PATCH 1/2] Check if last partition ends at GPT backup header T. Schaffner
2022-11-09  8:26   ` Henning Schild
2022-11-09  9:40     ` Schaffner, Tobias
2022-11-09 14:54       ` Henning Schild
2022-11-08 11:28 ` [PATCH 2/2] Always try resizing the fs in expand on first boot T. Schaffner
2022-11-09  8:52   ` Henning Schild
2022-11-09 10:11     ` Schaffner, Tobias

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox