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

[Claudius Heine <ch@denx.de>]

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

Hi Jan,

On 2020-06-29 14:41, Jan Kiszka wrote:
> 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.

I don't think you can rely on that. buildchroot-target is not image
specific, its architecture specific, so if you build multiple different
images in the same project (same TMPDIR) for the same debian
architecture the buildchroot-target will be reused for each on them and
possible contains additional packages from other image builds.

We should probably implement a similar mechanism as OE, where each
recipe gets their own build root fs in isar. Maybe using overlay, or at
least hardlinks should be possible.

regards,
Claudius


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]