From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6842309179660566528 X-Received: by 2002:a1c:2982:: with SMTP id p124mr148310wmp.26.1593162960154; Fri, 26 Jun 2020 02:16:00 -0700 (PDT) X-BeenThere: isar-users@googlegroups.com Received: by 2002:a1c:cc0a:: with SMTP id h10ls4089046wmb.3.gmail; Fri, 26 Jun 2020 02:15:59 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxLiy4kQ+wdf+QCyvYU5qbuIygLVzPeDSuRLKr0vvJRXTu+ocddyLCTY1TeSosFCZz6Nxjm X-Received: by 2002:a7b:cb98:: with SMTP id m24mr2362108wmi.98.1593162959631; Fri, 26 Jun 2020 02:15:59 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1593162959; cv=none; d=google.com; s=arc-20160816; b=Le/6vHLEhmyuuZ+t7rf+UYe3ezTR/Tg6QDhxrPQe7rydERXcgWyF2P5LGKwlcE9HZc i/lRw6U0jHm8qu3RF5wqhQv1OUN5qHhYEG1tXd+pSakmZ3h3cJoZTX3/ix43iwWRpJwl 00TP32ipWmhMDNyfOyiZ2Qpj1Y2EnyOEErnR/7DRVqGqV3JQlmR8nf6udAmLU99CVEHq dVCHBqEKbBQuApBsaAWCzJUH/5NqPiv6magy6yPcTm/8OSCEU9sJs3IL3a8tc03wrGg0 Rweo4u0naISjw6pStqaM/jtlGp/Y5adzAVEHnapo/DI8oA1hyaTOyX5HAviCdPaHIBoy Domg== 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=mk55dhEVeZhKTYQds1UyEyu0og354TTY4zuf2vQICdg=; b=xpLHmaBOoahj3wsNfOZmMj4gWd2OXmYNqzkZLWm+sryXesq3u1Ekbl5Tn0otLup3aJ SobtRQCuaQypbsgNjgfCHDSWZSrpOUeGgTBuzYZB9t0CvkiV+I2jNLykTOh/pcDvonUg Dx9SiABV6sQ3+Pn/dAbne/r4ooHu6IzmTGKwmXGKeBBpCQOOru0Gh08lnh8oU2ZNPgTD jNACFnkK3/wEx9/IdNd6Ob0L0lCspPqVeTVkGkv93HajiY+NCMVSKw42xpVr0oifadWD 4Qe1/58njNlnmjPr+qvdn8aANDcUDT5R7/NEyOOxH4v0WlmqNKT2WdkeeF4jbh3MToQz VeKg== ARC-Authentication-Results: i=1; gmr-mx.google.com; spf=pass (google.com: domain of henning.schild@siemens.com designates 192.35.17.28 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 goliath.siemens.de (goliath.siemens.de. [192.35.17.28]) by gmr-mx.google.com with ESMTPS id s207si640233wme.4.2020.06.26.02.15.59 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 26 Jun 2020 02:15:59 -0700 (PDT) Received-SPF: pass (google.com: domain of henning.schild@siemens.com designates 192.35.17.28 as permitted sender) client-ip=192.35.17.28; Authentication-Results: gmr-mx.google.com; spf=pass (google.com: domain of henning.schild@siemens.com designates 192.35.17.28 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 goliath.siemens.de (8.15.2/8.15.2) with ESMTPS id 05Q9FwTq026469 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 26 Jun 2020 11:15:58 +0200 Received: from md1za8fc.ad001.siemens.net ([167.87.29.237]) by mail1.sbs.de (8.15.2/8.15.2) with ESMTP id 05Q9Fw6h019375; Fri, 26 Jun 2020 11:15:58 +0200 Date: Fri, 26 Jun 2020 11:15:57 +0200 From: Henning Schild To: Jan Kiszka Cc: Harald Seiler , , Claudius Heine , Cedric Hombourger , Vijai Kumar K Subject: Re: [PATCH] image: Run copy_boot_files after rootfs postprocessing Message-ID: <20200626111557.508a10b1@md1za8fc.ad001.siemens.net> In-Reply-To: <71137783-3fd2-daff-52fa-f69465a47703@siemens.com> 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> <20200626102615.78cb3c3c@md1za8fc.ad001.siemens.net> <71137783-3fd2-daff-52fa-f69465a47703@siemens.com> 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: EpdbBoY7CoRK Am Fri, 26 Jun 2020 10:44:25 +0200 schrieb Jan Kiszka : > On 26.06.20 10:26, Henning Schild wrote: > > 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. > > That is obviously wrong. Upstream should care about anything > reasonable that is brought up from active downstream users. Sure. But ideally with a use-case or a test-case that shows the issue that is addressed. I appreciate the upstream work and am trying to squeeze out more ;). Not like anything gets merged here anyway ... Just ran a grep on a popular layer of not so active users, would not be surprised if they had an issue with this reordering. Added people to Cc and will not look into it. Henning > Jan