Re: [PATCH] expand-on-first-boot: wait for udev to create symlink

[Henning Schild <henning.schild@siemens.com>]

Am Thu, 8 Dec 2022 20:18:28 +0100
schrieb "Schaffner, Tobias (T CED SES-DE)"
<tobias.schaffner@siemens.com>:

> On 08.12.22 18:39, Schild, Henning (T CED SES-DE) wrote:
> > Am Thu, 8 Dec 2022 17:55:42 +0100
> > schrieb "T. Schaffner" <tobias.schaffner@siemens.com>:
> >   
> >> From: Tobias Schaffner <tobias.schaffner@siemens.com>
> >>
> >> systemd-growfs depends on a symlink to the partition of the
> >> filesystem that should be resized. This symlink is created by udev
> >> in /dev/block/.
> >>
> >> If this symlink is not yet created for example because
> >> systemd-udev is not up yet systemd-growfs will fail.
> >>
> >> We could use Require and After to depend on the systemd-udev
> >> service but this could again create a race condition if udev is up
> >> but not fast enough after the partx -u.
> >>
> >> Resolve the symlinks in /dev/block/ periodically until the symlink
> >> appears before running systemd-growfs.
> >> ---
> >>   .../files/expand-last-partition.sh               | 16
> >> +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
> >>
> >> 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..7ebb3e5 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
> >> @@ -38,7 +38,8 @@ if [ "$DISK_SIZE" -lt "$MINIMAL_SIZE" ]; then
> >> exit 0 fi -LAST_PART="$(sfdisk -d "${BOOT_DEV}" 2>/dev/null | tail
> >> -1 | cut -d ' ' -f 1)" +LAST_PART_NAME="$(lsblk -l -o NAME
> >> "${BOOT_DEV}" | tail -1)" +LAST_PART="/dev/${LAST_PART_NAME}"
> >>   
> >>   # Transform the partition table as follows:
> >>   #
> >> @@ -79,6 +80,19 @@ if [ ! -d "${MOUNT_POINT}" ]; then
> >>   	exit 1
> >>   fi
> >>   
> >> +START_TIME="$(date +%s)"
> >> +
> >> +# Wait for udev to create the symlink to the partition in  
> > 
> > Newer systemd versions will not need that anymore. Maybe write down
> > which version that would be and starting from which debian distro we
> > can drop that. Who knows when that udev stuff changes and the
> > symlinks will never appear ... We should only wait for them in
> > systemd versions that use them.
> > 
> > If we already have a version out there ... like bookworm? We can
> > already implement it without the wait.  
> 
> Yes you are right. I will add a check and skip the loop from V252 on.
> 
> >> /dev/block/ as +# systemd-growfs depends on it
> >> +while ! readlink /dev/block/* | grep -q "${LAST_PART_NAME}"; do  
> > 
> > I would make that much stricter to not be tricked by partial matches
> > 
> > sda4 vs sda42
> > 
> > readlink -f /dev/block/* | grep -e -q "^${LAST_PART}$"  
> 
> I get a relative path from readlink. So I will have to skip the ^ but
> to use match the end of the line is a good idea. -e is not needed for
> start and end line matches. ;)
> 
> >> +    sleep 0.1  
> > 
> > I wonder if there is anything we could do here. Maybe "udevadm
> > trigger" and depend on udev after all. Because the 5 is a nasty
> > guess ...  
> 
> 
> Nothing that I am aware of. The main reason for the issues that we
> have seen was that the udev service was not up yet. We could maybe
> check if it is up first and after that only allow a small period. But
> then again how long do we wait for the service to be up and running.
> 
> >> +    CURRENT_TIME="$(date +%s)"
> >> +    if [ $(( CURRENT_TIME - START_TIME )) -gt "5" ]; then  
> > 
> > we can simply i++ > 50, no need to call date
> > 
> > so maybe
> > 
> > err=1
> > for i in $(seq 0 50); do
> > 	if readlink -f /dev/block/* | grep -e -q "^${LAST_PART}$";
> > then err=0
> > 		break
> > 	fi
> > 	sleep 0.1
> > done
> > 
> > if $err ...  
> >> +        echo "Could not find symlink to last part in /dev/block/."
> >> +        exit 1
> >> +    fi
> >> +done
> >> +
> >>   mount "${LAST_PART}" "${MOUNT_POINT}"
> >>   /lib/systemd/systemd-growfs "${MOUNT_POINT}"  
> > 
> > we could also loop over calling this until it goes "0" or we reach a
> > retry counter, that way we magically handle new systemd versions
> > that do not need symlinks and do not implement anything with
> > symlinks.  
> 
> 
> That's a cool idea. As systemd-growfs returns 1 in all error cases we
> will have to parse stderr to determine if we ran in the error case
> that we want to handle. I will give it a try.

True ... parsing the error smells bad. But let us have a look at the
code we get. We should probably do some code reading of < 252 to make
sure the parser would catch it in all affected versions since stretch.
And if >= 252 solved that for us that code will hopefully never fail
again ... famous last expand-on-first-boot patch ;)
I will do a 252 (bookworm) test run on the raspbi which i used to
repro. In theory that should work without this upcoming patch already.
Also gives the chance to add "raspios-bookworm" to isar.

Henning

> 
> > Henning
> >   
> >>   umount "${MOUNT_POINT}"  
> >