* [PATCH 0/3] Fix data-race in deployment of initrd
@ 2023-02-17 10:01 Felix Moessbauer
2023-02-17 10:01 ` [PATCH 1/3] add initramfs to sstate-cache Felix Moessbauer
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Felix Moessbauer @ 2023-02-17 10:01 UTC (permalink / raw)
To: isar-users; +Cc: adriaan.schmidt, jan.kiszka, Felix Moessbauer
This patch fixes a data race in the deployment of the initrd, as discussed on
the ML in "Issues creating images with custom initramfs". As a measure against
further races on these files, we also use the sstate cache for deployment.
Please note, that imaging with wic + grub or systemd-boot with a custom
initramfs is still broken. This is due to the wic logic, which reads the
initrd in the rootfs, but not the one in the deploy dir.
Best regards,
Felix Moessbauer
Siemens AG
Felix Moessbauer (3):
add initramfs to sstate-cache
deploy boot files via sstate-cache
fix race-cond between default and custom initrd
meta/classes/image.bbclass | 38 ++++++++++++++++++++++------------
meta/classes/initramfs.bbclass | 18 +++++++++++-----
scripts/start_vm | 4 ++--
testsuite/start_vm.py | 2 +-
4 files changed, 41 insertions(+), 21 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/3] add initramfs to sstate-cache 2023-02-17 10:01 [PATCH 0/3] Fix data-race in deployment of initrd Felix Moessbauer @ 2023-02-17 10:01 ` Felix Moessbauer 2023-02-17 10:01 ` [PATCH 2/3] deploy boot files via sstate-cache Felix Moessbauer 2023-02-17 10:01 ` [PATCH 3/3] fix race-cond between default and custom initrd Felix Moessbauer 2 siblings, 0 replies; 6+ messages in thread From: Felix Moessbauer @ 2023-02-17 10:01 UTC (permalink / raw) To: isar-users; +Cc: adriaan.schmidt, jan.kiszka, Felix Moessbauer This patch changes how we deploy the initramfs. Instead of manually deploying, we use the sstate infrastructure for that. By that, accidental overrides of the artifact can be automatically detected. On clean, the artifact is also cleaned. Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com> --- meta/classes/initramfs.bbclass | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/meta/classes/initramfs.bbclass b/meta/classes/initramfs.bbclass index a5141a53..183d1933 100644 --- a/meta/classes/initramfs.bbclass +++ b/meta/classes/initramfs.bbclass @@ -2,16 +2,19 @@ # Make workdir and stamps machine-specific without changing common PN target WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}-${MACHINE}/${PV}-${PR}" +DEPLOYDIR = "${WORKDIR}/deploy" STAMP = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}-${MACHINE}/${PV}-${PR}" STAMPCLEAN = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}-${MACHINE}/*-*" # Sstate also needs to be machine-specific SSTATE_MANIFESTS = "${TMPDIR}/sstate-control/${MACHINE}-${DISTRO}-${DISTRO_ARCH}" +SSTATETASKS += "do_generate_initramfs" INITRAMFS_INSTALL ?= "" INITRAMFS_PREINSTALL ?= "" INITRAMFS_ROOTFS ?= "${WORKDIR}/rootfs" -INITRAMFS_IMAGE_FILE = "${DEPLOY_DIR_IMAGE}/${INITRAMFS_FULLNAME}.initrd.img" +INITRAMFS_IMAGE_NAME = "${INITRAMFS_FULLNAME}.initrd.img" +INITRAMFS_IMAGE_FILE = "${DEPLOY_DIR_IMAGE}/${INITRAMFS_IMAGE_NAME}" # Install proper kernel INITRAMFS_INSTALL += "${@ ("linux-image-" + d.getVar("KERNEL_NAME", True)) if d.getVar("KERNEL_NAME", True) else ""}" @@ -27,8 +30,10 @@ ROOTFS_PACKAGES = "initramfs-tools ${INITRAMFS_PREINSTALL} ${INITRAMFS_INSTALL}" inherit rootfs -do_generate_initramfs[dirs] = "${DEPLOY_DIR_IMAGE}" do_generate_initramfs[network] = "${TASK_USE_SUDO}" +do_generate_initramfs[cleandirs] += "${DEPLOYDIR}" +do_generate_initramfs[sstate-inputdirs] = "${DEPLOYDIR}" +do_generate_initramfs[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}" do_generate_initramfs() { rootfs_do_mounts rootfs_do_qemu @@ -44,8 +49,11 @@ do_generate_initramfs() { if [ ! -e "${INITRAMFS_ROOTFS}/initrd.img" ]; then die "No initramfs was found after generation!" fi - - rm -rf "${INITRAMFS_IMAGE_FILE}" - cp "${INITRAMFS_ROOTFS}/initrd.img" "${INITRAMFS_IMAGE_FILE}" + cp ${INITRAMFS_ROOTFS}/initrd.img ${DEPLOYDIR}/${INITRAMFS_IMAGE_NAME} } addtask generate_initramfs after do_rootfs before do_build + +python do_generate_initramfs_setscene () { + sstate_setscene(d) +} +addtask do_generate_initramfs_setscene -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] deploy boot files via sstate-cache 2023-02-17 10:01 [PATCH 0/3] Fix data-race in deployment of initrd Felix Moessbauer 2023-02-17 10:01 ` [PATCH 1/3] add initramfs to sstate-cache Felix Moessbauer @ 2023-02-17 10:01 ` Felix Moessbauer 2023-02-17 10:01 ` [PATCH 3/3] fix race-cond between default and custom initrd Felix Moessbauer 2 siblings, 0 replies; 6+ messages in thread From: Felix Moessbauer @ 2023-02-17 10:01 UTC (permalink / raw) To: isar-users; +Cc: adriaan.schmidt, jan.kiszka, Felix Moessbauer This patch changes how we deploy the boot files. Instead of manually deploying, we use the sstate infrastructure for that. By that, accidental overrides of the artifacts can be automatically detected. On clean, the artifacts are also cleaned. Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com> --- meta/classes/image.bbclass | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass index ef7d5a2a..e799d1d4 100644 --- a/meta/classes/image.bbclass +++ b/meta/classes/image.bbclass @@ -3,11 +3,13 @@ # Make workdir and stamps machine-specific without changing common PN target WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}-${MACHINE}/${PV}-${PR}" +DEPLOYDIR = "${WORKDIR}/deploy-${PN}" STAMP = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}-${MACHINE}/${PV}-${PR}" STAMPCLEAN = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}-${MACHINE}/*-*" # Sstate also needs to be machine-specific SSTATE_MANIFESTS = "${TMPDIR}/sstate-control/${MACHINE}-${DISTRO}-${DISTRO_ARCH}" +SSTATETASKS += "do_copy_boot_files" IMAGE_INSTALL ?= "" IMAGE_FSTYPES ?= "${@ d.getVar("IMAGE_TYPE", True) if d.getVar("IMAGE_TYPE", True) else "ext4"}" @@ -355,8 +357,9 @@ INITRD_IMG = "${PP_DEPLOY}/${INITRD_IMAGE}" # only one dtb file supported, pick the first DTB_IMG = "${PP_DEPLOY}/${@(d.getVar('DTB_FILES').split() or [''])[0]}" -do_copy_boot_files[dirs] = "${DEPLOY_DIR_IMAGE}" -do_copy_boot_files[lockfiles] += "${DEPLOY_DIR_IMAGE}/isar.lock" +do_copy_boot_files[cleandirs] += "${DEPLOYDIR}" +do_copy_boot_files[sstate-inputdirs] = "${DEPLOYDIR}" +do_copy_boot_files[sstate-outputdirs] = "${DEPLOY_DIR_IMAGE}" do_copy_boot_files[network] = "${TASK_USE_SUDO}" do_copy_boot_files() { kernel="$(realpath -q '${IMAGE_ROOTFS}'/vmlinu[xz])" @@ -364,7 +367,7 @@ do_copy_boot_files() { kernel="$(realpath -q '${IMAGE_ROOTFS}'/boot/vmlinu[xz])" fi if [ -f "$kernel" ]; then - sudo cat "$kernel" > "${DEPLOY_DIR_IMAGE}/${KERNEL_IMAGE}" + sudo cat "$kernel" > "${DEPLOYDIR}/${KERNEL_IMAGE}" fi initrd="$(realpath -q '${IMAGE_ROOTFS}/initrd.img')" @@ -372,7 +375,7 @@ do_copy_boot_files() { initrd="$(realpath -q '${IMAGE_ROOTFS}/boot/initrd.img')" fi if [ -f "$initrd" ]; then - cp -f "$initrd" '${DEPLOY_DIR_IMAGE}/${INITRD_IMAGE}' + cp -f "$initrd" '${DEPLOYDIR}/${INITRD_IMAGE}' fi for file in ${DTB_FILES}; do @@ -383,11 +386,16 @@ do_copy_boot_files() { die "${file} not found" fi - cp -f "$dtb" "${DEPLOY_DIR_IMAGE}/" + cp -f "$dtb" "${DEPLOYDIR}/" done } addtask copy_boot_files before do_rootfs_postprocess after do_rootfs_install +python do_copy_boot_files_setscene () { + sstate_setscene(d) +} +addtask do_copy_boot_files_setscene + python do_image_tools() { """Virtual task""" pass -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] fix race-cond between default and custom initrd 2023-02-17 10:01 [PATCH 0/3] Fix data-race in deployment of initrd Felix Moessbauer 2023-02-17 10:01 ` [PATCH 1/3] add initramfs to sstate-cache Felix Moessbauer 2023-02-17 10:01 ` [PATCH 2/3] deploy boot files via sstate-cache Felix Moessbauer @ 2023-02-17 10:01 ` Felix Moessbauer 2023-02-17 11:55 ` Jan Kiszka 2 siblings, 1 reply; 6+ messages in thread From: Felix Moessbauer @ 2023-02-17 10:01 UTC (permalink / raw) To: isar-users; +Cc: adriaan.schmidt, jan.kiszka, Felix Moessbauer This patch fixes a data race happening when building a custom initrd. Previously, both custom and default initrds were deployed to the image deploy dir. The race is fixed by conditionally deploying either the custom or the default one. For that, we introduce a new variable INITRD_DEPLOY_FILE which provides the name of the initrd in the deploy directory. The existing INITRD_IMAGE variable is defaulted to the empty string and used to control if a custom initrd is requrested. Only if this variable is empty, the default one is deployed. Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com> --- meta/classes/image.bbclass | 24 ++++++++++++++---------- scripts/start_vm | 4 ++-- testsuite/start_vm.py | 2 +- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass index e799d1d4..0d5a521e 100644 --- a/meta/classes/image.bbclass +++ b/meta/classes/image.bbclass @@ -3,7 +3,7 @@ # Make workdir and stamps machine-specific without changing common PN target WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}-${MACHINE}/${PV}-${PR}" -DEPLOYDIR = "${WORKDIR}/deploy-${PN}" +DEPLOYDIR = "${WORKDIR}/deploy" STAMP = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}-${MACHINE}/${PV}-${PR}" STAMPCLEAN = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}-${MACHINE}/*-*" @@ -22,8 +22,9 @@ IMAGE_INSTALL += "${KERNEL_IMAGE_PKG}" IMAGE_FULLNAME = "${PN}-${DISTRO}-${MACHINE}" # These variables are used by wic and start_vm -KERNEL_IMAGE ?= "${IMAGE_FULLNAME}-${KERNEL_FILE}" -INITRD_IMAGE ?= "${IMAGE_FULLNAME}-initrd.img" +KERNEL_IMAGE = "${IMAGE_FULLNAME}-${KERNEL_FILE}" +INITRD_IMAGE ?= "" +INITRD_DEPLOY_FILE = "${@d.getVar('INITRD_IMAGE') or '${IMAGE_FULLNAME}-initrd.img')" # This defines the deployed dtbs for reuse by imagers DTB_FILES ?= "" @@ -353,7 +354,7 @@ EOF # Default kernel, initrd and dtb image deploy paths (inside imager) KERNEL_IMG = "${PP_DEPLOY}/${KERNEL_IMAGE}" -INITRD_IMG = "${PP_DEPLOY}/${INITRD_IMAGE}" +INITRD_IMG = "${PP_DEPLOY}/${INITRD_DEPLOY_FILE}" # only one dtb file supported, pick the first DTB_IMG = "${PP_DEPLOY}/${@(d.getVar('DTB_FILES').split() or [''])[0]}" @@ -370,12 +371,15 @@ do_copy_boot_files() { sudo cat "$kernel" > "${DEPLOYDIR}/${KERNEL_IMAGE}" fi - initrd="$(realpath -q '${IMAGE_ROOTFS}/initrd.img')" - if [ ! -f "$initrd" ]; then - initrd="$(realpath -q '${IMAGE_ROOTFS}/boot/initrd.img')" - fi - if [ -f "$initrd" ]; then - cp -f "$initrd" '${DEPLOYDIR}/${INITRD_IMAGE}' + # deploy default initrd if no custom one is build + if [ -e "${INITRD_IMAGE}" ]; then + initrd="$(realpath -q '${IMAGE_ROOTFS}/initrd.img')" + if [ ! -f "$initrd" ]; then + initrd="$(realpath -q '${IMAGE_ROOTFS}/boot/initrd.img')" + fi + if [ -f "$initrd" ]; then + cp -f "$initrd" '${DEPLOYDIR}/${INITRD_DEPLOY_FILE}' + fi fi for file in ${DTB_FILES}; do diff --git a/scripts/start_vm b/scripts/start_vm index 17091d72..8c696a4a 100755 --- a/scripts/start_vm +++ b/scripts/start_vm @@ -125,10 +125,10 @@ case "$IMAGE_FSTYPES" in readonly ROOTFS_IMAGE=$IMAGE_FULLNAME.ext4 eval $(bitbake -e mc:qemu$ARCH-$DISTRO:isar-image-base | grep "^KERNEL_IMAGE=") - eval $(bitbake -e mc:qemu$ARCH-$DISTRO:isar-image-base | grep "^INITRD_IMAGE=") + eval $(bitbake -e mc:qemu$ARCH-$DISTRO:isar-image-base | grep "^INITRD_DEPLOY_FILE=") QKERNEL=$IMAGE_DIR/${KERNEL_IMAGE} QINITRD=/dev/null - [ -n "$INITRD_IMAGE" ] && QINITRD=$IMAGE_DIR/${INITRD_IMAGE} + [ -n "$INITRD_DEPLOY_FILE" ] && QINITRD=$IMAGE_DIR/${INITRD_DEPLOY_FILE} if [ "$ARCH" = "riscv64" ]; then EXTRA_ARGS="$EXTRA_ARGS -device loader,file=$QKERNEL,addr=0x80200000" QKERNEL="/usr/lib/riscv64-linux-gnu/opensbi/qemu/virt/fw_jump.elf" diff --git a/testsuite/start_vm.py b/testsuite/start_vm.py index 82ecc17d..ba1ba127 100755 --- a/testsuite/start_vm.py +++ b/testsuite/start_vm.py @@ -35,7 +35,7 @@ def format_qemu_cmdline(arch, build, distro, out, pid, enforce_pcbios=False): if image_type == 'ext4': rootfs_image = 'isar-image-base-' + base + '-' + distro + '-qemu' + arch + '.ext4' kernel_image = deploy_dir_image + '/' + get_bitbake_var(bb_output, 'KERNEL_IMAGE') - initrd_image = get_bitbake_var(bb_output, 'INITRD_IMAGE') + initrd_image = get_bitbake_var(bb_output, 'INITRD_DEPLOY_FILE') if not initrd_image: initrd_image = '/dev/null' -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] fix race-cond between default and custom initrd 2023-02-17 10:01 ` [PATCH 3/3] fix race-cond between default and custom initrd Felix Moessbauer @ 2023-02-17 11:55 ` Jan Kiszka 2023-02-18 10:28 ` Moessbauer, Felix 0 siblings, 1 reply; 6+ messages in thread From: Jan Kiszka @ 2023-02-17 11:55 UTC (permalink / raw) To: Felix Moessbauer, isar-users; +Cc: adriaan.schmidt On 17.02.23 11:01, Felix Moessbauer wrote: > This patch fixes a data race happening when building a custom initrd. > Previously, both custom and default initrds were deployed to the image > deploy dir. The race is fixed by conditionally deploying either the > custom or the default one. For that, we introduce a new variable > INITRD_DEPLOY_FILE which provides the name of the initrd in the deploy > directory. The existing INITRD_IMAGE variable is defaulted to the empty > string and used to control if a custom initrd is requrested. Only if > this variable is empty, the default one is deployed. > So, if I understand this correctly, the whole change comes effectively without a change visible at recipe level IF users already overwrote INITRD_IMAGE (like isar-cip-core did). Still, this nicely enhanced or clarified semantic of INITRD_IMAGE should be documented as recipe-api change. And then we are lacking an in-tree test case for such a scenario. Some WIC image that consumes a custom initramfs. We only have mc:qemuamd64-bullseye:isar-initramfs in the test cases. Not a must-have to move forward with this improvements, but likely an important follow-up topic. Jan > Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com> > --- > meta/classes/image.bbclass | 24 ++++++++++++++---------- > scripts/start_vm | 4 ++-- > testsuite/start_vm.py | 2 +- > 3 files changed, 17 insertions(+), 13 deletions(-) > > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass > index e799d1d4..0d5a521e 100644 > --- a/meta/classes/image.bbclass > +++ b/meta/classes/image.bbclass > @@ -3,7 +3,7 @@ > > # Make workdir and stamps machine-specific without changing common PN target > WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}-${MACHINE}/${PV}-${PR}" > -DEPLOYDIR = "${WORKDIR}/deploy-${PN}" > +DEPLOYDIR = "${WORKDIR}/deploy" > STAMP = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}-${MACHINE}/${PV}-${PR}" > STAMPCLEAN = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}-${MACHINE}/*-*" > > @@ -22,8 +22,9 @@ IMAGE_INSTALL += "${KERNEL_IMAGE_PKG}" > IMAGE_FULLNAME = "${PN}-${DISTRO}-${MACHINE}" > > # These variables are used by wic and start_vm > -KERNEL_IMAGE ?= "${IMAGE_FULLNAME}-${KERNEL_FILE}" > -INITRD_IMAGE ?= "${IMAGE_FULLNAME}-initrd.img" > +KERNEL_IMAGE = "${IMAGE_FULLNAME}-${KERNEL_FILE}" > +INITRD_IMAGE ?= "" > +INITRD_DEPLOY_FILE = "${@d.getVar('INITRD_IMAGE') or '${IMAGE_FULLNAME}-initrd.img')" > > # This defines the deployed dtbs for reuse by imagers > DTB_FILES ?= "" > @@ -353,7 +354,7 @@ EOF > > # Default kernel, initrd and dtb image deploy paths (inside imager) > KERNEL_IMG = "${PP_DEPLOY}/${KERNEL_IMAGE}" > -INITRD_IMG = "${PP_DEPLOY}/${INITRD_IMAGE}" > +INITRD_IMG = "${PP_DEPLOY}/${INITRD_DEPLOY_FILE}" > # only one dtb file supported, pick the first > DTB_IMG = "${PP_DEPLOY}/${@(d.getVar('DTB_FILES').split() or [''])[0]}" > > @@ -370,12 +371,15 @@ do_copy_boot_files() { > sudo cat "$kernel" > "${DEPLOYDIR}/${KERNEL_IMAGE}" > fi > > - initrd="$(realpath -q '${IMAGE_ROOTFS}/initrd.img')" > - if [ ! -f "$initrd" ]; then > - initrd="$(realpath -q '${IMAGE_ROOTFS}/boot/initrd.img')" > - fi > - if [ -f "$initrd" ]; then > - cp -f "$initrd" '${DEPLOYDIR}/${INITRD_IMAGE}' > + # deploy default initrd if no custom one is build > + if [ -e "${INITRD_IMAGE}" ]; then > + initrd="$(realpath -q '${IMAGE_ROOTFS}/initrd.img')" > + if [ ! -f "$initrd" ]; then > + initrd="$(realpath -q '${IMAGE_ROOTFS}/boot/initrd.img')" > + fi > + if [ -f "$initrd" ]; then > + cp -f "$initrd" '${DEPLOYDIR}/${INITRD_DEPLOY_FILE}' > + fi > fi > > for file in ${DTB_FILES}; do > diff --git a/scripts/start_vm b/scripts/start_vm > index 17091d72..8c696a4a 100755 > --- a/scripts/start_vm > +++ b/scripts/start_vm > @@ -125,10 +125,10 @@ case "$IMAGE_FSTYPES" in > readonly ROOTFS_IMAGE=$IMAGE_FULLNAME.ext4 > > eval $(bitbake -e mc:qemu$ARCH-$DISTRO:isar-image-base | grep "^KERNEL_IMAGE=") > - eval $(bitbake -e mc:qemu$ARCH-$DISTRO:isar-image-base | grep "^INITRD_IMAGE=") > + eval $(bitbake -e mc:qemu$ARCH-$DISTRO:isar-image-base | grep "^INITRD_DEPLOY_FILE=") > QKERNEL=$IMAGE_DIR/${KERNEL_IMAGE} > QINITRD=/dev/null > - [ -n "$INITRD_IMAGE" ] && QINITRD=$IMAGE_DIR/${INITRD_IMAGE} > + [ -n "$INITRD_DEPLOY_FILE" ] && QINITRD=$IMAGE_DIR/${INITRD_DEPLOY_FILE} > if [ "$ARCH" = "riscv64" ]; then > EXTRA_ARGS="$EXTRA_ARGS -device loader,file=$QKERNEL,addr=0x80200000" > QKERNEL="/usr/lib/riscv64-linux-gnu/opensbi/qemu/virt/fw_jump.elf" > diff --git a/testsuite/start_vm.py b/testsuite/start_vm.py > index 82ecc17d..ba1ba127 100755 > --- a/testsuite/start_vm.py > +++ b/testsuite/start_vm.py > @@ -35,7 +35,7 @@ def format_qemu_cmdline(arch, build, distro, out, pid, enforce_pcbios=False): > if image_type == 'ext4': > rootfs_image = 'isar-image-base-' + base + '-' + distro + '-qemu' + arch + '.ext4' > kernel_image = deploy_dir_image + '/' + get_bitbake_var(bb_output, 'KERNEL_IMAGE') > - initrd_image = get_bitbake_var(bb_output, 'INITRD_IMAGE') > + initrd_image = get_bitbake_var(bb_output, 'INITRD_DEPLOY_FILE') > > if not initrd_image: > initrd_image = '/dev/null' -- Siemens AG, Technology Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] fix race-cond between default and custom initrd 2023-02-17 11:55 ` Jan Kiszka @ 2023-02-18 10:28 ` Moessbauer, Felix 0 siblings, 0 replies; 6+ messages in thread From: Moessbauer, Felix @ 2023-02-18 10:28 UTC (permalink / raw) To: isar-users, Kiszka, Jan, Gylstorff, Quirin, Schild, Henning Cc: Schmidt, Adriaan On Fri, 2023-02-17 at 12:55 +0100, Jan Kiszka wrote: > On 17.02.23 11:01, Felix Moessbauer wrote: > > This patch fixes a data race happening when building a custom > > initrd. > > Previously, both custom and default initrds were deployed to the > > image > > deploy dir. The race is fixed by conditionally deploying either the > > custom or the default one. For that, we introduce a new variable > > INITRD_DEPLOY_FILE which provides the name of the initrd in the > > deploy > > directory. The existing INITRD_IMAGE variable is defaulted to the > > empty > > string and used to control if a custom initrd is requrested. Only > > if > > this variable is empty, the default one is deployed. > > > > So, if I understand this correctly, the whole change comes > effectively > without a change visible at recipe level IF users already overwrote > INITRD_IMAGE (like isar-cip-core did). Still, this nicely enhanced or > clarified semantic of INITRD_IMAGE should be documented as recipe-api > change. Will add that in the v2. Yes, currently cip-core is not affected, but the efibootguard plugin should anyways be adapted to the new API, as otherwise images without custom initrd are broken (cc Quirin). > > And then we are lacking an in-tree test case for such a scenario. > Some > WIC image that consumes a custom initramfs. We only have > mc:qemuamd64-bullseye:isar-initramfs in the test cases. Not a must- > have > to move forward with this improvements, but likely an important > follow-up topic. Yes, but the custom initramfs case is still broken on sd-boot and grub. Unfortunately this is also not easy to fix, as ISAR does some deliberate diversions from the OE pattern in the wic plugins (e.g. get kernel and initrd from rootfs instead of deploy dir). My fear is that there is much more broken but currently nobody tests for this (e.g. the wic initrd parameter). Maybe Henning can have a look on how to fix that. Felix > > Jan > > > Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com> > > --- > > meta/classes/image.bbclass | 24 ++++++++++++++---------- > > scripts/start_vm | 4 ++-- > > testsuite/start_vm.py | 2 +- > > 3 files changed, 17 insertions(+), 13 deletions(-) > > > > diff --git a/meta/classes/image.bbclass > > b/meta/classes/image.bbclass > > index e799d1d4..0d5a521e 100644 > > --- a/meta/classes/image.bbclass > > +++ b/meta/classes/image.bbclass > > @@ -3,7 +3,7 @@ > > > > # Make workdir and stamps machine-specific without changing common > > PN target > > WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}- > > ${MACHINE}/${PV}-${PR}" > > -DEPLOYDIR = "${WORKDIR}/deploy-${PN}" > > +DEPLOYDIR = "${WORKDIR}/deploy" > > STAMP = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}- > > ${MACHINE}/${PV}-${PR}" > > STAMPCLEAN = "${STAMPS_DIR}/${DISTRO}-${DISTRO_ARCH}/${PN}- > > ${MACHINE}/*-*" > > > > @@ -22,8 +22,9 @@ IMAGE_INSTALL += "${KERNEL_IMAGE_PKG}" > > IMAGE_FULLNAME = "${PN}-${DISTRO}-${MACHINE}" > > > > # These variables are used by wic and start_vm > > -KERNEL_IMAGE ?= "${IMAGE_FULLNAME}-${KERNEL_FILE}" > > -INITRD_IMAGE ?= "${IMAGE_FULLNAME}-initrd.img" > > +KERNEL_IMAGE = "${IMAGE_FULLNAME}-${KERNEL_FILE}" > > +INITRD_IMAGE ?= "" > > +INITRD_DEPLOY_FILE = "${@d.getVar('INITRD_IMAGE') or > > '${IMAGE_FULLNAME}-initrd.img')" > > > > # This defines the deployed dtbs for reuse by imagers > > DTB_FILES ?= "" > > @@ -353,7 +354,7 @@ EOF > > > > # Default kernel, initrd and dtb image deploy paths (inside > > imager) > > KERNEL_IMG = "${PP_DEPLOY}/${KERNEL_IMAGE}" > > -INITRD_IMG = "${PP_DEPLOY}/${INITRD_IMAGE}" > > +INITRD_IMG = "${PP_DEPLOY}/${INITRD_DEPLOY_FILE}" > > # only one dtb file supported, pick the first > > DTB_IMG = "${PP_DEPLOY}/${@(d.getVar('DTB_FILES').split() or > > [''])[0]}" > > > > @@ -370,12 +371,15 @@ do_copy_boot_files() { > > sudo cat "$kernel" > "${DEPLOYDIR}/${KERNEL_IMAGE}" > > fi > > > > - initrd="$(realpath -q '${IMAGE_ROOTFS}/initrd.img')" > > - if [ ! -f "$initrd" ]; then > > - initrd="$(realpath -q '${IMAGE_ROOTFS}/boot/initrd.img')" > > - fi > > - if [ -f "$initrd" ]; then > > - cp -f "$initrd" '${DEPLOYDIR}/${INITRD_IMAGE}' > > + # deploy default initrd if no custom one is build > > + if [ -e "${INITRD_IMAGE}" ]; then > > + initrd="$(realpath -q '${IMAGE_ROOTFS}/initrd.img')" > > + if [ ! -f "$initrd" ]; then > > + initrd="$(realpath -q > > '${IMAGE_ROOTFS}/boot/initrd.img')" > > + fi > > + if [ -f "$initrd" ]; then > > + cp -f "$initrd" '${DEPLOYDIR}/${INITRD_DEPLOY_FILE}' > > + fi > > fi > > > > for file in ${DTB_FILES}; do > > diff --git a/scripts/start_vm b/scripts/start_vm > > index 17091d72..8c696a4a 100755 > > --- a/scripts/start_vm > > +++ b/scripts/start_vm > > @@ -125,10 +125,10 @@ case "$IMAGE_FSTYPES" in > > readonly ROOTFS_IMAGE=$IMAGE_FULLNAME.ext4 > > > > eval $(bitbake -e mc:qemu$ARCH-$DISTRO:isar-image-base | grep > > "^KERNEL_IMAGE=") > > - eval $(bitbake -e mc:qemu$ARCH-$DISTRO:isar-image-base | grep > > "^INITRD_IMAGE=") > > + eval $(bitbake -e mc:qemu$ARCH-$DISTRO:isar-image-base | grep > > "^INITRD_DEPLOY_FILE=") > > QKERNEL=$IMAGE_DIR/${KERNEL_IMAGE} > > QINITRD=/dev/null > > - [ -n "$INITRD_IMAGE" ] && QINITRD=$IMAGE_DIR/${INITRD_IMAGE} > > + [ -n "$INITRD_DEPLOY_FILE" ] && > > QINITRD=$IMAGE_DIR/${INITRD_DEPLOY_FILE} > > if [ "$ARCH" = "riscv64" ]; then > > EXTRA_ARGS="$EXTRA_ARGS -device > > loader,file=$QKERNEL,addr=0x80200000" > > QKERNEL="/usr/lib/riscv64-linux- > > gnu/opensbi/qemu/virt/fw_jump.elf" > > diff --git a/testsuite/start_vm.py b/testsuite/start_vm.py > > index 82ecc17d..ba1ba127 100755 > > --- a/testsuite/start_vm.py > > +++ b/testsuite/start_vm.py > > @@ -35,7 +35,7 @@ def format_qemu_cmdline(arch, build, distro, out, > > pid, enforce_pcbios=False): > > if image_type == 'ext4': > > rootfs_image = 'isar-image-base-' + base + '-' + distro + > > '-qemu' + arch + '.ext4' > > kernel_image = deploy_dir_image + '/' + > > get_bitbake_var(bb_output, 'KERNEL_IMAGE') > > - initrd_image = get_bitbake_var(bb_output, 'INITRD_IMAGE') > > + initrd_image = get_bitbake_var(bb_output, > > 'INITRD_DEPLOY_FILE') > > > > if not initrd_image: > > initrd_image = '/dev/null' > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-02-18 10:28 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-02-17 10:01 [PATCH 0/3] Fix data-race in deployment of initrd Felix Moessbauer 2023-02-17 10:01 ` [PATCH 1/3] add initramfs to sstate-cache Felix Moessbauer 2023-02-17 10:01 ` [PATCH 2/3] deploy boot files via sstate-cache Felix Moessbauer 2023-02-17 10:01 ` [PATCH 3/3] fix race-cond between default and custom initrd Felix Moessbauer 2023-02-17 11:55 ` Jan Kiszka 2023-02-18 10:28 ` Moessbauer, Felix
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox