* [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts @ 2020-03-19 16:44 Bartosz Golaszewski 2020-03-19 16:44 ` [RFC PATCH 1/2] image.bbclass: add an intermediate deploy task Bartosz Golaszewski ` (3 more replies) 0 siblings, 4 replies; 8+ messages in thread From: Bartosz Golaszewski @ 2020-03-19 16:44 UTC (permalink / raw) To: Khem Raj, Richard Purdie, Armin Kuster, Jerome Neanne, Quentin Schulz Cc: Bartosz Golaszewski, openembedded-core From: Bartosz Golaszewski <bgolaszewski@baylibre.com> This is a follow-up to the discussion I started on the OE-core mailing list a couple days ago[1]. These patches propose to split the deployment of image artifacts into two stages where the first one includes all "regular" images and takes place before do_image_complete and the second is mostly aimed at wic right now and happens after do_image_complete. These patches work but I'm sending them as RFC mostly to continue the discussion about possible solutions for the circular dependencies between the rootfs and initramfs. [1] http://lists.openembedded.org/pipermail/openembedded-core/2020-March/294094.html Bartosz Golaszewski (2): image.bbclass: add an intermediate deploy task image.bbclass: deploy artifacts in two stages meta/classes/image.bbclass | 54 ++++++++++++++----- meta/classes/image_types.bbclass | 3 ++ meta/classes/image_types_wic.bbclass | 4 +- .../images/build-appliance-image_15.0.0.bb | 2 +- 4 files changed, 47 insertions(+), 16 deletions(-) -- 2.19.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/2] image.bbclass: add an intermediate deploy task 2020-03-19 16:44 [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts Bartosz Golaszewski @ 2020-03-19 16:44 ` Bartosz Golaszewski 2020-03-19 16:44 ` [RFC PATCH 2/2] image.bbclass: deploy artifacts in two stages Bartosz Golaszewski ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Bartosz Golaszewski @ 2020-03-19 16:44 UTC (permalink / raw) To: Khem Raj, Richard Purdie, Armin Kuster, Jerome Neanne, Quentin Schulz Cc: Bartosz Golaszewski, openembedded-core From: Bartosz Golaszewski <bgolaszewski@baylibre.com> Instead of using do_image_complete for both the deployment of image artifacts into DEPLOY_DIR_IMAGE as well as calling the image post-process commands, split the responsability between two separate tasks: do_image_deploy will take care of the sstate deployment, while do_image_complete will only do the post-processing. This way we can make the dependencies more fine-grained. Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> --- meta/classes/image.bbclass | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass index 07aa1f1fa5..6e2b864f73 100644 --- a/meta/classes/image.bbclass +++ b/meta/classes/image.bbclass @@ -144,7 +144,7 @@ python () { deps += " %s:%s" % (dep, task) return deps - d.appendVarFlag('do_image_complete', 'depends', extraimage_getdepends('do_populate_sysroot')) + d.appendVarFlag('do_image_deploy', 'depends', extraimage_getdepends('do_populate_sysroot')) deps = " " + imagetypes_getdepends(d) d.appendVarFlag('do_rootfs', 'depends', deps) @@ -264,6 +264,17 @@ do_image[dirs] = "${TOPDIR}" do_image[umask] = "022" addtask do_image after do_rootfs +do_image_deploy() { + # This is a placeholder really but it still needs to run for sstate + # to correctly deploy the image artifacts. + true +} +SSTATETASKS += "do_image_deploy" +do_image_deploy[sstate-inputdirs] = "${IMGDEPLOYDIR}" +do_image_deploy[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}" +SSTATE_SKIP_CREATION_task-image-deploy = '1' +addtask do_image_deploy after do_image before do_build + fakeroot python do_image_complete () { from oe.utils import execute_pre_post_process @@ -273,12 +284,8 @@ fakeroot python do_image_complete () { } do_image_complete[dirs] = "${TOPDIR}" do_image_complete[umask] = "022" -SSTATETASKS += "do_image_complete" -SSTATE_SKIP_CREATION_task-image-complete = '1' -do_image_complete[sstate-inputdirs] = "${IMGDEPLOYDIR}" -do_image_complete[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}" do_image_complete[stamp-extra-info] = "${MACHINE_ARCH}" -addtask do_image_complete after do_image before do_build +addtask do_image_complete after do_image_deploy before do_build python do_image_complete_setscene () { sstate_setscene(d) } @@ -500,8 +507,8 @@ python () { d.appendVarFlag(task, 'vardeps', ' ' + ' '.join(vardeps)) d.appendVarFlag(task, 'vardepsexclude', ' DATETIME DATE ' + ' '.join(vardepsexclude)) - bb.debug(2, "Adding task %s before %s, after %s" % (task, 'do_image_complete', after)) - bb.build.addtask(task, 'do_image_complete', after, d) + bb.debug(2, "Adding task %s before do_image_deploy, after %s" % (task, after)) + bb.build.addtask(task, 'do_image_deploy', after, d) } # -- 2.19.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 2/2] image.bbclass: deploy artifacts in two stages 2020-03-19 16:44 [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts Bartosz Golaszewski 2020-03-19 16:44 ` [RFC PATCH 1/2] image.bbclass: add an intermediate deploy task Bartosz Golaszewski @ 2020-03-19 16:44 ` Bartosz Golaszewski 2020-03-19 16:49 ` [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts Bartosz Golaszewski 2020-03-19 17:12 ` Richard Purdie 3 siblings, 0 replies; 8+ messages in thread From: Bartosz Golaszewski @ 2020-03-19 16:44 UTC (permalink / raw) To: Khem Raj, Richard Purdie, Armin Kuster, Jerome Neanne, Quentin Schulz Cc: Bartosz Golaszewski, openembedded-core From: Bartosz Golaszewski <bgolaszewski@baylibre.com> Currently the artifacts for all image types are deployed to the shared space at the same time by the do_image_deploy task. This however creates a problem with circular dependencies if we want to use certain security features[1]. Because wic is designed to fetch artifacts generated by other recipes as well as other images generated by the same recipe it's useful to delay its creation and deployment until after do_image_complete. This patch adds a new variable: IMAGE_TYPES_DEPLOY_LATE which contains a list of image types for which the associated IMAGE_CMD tasks should be called after do_image_complete. The deployment is now done in two stages: before do_image_complete for all regular types and after for types listed in the new variable. This will allow us to fine tune the dependencies in order to implement dm-verity support where initramfs on which the main image depends needs to access the partition image before we create the wic image. [1] http://lists.openembedded.org/pipermail/openembedded-core/2020-March/294094.html Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> --- meta/classes/image.bbclass | 39 ++++++++++++++----- meta/classes/image_types.bbclass | 3 ++ meta/classes/image_types_wic.bbclass | 4 +- .../images/build-appliance-image_15.0.0.bb | 2 +- 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass index 6e2b864f73..7d0dd6ee50 100644 --- a/meta/classes/image.bbclass +++ b/meta/classes/image.bbclass @@ -83,6 +83,7 @@ export PACKAGE_INSTALL ?= "${IMAGE_INSTALL} ${ROOTFS_BOOTSTRAP_INSTALL} ${FEATUR PACKAGE_INSTALL_ATTEMPTONLY ?= "${FEATURE_INSTALL_OPTIONAL}" IMGDEPLOYDIR = "${WORKDIR}/deploy-${PN}-image-complete" +LATEIMGDEPLOYDIR = "${WORKDIR}/deploy-${PN}-image-complete-late" # Images are generally built explicitly, do not need to be part of world. EXCLUDE_FROM_WORLD = "1" @@ -127,7 +128,7 @@ def rootfs_variables(d): 'IMAGE_ROOTFS_MAXSIZE','IMAGE_NAME','IMAGE_LINK_NAME','IMAGE_MANIFEST','DEPLOY_DIR_IMAGE','IMAGE_FSTYPES','IMAGE_INSTALL_COMPLEMENTARY','IMAGE_LINGUAS', 'IMAGE_LINGUAS_COMPLEMENTARY', 'MULTILIBRE_ALLOW_REP','MULTILIB_TEMP_ROOTFS','MULTILIB_VARIANTS','MULTILIBS','ALL_MULTILIB_PACKAGE_ARCHS','MULTILIB_GLOBAL_VARIANTS','BAD_RECOMMENDATIONS','NO_RECOMMENDATIONS', 'PACKAGE_ARCHS','PACKAGE_CLASSES','TARGET_VENDOR','TARGET_ARCH','TARGET_OS','OVERRIDES','BBEXTENDVARIANT','FEED_DEPLOYDIR_BASE_URI','INTERCEPT_DIR','USE_DEVFS', - 'CONVERSIONTYPES', 'IMAGE_GEN_DEBUGFS', 'ROOTFS_RO_UNNEEDED', 'IMGDEPLOYDIR', 'PACKAGE_EXCLUDE_COMPLEMENTARY', 'REPRODUCIBLE_TIMESTAMP_ROOTFS', 'IMAGE_INSTALL_DEBUGFS'] + 'CONVERSIONTYPES', 'IMAGE_GEN_DEBUGFS', 'ROOTFS_RO_UNNEEDED', 'IMGDEPLOYDIR', 'LATEIMGDEPLOYDIR', 'PACKAGE_EXCLUDE_COMPLEMENTARY', 'REPRODUCIBLE_TIMESTAMP_ROOTFS', 'IMAGE_INSTALL_DEBUGFS'] variables.extend(rootfs_command_variables(d)) variables.extend(variable_depends(d)) return " ".join(variables) @@ -247,7 +248,7 @@ fakeroot python do_rootfs () { progress_reporter.finish() } do_rootfs[dirs] = "${TOPDIR}" -do_rootfs[cleandirs] += "${S} ${IMGDEPLOYDIR}" +do_rootfs[cleandirs] += "${S} ${IMGDEPLOYDIR} ${LATEIMGDEPLOYDIR}" do_rootfs[umask] = "022" do_rootfs[file-checksums] += "${POSTINST_INTERCEPT_CHECKSUMS}" addtask rootfs after do_prepare_recipe_sysroot @@ -273,7 +274,21 @@ SSTATETASKS += "do_image_deploy" do_image_deploy[sstate-inputdirs] = "${IMGDEPLOYDIR}" do_image_deploy[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}" SSTATE_SKIP_CREATION_task-image-deploy = '1' -addtask do_image_deploy after do_image before do_build +addtask do_image_deploy after do_image before do_image_complete + +do_image_deploy_late() { + # Avoid using SSTATE_DUPWHITELIST - check which images have already been + # deployed and copy those that haven't into a separate pre-deploy directory + # which will serve as the sstate input directory for this task. + for file in $(ls ${IMGDEPLOYDIR}) ; do + test -e ${DEPLOY_DIR_IMAGE}/$file || cp -a ${IMGDEPLOYDIR}/$file ${LATEIMGDEPLOYDIR}/$file + done +} +SSTATETASKS += "do_image_deploy_late" +do_image_deploy_late[sstate-inputdirs] = "${LATEIMGDEPLOYDIR}" +do_image_deploy_late[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}" +SSTATE_SKIP_CREATION_task-image-deploy-late = '1' +addtask do_image_deploy_late after do_image_complete before do_build fakeroot python do_image_complete () { from oe.utils import execute_pre_post_process @@ -285,7 +300,7 @@ fakeroot python do_image_complete () { do_image_complete[dirs] = "${TOPDIR}" do_image_complete[umask] = "022" do_image_complete[stamp-extra-info] = "${MACHINE_ARCH}" -addtask do_image_complete after do_image_deploy before do_build +addtask do_image_complete after do_image_deploy before do_image_deploy_late python do_image_complete_setscene () { sstate_setscene(d) } @@ -412,6 +427,7 @@ python () { maskedtypes = (d.getVar('IMAGE_TYPES_MASKED') or "").split() maskedtypes = [dbg + t for t in maskedtypes for dbg in ("", "debugfs_")] + latetypes = d.getVar('IMAGE_TYPES_DEPLOY_LATE').split() for t in basetypes: vardeps = set() @@ -491,9 +507,14 @@ python () { for image in sorted(rm_tmp_images): cmds.append("\trm " + image) - after = 'do_image' - for dep in typedeps[t]: - after += ' do_image_%s' % dep.replace("-", "_").replace(".", "_") + if t in latetypes: + before = 'do_image_deploy_late' + after = 'do_image_complete' + else: + before = 'do_image_deploy' + after = 'do_image' + for dep in typedeps[t]: + after += ' do_image_%s' % dep.replace("-", "_").replace(".", "_") task = "do_image_%s" % t.replace("-", "_").replace(".", "_") @@ -507,8 +528,8 @@ python () { d.appendVarFlag(task, 'vardeps', ' ' + ' '.join(vardeps)) d.appendVarFlag(task, 'vardepsexclude', ' DATETIME DATE ' + ' '.join(vardepsexclude)) - bb.debug(2, "Adding task %s before do_image_deploy, after %s" % (task, after)) - bb.build.addtask(task, 'do_image_deploy', after, d) + bb.debug(2, "Adding task %s before %s, after %s" % (task, before, after)) + bb.build.addtask(task, before, after, d) } # diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass index f82f1d8862..665bd7c4b3 100644 --- a/meta/classes/image_types.bbclass +++ b/meta/classes/image_types.bbclass @@ -331,5 +331,8 @@ DEPLOYABLE_IMAGE_TYPES ?= "hddimg iso" # images that will not be built at do_rootfs time: vmdk, vdi, qcow2, hddimg, iso, etc. IMAGE_TYPES_MASKED ?= "" +# Image types that should be generated and deployed after do_image_complete task. +IMAGE_TYPES_DEPLOY_LATE ?= "wic" + # bmap requires python3 to be in the PATH EXTRANATIVEPATH += "${@'python3-native' if d.getVar('IMAGE_FSTYPES').find('.bmap') else ''}" diff --git a/meta/classes/image_types_wic.bbclass b/meta/classes/image_types_wic.bbclass index b83308b45c..80039ed19c 100644 --- a/meta/classes/image_types_wic.bbclass +++ b/meta/classes/image_types_wic.bbclass @@ -113,7 +113,7 @@ python () { # a variable and let the metadata deal with the deps. d.setVar('_WKS_TEMPLATE', body) bb.build.addtask('do_write_wks_template', 'do_image_wic', 'do_image', d) - bb.build.addtask('do_image_wic', 'do_image_complete', None, d) + bb.build.addtask('do_image_wic', None, 'do_image_complete', d) } # @@ -139,6 +139,6 @@ python do_rootfs_wicenv () { depdir = d.getVar('IMGDEPLOYDIR') bb.utils.copyfile(os.path.join(outdir, basename) + '.env', os.path.join(depdir, basename) + '.env') } -addtask do_rootfs_wicenv after do_image before do_image_wic +addtask do_rootfs_wicenv after do_image_complete before do_image_wic do_rootfs_wicenv[vardeps] += "${WICVARS}" do_rootfs_wicenv[prefuncs] = 'set_image_size' diff --git a/meta/recipes-core/images/build-appliance-image_15.0.0.bb b/meta/recipes-core/images/build-appliance-image_15.0.0.bb index 8c9fe92485..5ec66ebd76 100644 --- a/meta/recipes-core/images/build-appliance-image_15.0.0.bb +++ b/meta/recipes-core/images/build-appliance-image_15.0.0.bb @@ -138,4 +138,4 @@ python do_bundle_files() { bb.build.exec_func('create_bundle_files', d) } -addtask bundle_files after do_image_wic before do_image_complete +addtask bundle_files after do_image_wic -- 2.19.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts 2020-03-19 16:44 [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts Bartosz Golaszewski 2020-03-19 16:44 ` [RFC PATCH 1/2] image.bbclass: add an intermediate deploy task Bartosz Golaszewski 2020-03-19 16:44 ` [RFC PATCH 2/2] image.bbclass: deploy artifacts in two stages Bartosz Golaszewski @ 2020-03-19 16:49 ` Bartosz Golaszewski 2020-03-19 17:12 ` Richard Purdie 3 siblings, 0 replies; 8+ messages in thread From: Bartosz Golaszewski @ 2020-03-19 16:49 UTC (permalink / raw) To: Khem Raj, Richard Purdie, Armin Kuster, Jerome Neanne, Quentin Schulz Cc: Bartosz Golaszewski, Patches and discussions about the oe-core layer czw., 19 mar 2020 o 17:44 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a): > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > This is a follow-up to the discussion I started on the OE-core mailing > list a couple days ago[1]. These patches propose to split the deployment > of image artifacts into two stages where the first one includes all > "regular" images and takes place before do_image_complete and the second > is mostly aimed at wic right now and happens after do_image_complete. > > These patches work but I'm sending them as RFC mostly to continue the > discussion about possible solutions for the circular dependencies between > the rootfs and initramfs. > > [1] http://lists.openembedded.org/pipermail/openembedded-core/2020-March/294094.html > > Bartosz Golaszewski (2): > image.bbclass: add an intermediate deploy task > image.bbclass: deploy artifacts in two stages > > meta/classes/image.bbclass | 54 ++++++++++++++----- > meta/classes/image_types.bbclass | 3 ++ > meta/classes/image_types_wic.bbclass | 4 +- > .../images/build-appliance-image_15.0.0.bb | 2 +- > 4 files changed, 47 insertions(+), 16 deletions(-) > > -- > 2.19.1 > Just for clarity: this what the task order would look like for a standard beaglebone-yocto image with these patches: do_prepare_recipe_sysroot (32199): log.do_prepare_recipe_sysroot.32199 do_deploy_source_date_epoch (32201): log.do_deploy_source_date_epoch.32201 do_rootfs (32246): log.do_rootfs.32246 do_write_qemuboot_conf (8655): log.do_write_qemuboot_conf.8655 do_image_qa (8657): log.do_image_qa.8657 do_image (8661): log.do_image.8661 do_image_ext4 (35128): log.do_image_ext4.35128 do_image_tar (35133): log.do_image_tar.35133 do_image_jffs2 (35136): log.do_image_jffs2.35136 do_image_deploy (35246): log.do_image_deploy.35246 do_image_complete (35262): log.do_image_complete.35262 do_rootfs_wicenv (35264): log.do_rootfs_wicenv.35264 do_populate_lic_deploy (35267): log.do_populate_lic_deploy.35267 do_image_wic (35283): log.do_image_wic.35283 do_image_deploy_late (35448): log.do_image_deploy_late.35448 Bart ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts 2020-03-19 16:44 [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts Bartosz Golaszewski ` (2 preceding siblings ...) 2020-03-19 16:49 ` [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts Bartosz Golaszewski @ 2020-03-19 17:12 ` Richard Purdie 2020-03-19 18:20 ` Bartosz Golaszewski 3 siblings, 1 reply; 8+ messages in thread From: Richard Purdie @ 2020-03-19 17:12 UTC (permalink / raw) To: Bartosz Golaszewski, Khem Raj, Armin Kuster, Jerome Neanne, Quentin Schulz Cc: Bartosz Golaszewski, openembedded-core On Thu, 2020-03-19 at 17:44 +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > This is a follow-up to the discussion I started on the OE-core > mailing > list a couple days ago[1]. These patches propose to split the > deployment > of image artifacts into two stages where the first one includes all > "regular" images and takes place before do_image_complete and the > second > is mostly aimed at wic right now and happens after do_image_complete. > > These patches work but I'm sending them as RFC mostly to continue the > discussion about possible solutions for the circular dependencies > between > the rootfs and initramfs. > > [1] > http://lists.openembedded.org/pipermail/openembedded-core/2020-March/294094.html This works fine until we have some new image type which then has to depend on happening after wic. We then add a three stage process and so on. Basically this feels like we're hardcoding something for one specific use case which will later break and not scale to other problems/solutions. Sorry, I'm not convinced this is the right way to move forward. I will try and have a think about what the right way is but sadly I don't get much time to spend on specific problems like this :( Cheers, Richard ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts 2020-03-19 17:12 ` Richard Purdie @ 2020-03-19 18:20 ` Bartosz Golaszewski 2020-03-19 23:38 ` Richard Purdie 0 siblings, 1 reply; 8+ messages in thread From: Bartosz Golaszewski @ 2020-03-19 18:20 UTC (permalink / raw) To: Richard Purdie Cc: Jerome Neanne, Patches and discussions about the oe-core layer, Bartosz Golaszewski, Quentin Schulz czw., 19 mar 2020 o 18:13 Richard Purdie <richard.purdie@linuxfoundation.org> napisał(a): > > On Thu, 2020-03-19 at 17:44 +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > This is a follow-up to the discussion I started on the OE-core > > mailing > > list a couple days ago[1]. These patches propose to split the > > deployment > > of image artifacts into two stages where the first one includes all > > "regular" images and takes place before do_image_complete and the > > second > > is mostly aimed at wic right now and happens after do_image_complete. > > > > These patches work but I'm sending them as RFC mostly to continue the > > discussion about possible solutions for the circular dependencies > > between > > the rootfs and initramfs. > > > > [1] > > http://lists.openembedded.org/pipermail/openembedded-core/2020-March/294094.html > > This works fine until we have some new image type which then has to > depend on happening after wic. We then add a three stage process and so > on. Basically this feels like we're hardcoding something for one > specific use case which will later break and not scale to other > problems/solutions. > I understand you see it as controversial but let me try to convince you: If we'll really get to a point where we need to create a hierarchy of multiple levels of image deployment, then maybe we should switch to deploying each image right after it is created in its dedicated do_image_<type> task, then we could really fine-tune the inter-task dependencies. But we're not there yet and IMO currently it would be overkill. The rule of thumb for me is not to add new interfaces nobody asks for. The thing with wic is that it's aggregating other deployed images. What we're dealing with are *two* categories of image artifacts: let's call them simple and composed types. Obviously composed types may need to access the simple types and, due to the use-case I described in my previous thread, it's useful if we can depend on the tasks for simple and composed images separately. Maybe if I renamed the tasks: do_image_deploy_simple and do_image_deploy_composed it would look better than an ambiguous do_image_deploy_late. This also makes your argument about adding a third category purely theoretical: I don't see how we would need another category of images anytime soon. Even if we added another aggregating type (one could argue that hddimg qualifies as such) it could, with a 99% chance, run in parallel to wic and thus qualify as a deploy_late/composed image. > Sorry, I'm not convinced this is the right way to move forward. I will > try and have a think about what the right way is but sadly I don't get > much time to spend on specific problems like this :( > Please do - I'm open for other ideas. Just please keep in mind: there's obviously a problem with the current approach. I described it in detail and it turned out Quentin had encountered it too and dealt with it using a nasty workaround (fitImage deployed by the image recipe and not the kernel). I'd like to fix it upstream and proposed a solution that is IMO not horrible. I'd appreciate if we could reach a compromise that wouldn't leave us with downstream hacks. Please also note that sometimes "gets the job done" really is better than not solving problems. Just look at initcall levels in the linux kernel - they are similar to what I proposed here and serve as a surrogate for real dependencies. Best regards, Bartosz Golaszewski ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts 2020-03-19 18:20 ` Bartosz Golaszewski @ 2020-03-19 23:38 ` Richard Purdie 2020-03-20 13:11 ` Bartosz Golaszewski 0 siblings, 1 reply; 8+ messages in thread From: Richard Purdie @ 2020-03-19 23:38 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Jerome Neanne, Patches and discussions about the oe-core layer, Bartosz Golaszewski, Quentin Schulz On Thu, 2020-03-19 at 19:20 +0100, Bartosz Golaszewski wrote: > If we'll really get to a point where we need to create a hierarchy of > multiple levels of image deployment, then maybe we should switch to > deploying each image right after it is created in its dedicated > do_image_<type> task, then we could really fine-tune the inter-task > dependencies. But we're not there yet and IMO currently it would be > overkill. The rule of thumb for me is not to add new interfaces > nobody asks for. Going back in time I'm the person who split the image pieces off from do_rootfs and split the different image commands into different tasks. I did this for several reasons but one was we were developing new task dependency code inside the image construction which was just crazy. I was wondering about the option of having the image task deploy the image earlier. I think that could be made to work. I think conceptually it makes more sense architecturally than adding in arbitrary new task levels to the existing structure. > This also makes your argument about adding a third category purely > theoretical: I don't see how we would need another category of images > anytime soon. I often think things like that but then new use cases come along... > Please do - I'm open for other ideas. Just please keep in mind: > there's obviously a problem with the current approach. I described it > in detail and it turned out Quentin had encountered it too and dealt > with it using a nasty workaround (fitImage deployed by the image > recipe and not the kernel). I'd like to fix it upstream and proposed > a solution that is IMO not horrible. I'd appreciate if we could reach > a compromise that wouldn't leave us with downstream hacks. I do understand there is a problem. We also have problems with people not understanding the code as it works today. Your proposal adds something else with is hard to explain to users ("Is my image simple or composed?") and whilst we can and do add complexity where we need to, I'm not yet convinced this change makes enough sense to have it. Changing the way image deployment works would in many ways be much easier for people to understand and makes the system more flexible without adding problematic terminology. > Please also note that sometimes "gets the job done" really is better > than not solving problems. Just look at initcall levels in the linux > kernel - they are similar to what I proposed here and serve as a > surrogate for real dependencies. We have piles of "get the job done" and also need to sometimes take a step back and see if we can do something better. Right now I think your proposal makes things worse and will be hard to explain to people. My instinct is we can do better here. Cheers, Richard ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts 2020-03-19 23:38 ` Richard Purdie @ 2020-03-20 13:11 ` Bartosz Golaszewski 0 siblings, 0 replies; 8+ messages in thread From: Bartosz Golaszewski @ 2020-03-20 13:11 UTC (permalink / raw) To: Richard Purdie Cc: Jerome Neanne, Patches and discussions about the oe-core layer, Bartosz Golaszewski, Quentin Schulz pt., 20 mar 2020 o 00:38 Richard Purdie <richard.purdie@linuxfoundation.org> napisał(a): > > On Thu, 2020-03-19 at 19:20 +0100, Bartosz Golaszewski wrote: > > If we'll really get to a point where we need to create a hierarchy of > > multiple levels of image deployment, then maybe we should switch to > > deploying each image right after it is created in its dedicated > > do_image_<type> task, then we could really fine-tune the inter-task > > dependencies. But we're not there yet and IMO currently it would be > > overkill. The rule of thumb for me is not to add new interfaces > > nobody asks for. > > Going back in time I'm the person who split the image pieces off from > do_rootfs and split the different image commands into different tasks. > I did this for several reasons but one was we were developing new task > dependency code inside the image construction which was just crazy. > > I was wondering about the option of having the image task deploy the > image earlier. I think that could be made to work. I think conceptually > it makes more sense architecturally than adding in arbitrary new task > levels to the existing structure. > > > This also makes your argument about adding a third category purely > > theoretical: I don't see how we would need another category of images > > anytime soon. > > I often think things like that but then new use cases come along... > > > Please do - I'm open for other ideas. Just please keep in mind: > > there's obviously a problem with the current approach. I described it > > in detail and it turned out Quentin had encountered it too and dealt > > with it using a nasty workaround (fitImage deployed by the image > > recipe and not the kernel). I'd like to fix it upstream and proposed > > a solution that is IMO not horrible. I'd appreciate if we could reach > > a compromise that wouldn't leave us with downstream hacks. > > I do understand there is a problem. We also have problems with people > not understanding the code as it works today. Your proposal adds > something else with is hard to explain to users ("Is my image simple or > composed?") and whilst we can and do add complexity where we need to, > I'm not yet convinced this change makes enough sense to have it. > > Changing the way image deployment works would in many ways be much > easier for people to understand and makes the system more flexible > without adding problematic terminology. > > > Please also note that sometimes "gets the job done" really is better > > than not solving problems. Just look at initcall levels in the linux > > kernel - they are similar to what I proposed here and serve as a > > surrogate for real dependencies. > > We have piles of "get the job done" and also need to sometimes take a > step back and see if we can do something better. Right now I think your > proposal makes things worse and will be hard to explain to people. My > instinct is we can do better here. > Fair enough. I don't quite agree and I'd like to hear other voices too but I also don't want to waste time arguing either. As I'm afraid that if I don't follow up on this myself, it will simply be forgotten, I'm offering to spend some time figuring out per-task deployment. I started looking into it and figured that ideally we could have each image task use a separate local-deploy directory and make them all part of SSTATETASKS. Unfortunately a lot of places reference the IMGDEPLOYDIR variable so it probably has to stay in some form. I couldn't find any info on that - does bitbake offer anything like "per-task variables"? Variables that expand to different values depending on the task that calls them? Otherwise we could replace IMGDEPLOYDIR calls with some helper function that would return a different value depending on BB_CURRENTTASK? Any advice on technicalities is welcome. Bart ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-20 13:12 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-19 16:44 [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts Bartosz Golaszewski 2020-03-19 16:44 ` [RFC PATCH 1/2] image.bbclass: add an intermediate deploy task Bartosz Golaszewski 2020-03-19 16:44 ` [RFC PATCH 2/2] image.bbclass: deploy artifacts in two stages Bartosz Golaszewski 2020-03-19 16:49 ` [RFC PATCH 0/2] image.bbclass: support two-stage deployment of image artifacts Bartosz Golaszewski 2020-03-19 17:12 ` Richard Purdie 2020-03-19 18:20 ` Bartosz Golaszewski 2020-03-19 23:38 ` Richard Purdie 2020-03-20 13:11 ` Bartosz Golaszewski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.