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