* [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