Re: [PATCH] image: Run copy_boot_files after rootfs postprocessing

[Jan Kiszka <jan.kiszka@siemens.com>]

On 29.06.20 14:22, Harald Seiler wrote:
> Hi Jan,
> 
> On Fri, 2020-06-26 at 11:12 +0200, Jan Kiszka wrote:
>> On 26.06.20 09:17, Claudius Heine wrote:
>>> Hi Harald,
>>>
>>> On 2020-06-25 19:24, Harald Seiler wrote:
>>>> Hello Henning,
>>>>
>>>> On Thu, 2020-06-25 at 19:02 +0200, Jan Kiszka wrote:
>>>>> On 25.06.20 18:48, [ext] Henning Schild wrote:
>>>>>> Hi Harald,
>>>>>>
>>>>>> can you elaborate on those cases? The postprocessing is hacky, if the
>>>>>> problem is coming from your layer you should probably keep this patch
>>>>>> in you layer.
>>>>>
>>>>> Basically do_generate_image_uuid from
>>>>> https://lore.kernel.org/cip-dev/20200625141015.31719-4-Quirin.Gylstorff@siemens.com/T/#u,
>>>>> just modeled as post-processing hook, rather than a task.
>>>>
>>>> For reference, this is the exact code:
>>>>
>>>>       ROOTFS_POSTPROCESS_COMMAND =+ "image_postprocess_generate_uuid"
>>>>       image_postprocess_generate_uuid() {
>>>>           sudo sed -i '/^IMAGE_UUID=.*/d' '${IMAGE_ROOTFS}/etc/os-release'
>>>>           echo "IMAGE_UUID=\"${IMAGE_UUID}\"" | \
>>>>               sudo tee -a '${IMAGE_ROOTFS}/etc/os-release'
>>>>
>>>>           sudo -E chroot '${ROOTFSDIR}' \
>>>>               update-initramfs -u
>>>>       }
>>>>
>>>>> Jan
>>>>>
>>>>>> Maybe you can point out an issue in isar itself, or explain how you got
>>>>>> into this situation? We can then see if your change is generic enough
>>>>>> for upstream. You could also provide the error-case from your layer as
>>>>>> an upstream feature, if that is generic enough.
>>>>
>>>> I think this patch addresses an issue in isar itself.  There is no reason
>>>> for copy_boot_files() to run before the postprocessing does.  I've checked
>>>> through the git history and the reason this relationship was introduced
>>>> was a bigger refactor of the task dependency chain.  It does not seem to
>>>> be intentionally this way from what I can tell.
>>>>
>>>> The other way around makes more sense, in my opinion.  As stated in the
>>>> commit message, postprocessing might do an update to the initramfs (as
>>>> seen above) and this change needs to be reflected in the deployed
>>>> initramfs as well, instead of silently only living in the version that is
>>>> part of the rootfs.
>>>>
>>>> I also checked all existing postprocessing commands and did not see any
>>>> that assume to be run after the boot files have been deployed.
>>>
>>> Its been a while when I implemented this, but I also thought of the
>>> scenario where someone would like to 'minimize' a image via the root fs
>>> postprocessing by deleting everything that is not needed, and that could
>>> possible include the kernel + initramfs, if those are stored somewhere
>>> else outside the root file system. So the idea was, IIRC, to move the
>>> kernel and initrd to the deploy dir, out of harms way, before
>>> postprocessing does its rootfs manipulation.
>>>
>>> So by ordering the copy_boot_files behind the root fs post processing,
>>> you might break other layers that rely on this ordering and have such
>>> 'minimization' procedures, that remove the kernel package and specific
>>> files.
>>>
>>> We don't have such 'minimization' stuff in upstream isar, since it
>>> pretty much breaks apt and dpkg, but if you do image based update, you
>>> might not care.
>>
>> I think the problem with this pattern is elsewhere: We should not
>> install stuff on the rootfs in the first place that shall not end up in
>> the rootfs. That this copy_boot_files thing depends on the installation
>> on the rootfs is actually a bug. It should use the chroot for its work,
>> like the imager does (for the bootloader e.g.).
> 
> For kernel and DTB I am totally on your side but I'm not sure how you
> would want to do this for the initramfs.  I think the initramfs should
> definitely be generated from the 'real' rootfs because otherwise you might
> get packages from chroot 'polluting' it in unexpected ways.  Also,

That is no issue: buildchroot-target and target rootfs are in line. If 
not, you have a different issue.

> considering the IMAGE_UUID use-case in particular, how would you get the
> ID from the rootfs into the chroot for the hook to pick it up?  Not sure
> whether there is a clean solution for this ...
> 

E.g. by generating the UUID at bitbake level and injecting it into the 
images - rather than doing that inside one of the images.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux