From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6842309179660566528 X-Received: by 2002:adf:f104:: with SMTP id r4mr2651725wro.90.1593161067972; Fri, 26 Jun 2020 01:44:27 -0700 (PDT) X-BeenThere: isar-users@googlegroups.com Received: by 2002:adf:f58d:: with SMTP id f13ls145376wro.1.gmail; Fri, 26 Jun 2020 01:44:26 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz9pqyGxHGb7yZGOHtaURXlb6vZ0+sskR7aPvvJetLQyC24Z93s/AQVWd6rbd7697IxdS8A X-Received: by 2002:adf:e7c2:: with SMTP id e2mr2661203wrn.179.1593161066928; Fri, 26 Jun 2020 01:44:26 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1593161066; cv=none; d=google.com; s=arc-20160816; b=SmmLmI2s96d5tUmXXoI0wOrJTLjYcRlUVtf2TOlKIb2gwUTvvTW3G44lac8x8oMJ+l /v6X1ka+KyDEcMxT2DLzN/gKGea5ronS9m+cCGyytjfxaJEeQ1LKL8KHu7z7owFPo3WD /FwvZ/iJWWYqRY/9CaCj7qPnLcPEhnb4GeJ+KsRT8vImVTTmn49kBrCO92/T6X9ca+Ay qxCXWfxabT1ErWeC0xe+shgoR76OTu9Or9x5S/6PiOtFpnBruJ7hrEagoU3oC7vEROD4 tsMK8X478OYS5oJNSrzFD43bSZb2QsmTLvIMCp7qmH1ndYPNp5rcVUo1FnCiF4JHL2g7 92Yw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject; bh=zIUJA4wdTjfi/Lfx2SQSZua81mPU688sIYeruOHJ6J4=; b=ZURKd0sglvHWiZR8eIc//5DHL+9K4SLFlVw5VQt48C91T7a1srlwMmC9yJSkW3ASpt r8ULdK2x2kSgKBKn4qpGisZEVY95cYqAOoyIwt2oYfJ5VnzXMAkAPgg0mPIdnSKzVdPm HgV4wd7IZkarEouva9UhjbpgziN3YJ/WmrOYeF73zDTUDpbFayBbvm9WjN4R1HK0b9Bv swT8+A50yIF6Ga+5XN+SbhGyYD1MM0IjuDNKMXIoaJytJmPb2epL5YyxMBrn5r9tt+GA dKfmc3PvsDT3fXVm3EdIuRSkHAMkf2aVtCqV6i5bvSG+Rzq6S9yszh8AHwhHucgkLo9r dZ3A== ARC-Authentication-Results: i=1; gmr-mx.google.com; spf=pass (google.com: domain of jan.kiszka@siemens.com designates 194.138.37.40 as permitted sender) smtp.mailfrom=jan.kiszka@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 m16si849285wmg.2.2020.06.26.01.44.26 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 26 Jun 2020 01:44:26 -0700 (PDT) Received-SPF: pass (google.com: domain of jan.kiszka@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 jan.kiszka@siemens.com designates 194.138.37.40 as permitted sender) smtp.mailfrom=jan.kiszka@siemens.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=siemens.com Received: from mail2.sbs.de (mail2.sbs.de [192.129.41.66]) by gecko.sbs.de (8.15.2/8.15.2) with ESMTPS id 05Q8iQhg011741 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 26 Jun 2020 10:44:26 +0200 Received: from [167.87.241.222] ([167.87.241.222]) by mail2.sbs.de (8.15.2/8.15.2) with ESMTP id 05Q8iPjX006702; Fri, 26 Jun 2020 10:44:25 +0200 Subject: Re: [PATCH] image: Run copy_boot_files after rootfs postprocessing To: Henning Schild , Harald Seiler Cc: isar-users@googlegroups.com, Claudius Heine 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> From: Jan Kiszka Message-ID: <71137783-3fd2-daff-52fa-f69465a47703@siemens.com> Date: Fri, 26 Jun 2020 10:44:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: <20200626102615.78cb3c3c@md1za8fc.ad001.siemens.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-TUID: G/qH1H8/nnnY 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. Jan