| Message ID | 20221004114629.27275-1-henning.schild@siemens.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | image: remove / entry from fstab template | expand |
Hi, i think this one does not need an entry in the recipe changelog. But i am not sure about recipes-support/enable-fsck, it might be affected by a missing / line. Unless it is only used together with wic, in which case the change it does at first boot can maybe be moved into the wks. I think that whole package was written at a time where wic could not yet create / lines, which it now can. regards, Henning Am Tue, 4 Oct 2022 13:46:29 +0200 schrieb Henning Schild <henning.schild@siemens.com>: > An fstab entry for / should usually not be required. It just makes a > rootfs less portable (USB vs. NFS, vs. SSD). The kernel is the one > that should know which fs to mount and how. That is done via a > bootloader. > > Should additional mount options be needed for a concrete image, an > imager like wic might add an entry. i.e. for adding fspassno or > x-systemd.growfs > > So to make the rootfs more generic and to allow imagers to add their > individual / lines, instead of having to modify an existing one, > remove the line from the template. > > Signed-off-by: Henning Schild <henning.schild@siemens.com> > --- > meta/classes/image.bbclass | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass > index ccff81066178..d8d605d3af07 100644 > --- a/meta/classes/image.bbclass > +++ b/meta/classes/image.bbclass > @@ -327,7 +327,6 @@ image_configure_fstab[weight] = "2" > image_configure_fstab() { > sudo tee '${IMAGE_ROOTFS}/etc/fstab' << EOF > # Begin /etc/fstab > -/dev/root / auto > defaults 0 0 proc /proc > proc nosuid,noexec,nodev 0 0 > sysfs /sys sysfs > nosuid,noexec,nodev 0 0 devpts > /dev/pts devpts gid=5,mode=620 > 0 0
On 04.10.22 13:46, Henning Schild wrote: > An fstab entry for / should usually not be required. It just makes a > rootfs less portable (USB vs. NFS, vs. SSD). The kernel is the one > that should know which fs to mount and how. That is done via a bootloader. > > Should additional mount options be needed for a concrete image, an > imager like wic might add an entry. i.e. for adding fspassno or > x-systemd.growfs > > So to make the rootfs more generic and to allow imagers to add their > individual / lines, instead of having to modify an existing one, remove > the line from the template. > > Signed-off-by: Henning Schild <henning.schild@siemens.com> > --- > meta/classes/image.bbclass | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass > index ccff81066178..d8d605d3af07 100644 > --- a/meta/classes/image.bbclass > +++ b/meta/classes/image.bbclass > @@ -327,7 +327,6 @@ image_configure_fstab[weight] = "2" > image_configure_fstab() { > sudo tee '${IMAGE_ROOTFS}/etc/fstab' << EOF > # Begin /etc/fstab > -/dev/root / auto defaults 0 0 > proc /proc proc nosuid,noexec,nodev 0 0 > sysfs /sys sysfs nosuid,noexec,nodev 0 0 > devpts /dev/pts devpts gid=5,mode=620 0 0 This works also for other imagers than wic? Just poking to avoid that we miss the case where this was once added for (if that exists). Jan
Am Tue, 4 Oct 2022 13:54:38 +0200 schrieb Jan Kiszka <jan.kiszka@siemens.com>: > On 04.10.22 13:46, Henning Schild wrote: > > An fstab entry for / should usually not be required. It just makes a > > rootfs less portable (USB vs. NFS, vs. SSD). The kernel is the one > > that should know which fs to mount and how. That is done via a > > bootloader. > > > > Should additional mount options be needed for a concrete image, an > > imager like wic might add an entry. i.e. for adding fspassno or > > x-systemd.growfs > > > > So to make the rootfs more generic and to allow imagers to add their > > individual / lines, instead of having to modify an existing one, > > remove the line from the template. > > > > Signed-off-by: Henning Schild <henning.schild@siemens.com> > > --- > > meta/classes/image.bbclass | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass > > index ccff81066178..d8d605d3af07 100644 > > --- a/meta/classes/image.bbclass > > +++ b/meta/classes/image.bbclass > > @@ -327,7 +327,6 @@ image_configure_fstab[weight] = "2" > > image_configure_fstab() { > > sudo tee '${IMAGE_ROOTFS}/etc/fstab' << EOF > > # Begin /etc/fstab > > -/dev/root / auto > > defaults 0 0 proc /proc > > proc nosuid,noexec,nodev 0 0 > > sysfs /sys sysfs > > nosuid,noexec,nodev 0 0 devpts > > /dev/pts devpts gid=5,mode=620 > > 0 0 > > This works also for other imagers than wic? Just poking to avoid that > we miss the case where this was once added for (if that exists). AFAIK there are no other imagers that create fully bootable images. We have ext4, probably usually combined with "qemu -kernel" as "bootloader". And "tar.gz" usually combined with NFS and pxeboot or any other external bootloader like uboot on a flash. Both should remain working without the line, at least for qemu CI will shortly tell. And for NFS i am pretty sure it will just work. I can test that on request. Looking at its git history, that is very old and hard to say why it was ever done like that. Likely predates systemd and was taken from an older OE or raspbian. Henning > Jan >
04.10.2022 14:46, Henning Schild wrote: > An fstab entry for / should usually not be required. It just makes a > rootfs less portable (USB vs. NFS, vs. SSD). The kernel is the one > that should know which fs to mount and how. That is done via a bootloader. > > Should additional mount options be needed for a concrete image, an > imager like wic might add an entry. i.e. for adding fspassno or > x-systemd.growfs > > So to make the rootfs more generic and to allow imagers to add their > individual / lines, instead of having to modify an existing one, remove > the line from the template. > > Signed-off-by: Henning Schild <henning.schild@siemens.com> Did anyone managed to test this on real projects?
Am Tue, 11 Oct 2022 21:03:56 +0300 schrieb Anton Mikanovich <amikan@ilbers.de>: > 04.10.2022 14:46, Henning Schild wrote: > > An fstab entry for / should usually not be required. It just makes a > > rootfs less portable (USB vs. NFS, vs. SSD). The kernel is the one > > that should know which fs to mount and how. That is done via a > > bootloader. > > > > Should additional mount options be needed for a concrete image, an > > imager like wic might add an entry. i.e. for adding fspassno or > > x-systemd.growfs > > > > So to make the rootfs more generic and to allow imagers to add their > > individual / lines, instead of having to modify an existing one, > > remove the line from the template. > > > > Signed-off-by: Henning Schild <henning.schild@siemens.com> > > Did anyone managed to test this on real projects? > Why do you ask, do you maybe see a risk? I tried it but that might not count since i proposed that patch. Henning
13.10.2022 18:00, Henning Schild wrote: > Am Tue, 11 Oct 2022 21:03:56 +0300 > schrieb Anton Mikanovich <amikan@ilbers.de>: > >> Did anyone managed to test this on real projects? > Why do you ask, do you maybe see a risk? I tried it but that might not > count since i proposed that patch. > > Henning I've asked it just in case any possible regressions. Hope there will be some replies if any, so lets merge the patch.
04.10.2022 14:46, Henning Schild wrote: > An fstab entry for / should usually not be required. It just makes a > rootfs less portable (USB vs. NFS, vs. SSD). The kernel is the one > that should know which fs to mount and how. That is done via a bootloader. > > Should additional mount options be needed for a concrete image, an > imager like wic might add an entry. i.e. for adding fspassno or > x-systemd.growfs > > So to make the rootfs more generic and to allow imagers to add their > individual / lines, instead of having to modify an existing one, remove > the line from the template. > > Signed-off-by: Henning Schild <henning.schild@siemens.com> Applied to next, thanks.
On Fri, 2022-10-14 at 15:08 +0300, Anton Mikanovich wrote: > 13.10.2022 18:00, Henning Schild wrote: > > Am Tue, 11 Oct 2022 21:03:56 +0300 > > schrieb Anton Mikanovich <amikan@ilbers.de>: > > > > > Did anyone managed to test this on real projects? > > Why do you ask, do you maybe see a risk? I tried it but that might > > not > > count since i proposed that patch. > > > > Henning > > I've asked it just in case any possible regressions. > Hope there will be some replies if any, so lets merge the patch. > If it's still relevant: I've just tried it on a simple project on an HP workstation with some custom options in `--fsoptions` and it works well. Wadim
14.10.2022 19:27, Klincov, Wadim wrote: > If it's still relevant: I've just tried it on a simple project on an HP > workstation with some custom options in `--fsoptions` and it works > well. > > Wadim Hello Wadim, Thanks for checking, this patch was already integrated.
On Tue, 2022-10-04 at 13:46 +0200, Henning Schild wrote: > An fstab entry for / should usually not be required. It just makes a > rootfs less portable (USB vs. NFS, vs. SSD). The kernel is the one > that should know which fs to mount and how. That is done via a bootloader. > > Should additional mount options be needed for a concrete image, an > imager like wic might add an entry. i.e. for adding fspassno or > x-systemd.growfs > > So to make the rootfs more generic and to allow imagers to add their > individual / lines, instead of having to modify an existing one, remove > the line from the template. > > Signed-off-by: Henning Schild <henning.schild@siemens.com> Hi all, FYI: While trying to update Isar in one layer using LAVA tests we recognized that there is now a breaking change: Previously the rootfs was mounted rw on ext4 images because we had the / line in /etc/fstab, triggering systemd-remount logic. As the entry for the root (/) has been removed in /etc/fstab remounting does no longer happen. This breaks several LAVA tests here as well as start-qemu scripts. Adding "rw" to the kernel cmdline would be one option but we have a lot of LAVA job descriptions and even more start- qemu scripts in downstream layers. Felix already send out patches to OE (open embedded) to address that in wic. Necessary Isar patches (wic update, ...) will follow soon. Thanks a lot Felix for taking over. Best regards, Florian
On Thu, 2023-01-05 at 14:08 +0100, Florian Bezdeka wrote: > On Tue, 2022-10-04 at 13:46 +0200, Henning Schild wrote: > > An fstab entry for / should usually not be required. It just makes > > a > > rootfs less portable (USB vs. NFS, vs. SSD). The kernel is the one > > that should know which fs to mount and how. That is done via a > > bootloader. > > > > Should additional mount options be needed for a concrete image, an > > imager like wic might add an entry. i.e. for adding fspassno or > > x-systemd.growfs > > > > So to make the rootfs more generic and to allow imagers to add > > their > > individual / lines, instead of having to modify an existing one, > > remove > > the line from the template. > > > > Signed-off-by: Henning Schild <henning.schild@siemens.com> > > Hi all, > > FYI: While trying to update Isar in one layer using LAVA tests we > recognized that there is now a breaking change: > > Previously the rootfs was mounted rw on ext4 images because we had > the > / line in /etc/fstab, triggering systemd-remount logic. > > As the entry for the root (/) has been removed in /etc/fstab > remounting > does no longer happen. This breaks several LAVA tests here as well as > start-qemu scripts. Adding "rw" to the kernel cmdline would be one > option but we have a lot of LAVA job descriptions and even more > start- > qemu scripts in downstream layers. > > Felix already send out patches to OE (open embedded) to address that > in > wic. Necessary Isar patches (wic update, ...) will follow soon. > Thanks > a lot Felix for taking over. Hi Devs, let me add some additional information: - in OE, the fstab in the rootfs contains the / entry - we added a patch to OE to let WIC handle the root partition, but this had been reverted because it led to duplicated entries (which break systemd-remount) By that, we are no longer in sync with OE. Also considering the revert of the WIC patch, this shows that OE want to keep the / entry in the rootfs fstab for the meantime. Instead of discussing fundamental aspects (like if the line should be in the rootfs fstab or not) I vote for just integrating the deduplication logic in WIC and re-add the / entry in the rootfs fstab. This brings us in sync with OE again and from then on we can discuss how the general design should look like. Felix > > Best regards, > Florian >
On 06.01.23 05:58, Moessbauer, Felix (T CED INW-CN) wrote: > On Thu, 2023-01-05 at 14:08 +0100, Florian Bezdeka wrote: >> On Tue, 2022-10-04 at 13:46 +0200, Henning Schild wrote: >>> An fstab entry for / should usually not be required. It just makes >>> a >>> rootfs less portable (USB vs. NFS, vs. SSD). The kernel is the one >>> that should know which fs to mount and how. That is done via a >>> bootloader. >>> >>> Should additional mount options be needed for a concrete image, an >>> imager like wic might add an entry. i.e. for adding fspassno or >>> x-systemd.growfs >>> >>> So to make the rootfs more generic and to allow imagers to add >>> their >>> individual / lines, instead of having to modify an existing one, >>> remove >>> the line from the template. >>> >>> Signed-off-by: Henning Schild <henning.schild@siemens.com> >> >> Hi all, >> >> FYI: While trying to update Isar in one layer using LAVA tests we >> recognized that there is now a breaking change: >> >> Previously the rootfs was mounted rw on ext4 images because we had >> the >> / line in /etc/fstab, triggering systemd-remount logic. >> >> As the entry for the root (/) has been removed in /etc/fstab >> remounting >> does no longer happen. This breaks several LAVA tests here as well as >> start-qemu scripts. Adding "rw" to the kernel cmdline would be one >> option but we have a lot of LAVA job descriptions and even more >> start- >> qemu scripts in downstream layers. >> >> Felix already send out patches to OE (open embedded) to address that >> in >> wic. Necessary Isar patches (wic update, ...) will follow soon. >> Thanks >> a lot Felix for taking over. > > Hi Devs, > > let me add some additional information: > > - in OE, the fstab in the rootfs contains the / entry > - we added a patch to OE to let WIC handle the root partition, but this > had been reverted because it led to duplicated entries (which break > systemd-remount) > > By that, we are no longer in sync with OE. Also considering the revert > of the WIC patch, this shows that OE want to keep the / entry in the > rootfs fstab for the meantime. > > Instead of discussing fundamental aspects (like if the line should be > in the rootfs fstab or not) I vote for just integrating the > deduplication logic in WIC and re-add the / entry in the rootfs fstab. > This brings us in sync with OE again and from then on we can discuss > how the general design should look like. +1. Let's fix the current bug (duplicate entry for / in /etc/fstab) first. Anything else can be addressed afterwards - if necessary at all. > > Felix > >> >> Best regards, >> Florian >> >
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass index ccff81066178..d8d605d3af07 100644 --- a/meta/classes/image.bbclass +++ b/meta/classes/image.bbclass @@ -327,7 +327,6 @@ image_configure_fstab[weight] = "2" image_configure_fstab() { sudo tee '${IMAGE_ROOTFS}/etc/fstab' << EOF # Begin /etc/fstab -/dev/root / auto defaults 0 0 proc /proc proc nosuid,noexec,nodev 0 0 sysfs /sys sysfs nosuid,noexec,nodev 0 0 devpts /dev/pts devpts gid=5,mode=620 0 0
An fstab entry for / should usually not be required. It just makes a rootfs less portable (USB vs. NFS, vs. SSD). The kernel is the one that should know which fs to mount and how. That is done via a bootloader. Should additional mount options be needed for a concrete image, an imager like wic might add an entry. i.e. for adding fspassno or x-systemd.growfs So to make the rootfs more generic and to allow imagers to add their individual / lines, instead of having to modify an existing one, remove the line from the template. Signed-off-by: Henning Schild <henning.schild@siemens.com> --- meta/classes/image.bbclass | 1 - 1 file changed, 1 deletion(-)