| Message ID | 20221213101509.535-2-henning.schild@siemens.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | expand-on-first-boot btrfs and CI testing | expand |
On Tue, 13 Dec 2022 at 11:15, <henning.schild@siemens.com> wrote: > > +MOUNT_POINT=$(mktemp -d -p "" "$(basename "$0").XXXXXXXXXX") this uses /tmp which could not be available debraf@debraf:~$ sudo find / -name expand-last-partition.sh /usr/share/expand-on-first-boot/expand-last-partition.sh debraf@debraf:~$ mktemp -d -p "" "expand-last-partition.sh.XXXXXXXXXX" /tmp/expand-last-partition.sh.K9OhJ5KxIF my suggestion is to use /dev/shm which usually is configured into the kernel or at least try also /dev/shm if the use of /tmp fails. == FELIX == One problem remains: Where to mount. If we mount on /mnt this will not work on an RO rootfs out of the box. We currently have this issue in CIP core. We could mount below /tmp which should be writable, but I don't know which side effects this as the tmpfs itself will be mounted / created by systemd. == == Best regards, R-
On Tue, 13 Dec 2022 at 11:15, <henning.schild@siemens.com> wrote: > > 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. > It is possible to avoid the udevadm settle constraints only because the boot device [1] is the same as the rootfs device. The boot device should be mapped by the kernel at boot time, so it would be available but not necessarily present in /dev partition if this has no static links or devfs mounted at the time of mount. If udevd is the demon that we choose to populate the /dev then it would be better to wait until it settles down. The stats about boot time on different platforms will show that only hardware slow to detect has a larger boot time. [1] in some embedded devices the boot device is an internal and small device dedicated to the boot while the rootfs is on a separate plug-able device. I have no clue how this would matter in the case of ISAR. Best regards, R-
Am Tue, 13 Dec 2022 17:45:23 +0100 schrieb "Roberto A. Foglietta" <roberto.foglietta@gmail.com>: > On Tue, 13 Dec 2022 at 11:15, <henning.schild@siemens.com> wrote: > > > > +MOUNT_POINT=$(mktemp -d -p "" "$(basename "$0").XXXXXXXXXX") > > this uses /tmp which could not be available The remark from Felix triggered the switch from '-p "/mnt"' to '-p ""', one can set TMPDIR if that does not work. A valid remark, while yours seems made up just to keep trolling around in the whole discussion. Stop it please. Henning > debraf@debraf:~$ sudo find / -name expand-last-partition.sh > /usr/share/expand-on-first-boot/expand-last-partition.sh > debraf@debraf:~$ mktemp -d -p "" "expand-last-partition.sh.XXXXXXXXXX" > /tmp/expand-last-partition.sh.K9OhJ5KxIF > > my suggestion is to use /dev/shm which usually is configured into the > kernel or at least try also /dev/shm if the use of /tmp fails. > > == FELIX == > > One problem remains: Where to mount. > If we mount on /mnt this will not work on an RO rootfs out of the box. > We currently have this issue in CIP core. > > We could mount below /tmp which should be writable, but I don't know > which side effects this as the tmpfs itself will be mounted / created > by systemd. > > == == > > Best regards, R-
On Tue, 13 Dec 2022 at 18:18, Henning Schild <henning.schild@siemens.com> wrote: > > Am Tue, 13 Dec 2022 17:45:23 +0100 > schrieb "Roberto A. Foglietta" <roberto.foglietta@gmail.com>: > > > On Tue, 13 Dec 2022 at 11:15, <henning.schild@siemens.com> wrote: > > > > > > +MOUNT_POINT=$(mktemp -d -p "" "$(basename "$0").XXXXXXXXXX") > > > > this uses /tmp which could not be available > > The remark from Felix triggered the switch from '-p "/mnt"' to '-p ""', > one can set TMPDIR if that does not work. > > A valid remark, while yours seems made up just to keep trolling around > in the whole discussion. Stop it please. Please Hanning, tell to it to my ubuntu - she has some issue to understand your slang roberto:~$ export TMPDIR=/dev/shm roberto:~$ mktemp -d -p "" ciao.XXXXX /dev/shm/ciao.X3wlT Ah, oops - in this following way it working, instead roberto:~$ mktemp -d -p "$TMPDIR" ciao.XXXXX /dev/shm/ciao.jQFYr Possibly you did not catch the Felix suggestion? LOL Ciao, R-
Am Tue, 13 Dec 2022 21:44:01 +0100 schrieb "Roberto A. Foglietta" <roberto.foglietta@gmail.com>: > On Tue, 13 Dec 2022 at 18:18, Henning Schild > <henning.schild@siemens.com> wrote: > > > > Am Tue, 13 Dec 2022 17:45:23 +0100 > > schrieb "Roberto A. Foglietta" <roberto.foglietta@gmail.com>: > > > > > On Tue, 13 Dec 2022 at 11:15, <henning.schild@siemens.com> wrote: > > > > > > > > > > > +MOUNT_POINT=$(mktemp -d -p "" "$(basename "$0").XXXXXXXXXX") > > > > > > this uses /tmp which could not be available > > > > The remark from Felix triggered the switch from '-p "/mnt"' to '-p > > ""', one can set TMPDIR if that does not work. > > > > A valid remark, while yours seems made up just to keep trolling > > around in the whole discussion. Stop it please. > > Please Hanning, tell to it to my ubuntu - she has some issue to > understand your slang My name is "Henning" and yours is not Joe! > roberto:~$ export TMPDIR=/dev/shm > roberto:~$ mktemp -d -p "" ciao.XXXXX > /dev/shm/ciao.X3wlT > > Ah, oops - in this following way it working, instead > > roberto:~$ mktemp -d -p "$TMPDIR" ciao.XXXXX > /dev/shm/ciao.jQFYr Not sure what i see here but it looks like both temporary directories have been created under TMPDIR with -p "" and with -p "$TMPDIR". I like the "ciao" since it seems to suggest you are willing to leave it. Henning > Possibly you did not catch the Felix suggestion? LOL > > Ciao, R-
On Tue, 13 Dec 2022 at 22:06, Henning Schild <henning.schild@siemens.com> wrote: > > Am Tue, 13 Dec 2022 21:44:01 +0100 > schrieb "Roberto A. Foglietta" <roberto.foglietta@gmail.com>: > > > On Tue, 13 Dec 2022 at 18:18, Henning Schild > > <henning.schild@siemens.com> wrote: > > > > > > Am Tue, 13 Dec 2022 17:45:23 +0100 > > > schrieb "Roberto A. Foglietta" <roberto.foglietta@gmail.com>: > > > > > > > On Tue, 13 Dec 2022 at 11:15, <henning.schild@siemens.com> wrote: > > > > > > > > > > > > > > +MOUNT_POINT=$(mktemp -d -p "" "$(basename "$0").XXXXXXXXXX") > > > > > > > > this uses /tmp which could not be available > > > > > > The remark from Felix triggered the switch from '-p "/mnt"' to '-p > > > ""', one can set TMPDIR if that does not work. > > > > > > A valid remark, while yours seems made up just to keep trolling > > > around in the whole discussion. Stop it please. > > > > Please Hanning, tell to it to my ubuntu - she has some issue to > > understand your slang > > My name is "Henning" and yours is not Joe! > > > roberto:~$ export TMPDIR=/dev/shm > > roberto:~$ mktemp -d -p "" ciao.XXXXX > > /dev/shm/ciao.X3wlT > > > > Ah, oops - in this following way it working, instead > > > > roberto:~$ mktemp -d -p "$TMPDIR" ciao.XXXXX > > /dev/shm/ciao.jQFYr > > Not sure what i see here but it looks like both temporary directories > have been created under TMPDIR with -p "" and with -p "$TMPDIR". What Felix tried to tell you was to use a variable like in my patch but using TMPDIR as the name of the variable. When that variable is not defined then "" is the default value. Now, what happens to mktemp when it is called with -p ""? Nothing bad, it simply performs without -p. Why was Felix pointing this out? Probably because in my patch tmp variable should be set if null but using mktemp this is not necessary anymore. > > I like the "ciao" since it seems to suggest you are willing to leave > it. > I have the light sensation that after two and more months needed to solve an issue that we solved in 2 days after my intervention in this list... ciao is for you. > Henning > > > Possibly you did not catch the Felix suggestion? LOL > > > > Ciao, R- >
Am Tue, 13 Dec 2022 18:01:49 +0100 schrieb "Roberto A. Foglietta" <roberto.foglietta@gmail.com>: > On Tue, 13 Dec 2022 at 11:15, <henning.schild@siemens.com> wrote: > > > > 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. > > > > It is possible to avoid the udevadm settle constraints only because > the boot device [1] is the same as the rootfs device. The boot device > should be mapped by the kernel at boot time, so it would be available > but not necessarily present in /dev partition if this has no static > links or devfs mounted at the time of mount. If udevd is the demon > that we choose to populate the /dev then it would be better to wait > until it settles down. The stats about boot time on different > platforms will show that only hardware slow to detect has a larger > boot time. > > [1] in some embedded devices the boot device is an internal and small > device dedicated to the boot while the rootfs is on a separate > plug-able device. > > I have no clue how this would matter in the case of ISAR. The whole thing is about only one device and its last partition, not more! And we clearly expect boot and root to reside on the same mass-storage. Or any other last partition (my series was also manually tested against another /data partition to be grown). Thanks for approving that we do not need to "udevadm settle" in that case! For any more advanced scenario people can always run whatever they want, possibly systemd-repart. Isar is all about a single image for one mass storage device, it can be used for more but such code will have to live in layers. With btrfs we could have one filesystem with multiple devices/partitions and the code i (and you and Joe) proposed here will likely not work. Let us keep things simple in the core while allowing layers to do whatever they please. expand-on-first-boot is for the "normal rpi case" where a stand-alone image gets flashed to one device of which we only know the final size when we boot it up. Not just rpi ... it could also be your qemuamd64 image flashed to an ssd/hdd in any PC. ciao, Henning > Best regards, R- >
On Tue, 13 Dec 2022 at 22:22, Henning Schild <henning.schild@siemens.com> wrote: > > Am Tue, 13 Dec 2022 18:01:49 +0100 > schrieb "Roberto A. Foglietta" <roberto.foglietta@gmail.com>: > > [1] in some embedded devices the boot device is an internal and small > > device dedicated to the boot while the rootfs is on a separate > > plug-able device. > > > > I have no clue how this would matter in the case of ISAR. > > The whole thing is about only one device and its last partition, not > more! And we clearly expect boot and root to reside on the same > mass-storage. Or any other last partition (my series was also manually > tested against another /data partition to be grown). > > Thanks for approving that we do not need to "udevadm settle" in that > case! Thank you for letting us know that the main problem was even easier than supposed to be. Trying to help you or give you an assist is clearly a boomerang. I will take it into consideration for the future. By the way, with my virtualbox machine (basic-os image) the kernel time indicates 4.8s with btrfs resize at boot completion and udevadm settle takes so little that it has no meaning to be accounted for. Using /usr/bin/time -o/dev/shm/udevadm-settle.time udevadm settle, the result is here below shown: debraf@debraf:~$ cat /dev/shm/udevadm-settle.time 0.00user 0.00system 0:00.50elapsed 1%CPU (0avgtext+0avgdata 4196maxresident)k 0inputs+0outputs (0major+248minor)pagefaults 0swaps Best regards, R-
On Tue, 2022-12-13 at 22:20 +0100, Roberto A. Foglietta wrote: > On Tue, 13 Dec 2022 at 22:06, Henning Schild < > henning.schild@siemens.com> wrote: > > > > Am Tue, 13 Dec 2022 21:44:01 +0100 > > schrieb "Roberto A. Foglietta" <roberto.foglietta@gmail.com>: > > > > > On Tue, 13 Dec 2022 at 18:18, Henning Schild > > > <henning.schild@siemens.com> wrote: > > > > > > > > Am Tue, 13 Dec 2022 17:45:23 +0100 > > > > schrieb "Roberto A. Foglietta" <roberto.foglietta@gmail.com>: > > > > > > > > > On Tue, 13 Dec 2022 at 11:15, <henning.schild@siemens.com> > > > > > wrote: > > > > > > > > > > > > > > > > > +MOUNT_POINT=$(mktemp -d -p "" "$(basename > > > > > > "$0").XXXXXXXXXX") > > > > > > > > > > this uses /tmp which could not be available > > > > > > > > The remark from Felix triggered the switch from '-p "/mnt"' to > > > > '-p > > > > ""', one can set TMPDIR if that does not work. > > > > > > > > A valid remark, while yours seems made up just to keep trolling > > > > around in the whole discussion. Stop it please. > > > > > > Please Hanning, tell to it to my ubuntu - she has some issue to > > > understand your slang > > > > My name is "Henning" and yours is not Joe! > > > > > roberto:~$ export TMPDIR=/dev/shm > > > roberto:~$ mktemp -d -p "" ciao.XXXXX > > > /dev/shm/ciao.X3wlT > > > > > > Ah, oops - in this following way it working, instead > > > > > > roberto:~$ mktemp -d -p "$TMPDIR" ciao.XXXXX > > > /dev/shm/ciao.jQFYr > > > > Not sure what i see here but it looks like both temporary > > directories > > have been created under TMPDIR with -p "" and with -p "$TMPDIR". > > What Felix tried to tell you was to use a variable like in my patch > but using TMPDIR as the name of the variable. When that variable is > not defined then "" is the default value. Now, what happens to mktemp > when it is called with -p ""? Nothing bad, it simply performs without > -p. Why was Felix pointing this out? Probably because in my patch tmp > variable should be set if null but using mktemp this is not necessary > anymore. No! Please also stop guessing what I could have meant and wait at least a day for me to answer (instead of trying to convince others of your interpretation of my opinion). The way Henning implemented it is exactly how I meant it. As this whole thing runs as a systemd service, a user can easily set TMPDIR via a drop-in file. Felix > > > > > I like the "ciao" since it seems to suggest you are willing to > > leave > > it. > > > > I have the light sensation that after two and more months needed to > solve an issue that we solved in 2 days after my intervention in this > list... ciao is for you. > > > Henning > > > > > Possibly you did not catch the Felix suggestion? LOL > > > > > > Ciao, R- > >
On Wed, 14 Dec 2022 at 04:33, Moessbauer, Felix <felix.moessbauer@siemens.com> wrote: > > What Felix tried to tell you was to use a variable like in my patch > > but using TMPDIR as the name of the variable. When that variable is > > not defined then "" is the default value. Now, what happens to mktemp > > when it is called with -p ""? Nothing bad, it simply performs without > > -p. Why was Felix pointing this out? Probably because in my patch tmp > > variable should be set if null but using mktemp this is not necessary > > anymore. > > No! Please also stop guessing what I could have meant and wait at least > a day for me to answer (instead of trying to convince others of your > interpretation of my opinion). > > The way Henning implemented it is exactly how I meant it. > > As this whole thing runs as a systemd service, a user can easily set > TMPDIR via a drop-in file. > Felix, thanks for the clarification, the behaviour is aligned with the manual but expliciting the variable would have been easier to read without checking the manal -p DIR, --tmpdir[=DIR] interpret TEMPLATE relative to DIR; if DIR is not specified, use $TMPDIR if set, else /tmp. With this option, TEMPLATE must not be an absolute name; unlike with -t, TEMPLATE may contain slashes, but mktemp creates only the final component This is the output of the command without, with and with specified TMPDIR variable roberto:~/robang74/isar-nvidia-debian$ echo "x$TMPDIR" x roberto:~/robang74/isar-nvidia-debian$ mktemp -d -p "" ciao.XXX /tmp/ciao.wLK roberto:~/robang74/isar-nvidia-debian$ export TMPDIR=/dev/shm roberto:~/robang74/isar-nvidia-debian$ mktemp -d -p "" ciao.XXX /dev/shm/ciao.fYb roberto:~/robang74/isar-nvidia-debian$ mktemp -d ciao.XXX ciao.Lwp roberto:~/robang74/isar-nvidia-debian$ mktemp -d -p "$TMPDIR" ciao.XXX /dev/shm/ciao.gw1 Cheers, R-
On Wed, 14 Dec 2022 at 07:50, Roberto A. Foglietta <roberto.foglietta@gmail.com> wrote: > > > > > The way Henning implemented it is exactly how I meant it. > ..and moreover my suggestion to him was to make a second try with /dev/shm before quitting with error roberto:~/robang74/isar-nvidia-debian$ export TMPDIR="/dev/null" roberto:~/robang74/isar-nvidia-debian$ tmpdir=$(mktemp -d -p "$TMPDIR" ciao.XXX || mktemp -d -p "/dev/shm" ciao.XXX) mktemp: failed to create directory via template ‘/dev/null/ciao.XXX’: Not a directory roberto:~/robang74/isar-nvidia-debian$ echo $tmpdir /dev/shm/ciao.NWo More in general - maintainability: make code that are easier to read - resilience: provide a reasonable alternative to failure Best regards, R-
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..0bc686059718 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 -p "" "$(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