diff mbox series

[v5,2/6] expand-on-first-boot: support resizing a btrfs

Message ID 20221215142759.26327-3-henning.schild@siemens.com
State Superseded, archived
Headers show
Series [v5,1/6] expand-on-first-boot: really only do that once, especially on failure | expand

Commit Message

Henning Schild Dec. 15, 2022, 2:27 p.m. UTC 
From: Henning Schild <henning.schild@siemens.com>

This adds support for resizing btrfs filesystems if they are in that
last partition. It also prepares for potentially other filesystems to
come in the future by introducing a switch-case.

The mounting logic is taken from the systemd-growfs patches we had to
revert again. Some filesystems need to online for resizing, but in order
to find the filesystem of a partition (without udev) mounting it and
letting the kernel detect seems a good idea.

Suggested-by: Roberto A. Foglietta <roberto.foglietta@gmail.com>
Suggested-by: Joe MacDonald <joe_macdonald@mentor.com>
Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 ...oot_1.3.bb => expand-on-first-boot_1.4.bb} |  0
 .../files/expand-last-partition.sh            | 35 ++++++++++++++++---
 2 files changed, 31 insertions(+), 4 deletions(-)
 rename meta/recipes-support/expand-on-first-boot/{expand-on-first-boot_1.3.bb => expand-on-first-boot_1.4.bb} (100%)

Comments

Roberto A. Foglietta Dec. 15, 2022, 5:48 p.m. UTC | #1 
On Thu, 15 Dec 2022 at 15:28, <henning.schild@siemens.com> wrote:

> +MOUNT_POINT=$(mktemp -d --tmpdir "$(basename "$0").XXXXXXXXXX")

Good choice to use the long option --tmpdir but it is still less
readable than -p "$TMPDIR"

> +if [ ! -d "${MOUNT_POINT}" ]; then
> +       echo "Cannot create temporary mount point ${MOUNT_POINT}." >&2
> +       exit 1
> +fi

This is useless because set -e detected the failure also in child shells.

Just try these two commands:

roberto$ set -e
roberto$ test=$(false)

The terminal / shell will be closed immediately after pressing ENTER

> +mount "${LAST_PART}" "${MOUNT_POINT}"

If mktemp fails then also the mount will fail so whatever the shell
takes in account or not the subshell errors it is useless explicitly
check the mount point existence

> +
> +ret=0
> +# Determine the filesystem type and perform the appropriate resize function
> +FS_TYPE=$(findmnt -fno FSTYPE "${MOUNT_POINT}" )
> +case ${FS_TYPE} in
> +ext*)

Using ext*) seems legit but what about the introduction of extmyfs? A
completely different filesystem from ext[234]? I suggest ext[234]),
instead.

> +       # Do not fail resize2fs if no mtab entry is found, e.g.,
> +       # when using systemd mount units.
> +       export EXT2FS_NO_MTAB_OK=1
> +       resize2fs "${LAST_PART}"
> +       ;;
> +btrfs)
> +       btrfs filesystem resize max "${MOUNT_POINT}"Using ext*) seems legit but what about the introduction of extmyfs? A completely different filesystem from ext[234]? I suggest ext[234]), instead.
> +       ;;
> +*)
> +       echo "Unrecognized filesystem type ${FS_TYPE} - no resize performed"
> +       ret=1

Looking at your other patches this script has been added by default in
many templates. So, why consider a failure the execution of something
that the user might not even choose to use but just accepted by
default? Would it not be better to have a simple warning and nothing
else? Moreover, if the failure of this script does not deactivate its
service [as I hope] this script will run at every boot without any
reason. Adding an exit 0 here will also remove the use of ret variable
simplifying the code.

> +       ;;
> +esac
>
> -resize2fs "${LAST_PART}"
> +umount "${MOUNT_POINT}"
> +rmdir "${MOUNT_POINT}"
> +exit $ret
> --
> 2.37.4
>

Best regards, R-
diff mbox series

Patch

diff --git a/meta/recipes-support/expand-on-first-boot/expand-on-first-boot_1.3.bb b/meta/recipes-support/expand-on-first-boot/expand-on-first-boot_1.4.bb
similarity index 100%
rename from meta/recipes-support/expand-on-first-boot/expand-on-first-boot_1.3.bb
rename to meta/recipes-support/expand-on-first-boot/expand-on-first-boot_1.4.bb
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 93eddda2a3b7..753b8c33399c 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
@@ -62,8 +62,35 @@  if grep -q x-systemd.growfs /etc/fstab; then
 	exit 0
 fi
 
-# Do not fail resize2fs if no mtab entry is found, e.g.,
-# when using systemd mount units.
-export EXT2FS_NO_MTAB_OK=1
+# some filesystems need to be mounted i.e. btrfs, but mounting also helps
+# detect the filesystem type without having to wait for udev
+# mount $LAST_PART out of tree, so we won't conflict with other mounts
+MOUNT_POINT=$(mktemp -d --tmpdir "$(basename "$0").XXXXXXXXXX")
+if [ ! -d "${MOUNT_POINT}" ]; then
+	echo "Cannot create temporary mount point ${MOUNT_POINT}." >&2
+	exit 1
+fi
+mount "${LAST_PART}" "${MOUNT_POINT}"
+
+ret=0
+# Determine the filesystem type and perform the appropriate resize function
+FS_TYPE=$(findmnt -fno FSTYPE "${MOUNT_POINT}" )
+case ${FS_TYPE} in
+ext*)
+	# Do not fail resize2fs if no mtab entry is found, e.g.,
+	# when using systemd mount units.
+	export EXT2FS_NO_MTAB_OK=1
+	resize2fs "${LAST_PART}"
+	;;
+btrfs)
+	btrfs filesystem resize max "${MOUNT_POINT}"
+	;;
+*)
+	echo "Unrecognized filesystem type ${FS_TYPE} - no resize performed"
+	ret=1
+	;;
+esac
 
-resize2fs "${LAST_PART}"
+umount "${MOUNT_POINT}"
+rmdir "${MOUNT_POINT}"
+exit $ret