From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6842309179660566528 X-Received: by 2002:aa7:dd8e:: with SMTP id g14mr17936466edv.208.1593435331814; Mon, 29 Jun 2020 05:55:31 -0700 (PDT) X-BeenThere: isar-users@googlegroups.com Received: by 2002:a17:906:1b1b:: with SMTP id o27ls4773464ejg.3.gmail; Mon, 29 Jun 2020 05:55:31 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw/safi4Tpdvl6OlL/VeTxthRPPxNRwvCxo6ic/kRNbPeNdRQsDSN/EnSADpv5romcmoCit X-Received: by 2002:a17:906:b292:: with SMTP id q18mr14399181ejz.253.1593435331261; Mon, 29 Jun 2020 05:55:31 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1593435331; cv=none; d=google.com; s=arc-20160816; b=ajwKmiDcYSC9zGsxQIjg5yD1hxJrv9fENlpPTVR8n5THFXHZ/WwuY6jH0oi4j6pk3p kDUkWgjB60QCwLkb2SgA9hRGscTOOA1v527ZSTZSYJ/Hmwl48hlZgeeWML7gb5QY9d6s RBFgPNpiPgKMu1ZUh+1LsK5IoHWG+bmerZQAN0GZ9EUuXx5yF9geZZERAdhKCjIRzyDQ EdhSv0/DthfO6z2HVMhoQMFmpf0AIsXacX592ja9Uju856nAASIzCifVmBet27WA1ZPb kkCS/8SYFzPVcN00LdpRga5riSKlrCGkMVjCRdy5q2L4YZK2u+WD0eWdVBWzDdX6Aac8 EQQw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date; bh=mDaSWWCJA/+Ubr7EhFaGoj7zpcYAq4NqA7veQDZJA6c=; b=PNbbqypVKbGh9lypuq7WspmvBEj1eeWl486AAkK17PmJVNPYhxSnNS7QHpXwnFmn7J fHqrhrDz//4ZEoG/zvUvcq4IZhXCAMhnpqDMNlBo+5ky9hlJlAf0yFDkppaH9r67XAZ3 mqMPHWyG+K5cs7SY54IcgnNu1+umeR3LoRjF9qBHf07Yph/vtzBkBBLFgUOoXD0j/z5p coKBXfrwYVnK6ELI6r2Z+pdgZ85zdpxSOjGTdxUu6h0AwjaRe9ag1n/NOfA4PRVprQVd kEXNPlcKvFF9G5QTWtf2BtNfQJ2tx/IG0T6sR9FlHrOqBdp6ToQGOYLQM4xhP0CYDgNA o3XQ== ARC-Authentication-Results: i=1; gmr-mx.google.com; spf=pass (google.com: domain of henning.schild@siemens.com designates 194.138.37.40 as permitted sender) smtp.mailfrom=henning.schild@siemens.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=siemens.com Return-Path: Received: from gecko.sbs.de (gecko.sbs.de. [194.138.37.40]) by gmr-mx.google.com with ESMTPS id x21si1090172eju.0.2020.06.29.05.55.31 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 29 Jun 2020 05:55:31 -0700 (PDT) Received-SPF: pass (google.com: domain of henning.schild@siemens.com designates 194.138.37.40 as permitted sender) client-ip=194.138.37.40; Authentication-Results: gmr-mx.google.com; spf=pass (google.com: domain of henning.schild@siemens.com designates 194.138.37.40 as permitted sender) smtp.mailfrom=henning.schild@siemens.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=siemens.com Received: from mail1.sbs.de (mail1.sbs.de [192.129.41.35]) by gecko.sbs.de (8.15.2/8.15.2) with ESMTPS id 05TCtUT2003812 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 29 Jun 2020 14:55:30 +0200 Received: from md1za8fc.ad001.siemens.net ([167.87.18.70]) by mail1.sbs.de (8.15.2/8.15.2) with ESMTP id 05TCtT3S015074; Mon, 29 Jun 2020 14:55:29 +0200 Date: Mon, 29 Jun 2020 14:55:28 +0200 From: Henning Schild To: Jan Kiszka Cc: Harald Seiler , Claudius Heine , Subject: Re: [PATCH] image: Run copy_boot_files after rootfs postprocessing Message-ID: <20200629145528.37c2c8d8@md1za8fc.ad001.siemens.net> In-Reply-To: References: <20200625153351.3402179-1-hws@denx.de> <20200625184822.236ff069@md1za8fc.ad001.siemens.net> <91ce92c15d267e4836ab4d9de2870bc8e6f6dfa1.camel@denx.de> <5a86e555-416e-d788-2655-003403f1d190@siemens.com> <6bd78ab80bde335821d792046ad2803e4bc9bb57.camel@denx.de> X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TUID: eSOBNG/PElwz On Mon, 29 Jun 2020 14:41:29 +0200 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. > > > 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. I anyway do not get why that UUID is needed in the first place. Which partition to take as root partition can be decided with a partition label. You just have to be careful with swupdate, it destroy those labels when flashing whole partitions. Even if you decide to use an UUID, the bootloader should know that and pass that arg. ... root=$(get_part_uuid(initrd-file)) ... Might require improving bootloaders ;) or writing bootloader configs instead of initrds in postrprocess ... You can also put your static desired UUID in your wks-file and a postinst of a customization-package. But that might be only for recipes/layers that are not meant for sharing Henning > Jan >