| Message ID | 20221108112837.435213-2-tobias.schaffner@siemens.com |
|---|---|
| State | Under Review |
| Headers | show |
| Series | expand-on-first-boot: try fs resize on expanded partitions | expand |
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.
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. >
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. > >
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.