Re: [PATCH v2 10/10] meta-isar/canned-wks: Remove /boot mountpoint

["vijaikumar....@gmail.com" <vijaikumar.kanagarajan@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 9486 bytes --]



On Wednesday, September 9, 2020 at 8:39:27 PM UTC+5:30 Henning Schild wrote:

> On Sat, 5 Sep 2020 09:06:08 -0700 (PDT) 
> "vijaikumar....@gmail.com" <vijaikumar....@gmail.com> wrote: 
>
> > On Saturday, September 5, 2020 at 2:28:17 PM UTC+5:30 Henning Schild 
> > wrote: 
> > 
> > > Hi, 
> > > 
> > > i do not like this one at all. Suggesting people to hide the boot 
> > > partition from debian as hard as they can? If i understand it 
> > > correctly. 
> > 
> > AFAIU , /boot mount-point[1] is wrong and is not needed without 
> > --exclude-path of /boot in rootfs partition. 
> > This optional mount-point serves only one purpose, i.e, to have an 
> > entry added in fstab or not by wic. 
> > We were okay with saying this EFI/bootloader partition mount-point as 
> > /boot, since older wic 
> > was not adding the /boot mount-point to fstab. 
> > With this commit [2] in OE this is no longer the case. 
> > /boot is added to fstab and at boot the system would try to mount to 
> > this efi partiton to /boot 
> > directory which, if successful, will make the kernel, initrd and 
> > config files which were already available in /boot 
> > of rootfs unavailable when the system is booted. I donot see 
> > --exclude-path for /boot in the wks file, 
> > so this would just be wrong to have a mount-point like this. 
>
> Not sure i get all that, but to me it seems we need a wks file without 
> boot or we need one with boot and the exclude. The exclude might not be 
> needed on OE, on debian it is ... so adding the exclude could be a 
> better default/example than removing the boot partition. 
>
> > [1] 
> > 
> https://github.com/ilbers/isar/blob/a0d6de4ac7dfb926b38cfcd44f2bc8852f2e43e3/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks#L5 
> > [2] 
> > 
> https://github.com/openembedded/openembedded-core/commit/2376b05512ddb8c4ec3aaf1df11071f536a76bd9 
> > 
> > 
> > > In fact the hiding is an issue that should be solved, debian should 
> > > know about that partition and use it correctly. 
> > > 
> > > On EFI it only contains the bootloader, so such isar images will be 
> > > able to receive kernel updates with apt-get during their lifetime. 
> > > They will never be able to update the bootloader, but that might be 
> > > ok. 
> > > 
> > > On BIOS the story is different. These systems have the kernel in 
> > > the roots/boot only that kernel is never used for booting and the 
> > > machines will never be able to update the kernel they boot. 
> > > 
> > > I always wanted to solve that but it was not too easy. The first 
> > > idea was just bootloader in the boot partition, like efi. But 
> > > syslinux can not boot files from other partitions, so we need the 
> > > kernel and initrd there. 
> > > But now with the exclude feature it might be easier. Place the 
> > > bootloader and config into rootfs/boot and point to the kernel 
> > > initrd symlinks in the config. (Done in our custom BIOS plugin) 
> > > And now take rootfs/boot as boot, excluding it from rootfs. 
> > 
> > 
> > Can you point me to some code where these kind of scenarios are? I 
> > assume these are all downstream? For downstream you could customize 
> > the wks with neccessary /boot as mountpoint for EFI and 
> > "--exclude-path /boot" to achieve 
> > what you want. I donot see the issue in removing it here. 
>
> There is no code to point to. What i described is the current state of 
> Isar and a possible direction it could take to fix the fact that the 
> boot partition is never visible in the running system. An issue 
> especially problematic for BIOS boot, since kernel updating is broken. 
>

I see, this is only with the the legacy bios based boot. In that case, 
since we
have the /boot mount automatically, we can modify the wks file to exclude
/boot and mount the syslinux boot partition in the rootfs. we might also 
need
to copy over the config and system map files to the syslinux partition for 
completion.

I will add an additional patch for legacy bios and send in v3.

For rest of the images, not specifying /boot is the right way, since those 
fetch
the kernel and initrd directly from the root partition.

Thanks,
Vijai Kumar K
 

>
> Just build a BIOS booting qemu (conf/machine/qemui386.conf), and install 
> i.e. linux-image-rt in the running system with apt-get 
>
> I am not saying this should not be merged. But this is a good time to 
> look at the "debians do not know about that boot partition" issue 
> again, instead of possibly making it worse and including bad examples 
> into meta-isar ... which will later end up in other layers. 
>
> Henning 
>
> > 
> > > 
> > > Henning 
> > > 
> > > On Thu, 3 Sep 2020 00:32:47 +0530 
> > > Vijai Kumar K <Vijaikumar_...@mentor.com> wrote: 
> > > 
> > > > In the older version of wic, the fstab entry for /boot mountpoint 
> > > > was skipped. 
> > > > However in the latest wic this is not the case. Adding a /boot 
> > > > mountpoint in part results in creating a fstab entry for 
> > > > automount. Remove it to avoid the unwanted mount which makes 
> > > > original contents unavailable in debian rootfilesystem. 
> > > > 
> > > > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com> 
> > > > --- 
> > > > RECIPE-API-CHANGELOG.md | 17 
> > > > +++++++++++++++++ .../lib/wic/canned-wks/common-isar.wks.inc | 
> > > > 2 +- meta-isar/scripts/lib/wic/canned-wks/hikey.wks | 2 +- 
> > > > .../scripts/lib/wic/canned-wks/sdimage-efi.wks | 2 +- 
> > > > 4 files changed, 20 insertions(+), 3 deletions(-) 
> > > > 
> > > > diff --git a/RECIPE-API-CHANGELOG.md b/RECIPE-API-CHANGELOG.md 
> > > > index 1839770..070cde3 100644 
> > > > --- a/RECIPE-API-CHANGELOG.md 
> > > > +++ b/RECIPE-API-CHANGELOG.md 
> > > > @@ -246,3 +246,20 @@ by setting DEBIAN_BUILD_DEPENDS. 
> > > > 
> > > > ${S} can now be used for checking out sources without being 
> > > > linked implicitly with ${D} which needs to be filled explicitly 
> > > > in do_install as before. + 
> > > > +### Wic adds /boot mountpoint to fstab 
> > > > + 
> > > > +With the latest wic, /boot mount point, if any, is added to 
> > > > /etc/fstab. Debian +uses /boot to store the kernel and initrd 
> > > > images. If an image's wks file +specifies /boot mountpoint for 
> > > > boot partitions like EFI, the image may fail to +boot or might 
> > > > make the original contents of /boot unavailable after boot. +All 
> > > > unwanted /boot mountpoints should be removed. + 
> > > 
> > 
> > Maybe I can add a bit more to 
> > this sentence to make it clear, something like unless you know what 
> > you are doing. 
> > I could add the rest of the wks entry below to highlight that /boot 
> > is not excluded in the first place. 
> > 
> > 
> > > > +Below is an example wks entry that might cause an issue. 
> > 
> > > +``` 
> > > > +part /boot --source bootimg-efi-isar --sourceparams 
> > > > "loader=grub-efi" --ondisk sda --label efi --part-type EF00 
> > > > --align 1024 +``` +Drop the /boot entry to avoid issues. 
> > > > +``` 
> > > > +part --source bootimg-efi-isar --sourceparams "loader=grub-efi" 
> > > > --ondisk sda --label efi --part-type EF00 --align 1024 +``` 
> > > > diff --git 
> > > > a/meta-isar/scripts/lib/wic/canned-wks/common-isar.wks.inc 
> > > > b/meta-isar/scripts/lib/wic/canned-wks/common-isar.wks.inc index 
> > > > c8ea4c2..e7dabd4 100644 --- 
> > > > a/meta-isar/scripts/lib/wic/canned-wks/common-isar.wks.inc +++ 
> > > > b/meta-isar/scripts/lib/wic/canned-wks/common-isar.wks.inc @@ 
> > > > -1,3 +1,3 @@ # This file is included into 3 canned wks files from 
> > > > this directory -part /boot --source bootimg-pcbios-isar --ondisk 
> > > > sda --label boot --active --align 1024 +part --source 
> > > > bootimg-pcbios-isar --ondisk sda --label boot --active --align 
> > > > 1024 part / --source rootfs --ondisk sda --fstype=ext4 --label 
> > > > platform --align 1024 diff --git 
> > > > a/meta-isar/scripts/lib/wic/canned-wks/hikey.wks 
> > > > b/meta-isar/scripts/lib/wic/canned-wks/hikey.wks index 
> > > > 1e82f8c..840858e 100644 --- 
> > > > a/meta-isar/scripts/lib/wic/canned-wks/hikey.wks +++ 
> > > > b/meta-isar/scripts/lib/wic/canned-wks/hikey.wks @@ -3,7 +3,7 @@ 
> > > > # # SPDX-License-Identifier: MIT 
> > > > 
> > > > -part /boot --source bootimg-efi-isar --sourceparams 
> > > > "loader=grub-efi" --ondisk mmcblk1 --label efi --part-type EF00 
> > > > --align 1024 +part --source bootimg-efi-isar --sourceparams 
> > > > "loader=grub-efi" --ondisk mmcblk1 --label efi --part-type EF00 
> > > > --align 1024 part / --source rootfs --ondisk mmcblk1 --fstype 
> > > > ext4 --label platform --align 1024 --use-uuid diff --git 
> > > > a/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks 
> > > > b/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks index 
> > > > 2afa016..e0fa7a1 100644 --- 
> > > > a/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks +++ 
> > > > b/meta-isar/scripts/lib/wic/canned-wks/sdimage-efi.wks @@ -2,7 
> > > > +2,7 @@ # long-description: Creates a partitioned EFI disk image 
> > > > without any swap that # the user can directly dd to boot media. 
> > > > -part /boot --source bootimg-efi-isar --sourceparams 
> > > > "loader=grub-efi" --ondisk sda --label efi --part-type EF00 
> > > > --align 1024 +part --source bootimg-efi-isar --sourceparams 
> > > > "loader=grub-efi" --ondisk sda --label efi --part-type EF00 
> > > > --align 1024 part / --source rootfs --ondisk sda --fstype ext4 
> > > > --label platform --align 1024 --use-uuid 
> > > 
> > > 
> > 
>
>

[-- Attachment #1.2: Type: text/html, Size: 12390 bytes --]