From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6842309179660566528 X-Received: by 2002:adf:ab4a:: with SMTP id r10mr2648063wrc.103.1593159978304; Fri, 26 Jun 2020 01:26:18 -0700 (PDT) X-BeenThere: isar-users@googlegroups.com Received: by 2002:a1c:cc0a:: with SMTP id h10ls4014394wmb.3.gmail; Fri, 26 Jun 2020 01:26:17 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxXgcjdDsop5lv+og5OMvjAmZ2XRGImSrhKLsVqRTqkiJlmZe91defuaBZgnQjNbWzKKiU/ X-Received: by 2002:a7b:c381:: with SMTP id s1mr2260016wmj.25.1593159977774; Fri, 26 Jun 2020 01:26:17 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1593159977; cv=none; d=google.com; s=arc-20160816; b=QRsMgkldqO87geHRBQRgyyn6ZuU0ytL+p0C6cKvi07de5WxF6T83LFilfLbFG/X3q4 0D6+G3RU7kfHbfFpFcWQfbf5AFW8AEBaicG//8+qHLkE+d9qQRyDLpLaZI8V1/A+0TAG YqSLFcGeyQXLlIs/Qi6YpN/qIQg2b/tlRnQKhplebW8+6Xo6S8fgrE0m//PPZ838zU7c r/bec7N6DC97YVi3sC/PKerVZ9XYt3bghd/YZup8s6DthVwIzCsgZC2tM0TOyirXSJb8 JTvela2Teiux4wNqhKMq/kj+d5JvyoyJDzBrNL59vP7cxR6xjCmhgcJb9EF8NSJAQIKJ S94g== 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=TDBpzEC2/+E0QCc8Rnv1RpJ/CM81IJnn7ledH2+cNkU=; b=JsibPlKxBBXAWl5BmDYUG9ym3FDCbEzFlfAex69dJ7hl5nSA04Jjn/NwSlWM8rOoqa 7IdKk/y3iEO6AyB2j3XU/18khpDcerdb4k4/eCFO+yHk4H3LJNpQQHG6axf6zVqMqv5C 1RPzju0oc7AmXJR+vKYYZf4LuLgowacv9RqDxIMe+7LzPsf73CyNo+PblrKvtM0QjyUE lkn5b4yKVBIRUa6Q/CgmMn5b2gnLwLMfb/w6naYhNVCboKaJ/GC9GfGErDIdLSRpuuY+ Dwd+MHjGba4/oQM8Pbk5IvqNECg/3+ivzU2eZg+Bv2OjXcLLBT5Vc/BWu7JF5nX545OH he0g== 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 q12si580141wmj.0.2020.06.26.01.26.17 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 26 Jun 2020 01:26:17 -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 05Q8QG9W024564 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 26 Jun 2020 10:26:16 +0200 Received: from md1za8fc.ad001.siemens.net ([167.87.29.237]) by mail1.sbs.de (8.15.2/8.15.2) with ESMTP id 05Q8QFrx028011; Fri, 26 Jun 2020 10:26:16 +0200 Date: Fri, 26 Jun 2020 10:26:15 +0200 From: Henning Schild To: Harald Seiler Cc: Jan Kiszka , , Claudius Heine Subject: Re: [PATCH] image: Run copy_boot_files after rootfs postprocessing Message-ID: <20200626102615.78cb3c3c@md1za8fc.ad001.siemens.net> In-Reply-To: References: <20200625153351.3402179-1-hws@denx.de> <20200625184822.236ff069@md1za8fc.ad001.siemens.net> <91ce92c15d267e4836ab4d9de2870bc8e6f6dfa1.camel@denx.de> <20200625204307.0aede1b9@md1za8fc.ad001.siemens.net> <20200625212354.55d907b3@md1za8fc.ad001.siemens.net> <20200625212715.692fda49@md1za8fc.ad001.siemens.net> 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: qDKRajv7nJff Am Fri, 26 Jun 2020 10:13:36 +0200 schrieb Harald Seiler : > Hello Henning, > > On Thu, 2020-06-25 at 21:27 +0200, Henning Schild wrote: > > Am Thu, 25 Jun 2020 21:23:54 +0200 > > schrieb "[ext] Henning Schild" : > > > > > Am Thu, 25 Jun 2020 20:43:07 +0200 > > > schrieb "[ext] Henning Schild" : > > > > > > > Am Thu, 25 Jun 2020 19:24:30 +0200 > > > > schrieb Harald Seiler : > > > > > > > > > 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 > > > > > } > > > > > > > > If /etc/os-release goes into the initrd we have the issue with > > > > image_postprocess_mark in isar, a valid reason for a merge. > > It does not. The reason /etc/os-release and the initramfs become > related here is because the IMAGE_UUID from the rootfs needs to be > added into the initramfs so it can later identify the rootfs it > belongs to (when multiple copies exist). This is done by a separate > initramfs hook installed as a package. > > > > But that is not the case. You re-create the initrd in your layer > > > so it might just be your problem! How about we discuss the > > > IMAGE_UUID upstream together with the reordering? > > > > Or do we need the "update-initramfs -u" for image_postprocess_mark? > > > > No, as described above, this is only necessary for IMAGE_UUID because > there, a hook copies the IMAGE_UUID into the initramfs. The rest of > /etc/os-release is irrelevant for initrd (AFAIK). > > > If so that should be done as a patch in this queue, you will get > > your reordering for IMAGE_UUID in your layer if you decide to keep > > that downstream. > > I don't know what the plan is; whether IMAGE_UUID should be > upstreamed at some point. Jan, you're probably the person to ask > here? From my side, I think this would be a useful feature to have. > > > Henning > > > > > Henning > > > > > > > > > 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. > > > > > > > > Ok, sounds fair. > > In the other thread, Claudius has shed some light on the design > decisions that let to this. But let's keep that discussion over > there. > > > > > > 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. > > > > > > > > I just worried about people abusing postprocess for changes that > > > > should be packages instead. Thanks for going into detail! > > If you have a better approach than what I came up with, please do > tell! I decided on postprocessing for a few reasons: > > - The IMAGE_UUID is an 'image feature' as in, the image recipe should > decide what its ID is (an image inheriting `image_uuid` can set the > ID string). So pushing that into a package is difficult. > - As this only adds a value to /etc/os-release instead of deploying > the whole file, a package approach would need to work via postinst > which seems like a hack to me. > - The previous approach from Quirin was to add a separate task for > adding the UUID. This task can then be placed in the dependency > chain where I have now moved the postprocessing but this feels like > duplicated work when a postprocessing task exists as a feature > already. I did not read this since nobody proposed the UUID-feature. Upstream should not care about all of this. There are two different potential changes in some layers that favor one or the other order. But there is no reason for Isar to change the order. A patch without a use-case needs to be rejected. And i think that Claudius "made up cleaning step" might actually be somewhere out there. So we are talking about an interface change without a use-case ... strong reject! Henning