Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BootimgPcbiosIsarPlugin wic plugin files fails for lack of syslinux #69

Open
ydirson opened this issue Sep 17, 2021 · 25 comments
Open

BootimgPcbiosIsarPlugin wic plugin files fails for lack of syslinux #69

ydirson opened this issue Sep 17, 2021 · 25 comments

Comments

@ydirson
Copy link

ydirson commented Sep 17, 2021

I can generate an UEFI image with the following settings:

MACHINE ??= "qemuamd64"
DISTRO ??= "debian-buster"

To get a legacy/pcbios image, I try WKS_FILE = "directdisk-isar.wks" or directdisk-gpt.wks ... which fail looking for syslinux, which is apparently not pulled. Some missing dependency ?

ERROR: mc:qemuamd64-buster:isar-image-base-1.0-r0 do_wic_image: Error executing a python function in exec_python_func() autogenerated:

The stack trace of python calls that resulted in this exception/failure was:
File: 'exec_python_func() autogenerated', lineno: 2, function: <module>
     0001:
 *** 0002:do_wic_image(d)
     0003:
File: '/home/user/isar/meta/classes/wic-img.bbclass', lineno: 141, function: do_wic_image
     0137:}
     0138:
     0139:do_wic_image[file-checksums] += "${WKS_FILE_CHECKSUM}"
     0140:python do_wic_image() {
 *** 0141:    bb.build.exec_func("generate_wic_image", d)
     0142:    bb.build.exec_func("check_for_wic_warnings", d)
     0143:}
     0144:addtask wic_image before do_image after do_image_tools
     0145:
File: '/home/user/isar/bitbake/lib/bb/build.py', lineno: 251, function: exec_func
     0247:    with bb.utils.fileslocked(lockfiles):
     0248:        if ispython:
     0249:            exec_func_python(func, d, runfile, cwd=adir)
     0250:        else:
 *** 0251:            exec_func_shell(func, d, runfile, cwd=adir)
     0252:
     0253:    try:
     0254:        curcwd = os.getcwd()
     0255:    except:
File: '/home/user/isar/bitbake/lib/bb/build.py', lineno: 452, function: exec_func_shell
     0448:    with open(fifopath, 'r+b', buffering=0) as fifo:
     0449:        try:
     0450:            bb.debug(2, "Executing shell function %s" % func)
     0451:            with open(os.devnull, 'r+') as stdin, logfile:
 *** 0452:                bb.process.run(cmd, shell=False, stdin=stdin, log=logfile, extrafiles=[(fifo,readfifo)])
     0453:        finally:
     0454:            os.unlink(fifopath)
     0455:
     0456:    bb.debug(2, "Shell function %s finished" % func)
File: '/home/user/isar/bitbake/lib/bb/process.py', lineno: 182, function: run
     0178:        if not stderr is None:
     0179:            stderr = stderr.decode("utf-8")
     0180:
     0181:    if pipe.returncode != 0:
 *** 0182:        raise ExecutionError(cmd, pipe.returncode, stdout, stderr)
     0183:    return stdout, stderr
Exception: bb.process.ExecutionError: Execution of '/home/user/build/tmp/work/debian-buster-amd64/isar-image-base-qemuamd64-wic-img/1.0-r0/temp/run.generate_wic_image.26597' failed with exit code 1:
INFO: Creating image(s)...

ERROR: Couldn't find syslinux directory, exiting

WARNING: exit code 1 from a shell command.


ERROR: Logfile of failure stored in: /home/user/build/tmp/work/debian-buster-amd64/isar-image-base-qemuamd64-wic-img/1.0-r0/temp/log.do_wic_image.26597
ERROR: Task (mc:qemuamd64-buster:/home/user/isar/meta-isar/recipes-core/images/isar-image-base.bb:do_wic_image) failed with exit code '1'

Diggig a bit I see that BootimgPcbiosIsarPlugin is heavily depending in syslinux. However, it does not look like there is any recipe to get the latter into STAGING_DATADIR.

@ydirson ydirson changed the title pcbios wks files fails for lack of syslinux BootimgPcbiosIsarPlugin wic plugin files fails for lack of syslinux Sep 18, 2021
@ismagulb
Copy link
Contributor

I've tried qemuamd64.conf with WKS_FILE = "directdisk-isar.wks" but get a different error message from run.generate_wic_image. Did you do some other steps / how can I reproduce your problem?

@ydirson
Copy link
Author

ydirson commented Sep 18, 2021

This is in a Debian10 VM, the following steps reproduce the error:

user@isar-build:~/isar$ git clean -fdx
user@isar-build:~/isar$ git describe --dirty
v0.7-436-g03124cc
user@isar-build:~/isar$ . isar-init-build-env ../build2
...
user@isar-build:~/build2$ cat >> conf/local.conf 
MACHINE = "qemuamd64"
DISTRO = "debian-buster"
DISTRO_ARCH = "amd64"
WKS_FILE = "directdisk-isar.wks"
user@isar-build:~/build2$ bitbake mc:qemuamd64-buster:isar-image-base

@henning-schild
Copy link
Contributor

IMAGER_INSTALL += "${SYSLINUX_BOOTLOADER_INSTALL}"

@ismagulb
Copy link
Contributor

Thanks Henning, worked for me.

@ydirson
Copy link
Author

ydirson commented Sep 20, 2021

I confirm, adding this to eg. local.conf does the trick. Getting it to be selected automatically seems more tricky, though.

@henning-schild
Copy link
Contributor

Well it can be kind of found in the examples. IMAGER_INSTALL adds stuff to the buildchroot that is needed for the do_image task. What that will be depends on which image class you choose. In the case of wic it can even differ depending on which plugins you use in your WKS_FILE. When choosing "pcbios" it will be "syslinux", for "efi" it can be "grub" (but can also be empty when using systemd-boot).

Not sure how really to automate the choice. In fact we could always add "syslinux" and "grub" as potentially needed runtime deps of wic. Adding a few too many packages to buildchroot does not hurt, will not grow the the target image in terms of size or manifest. But doing so would have to take into account that some packages are not available in all arches or change their names.

i.e. "syslinux" is amd64/i386-only while "grub" gets called many names

meta/conf/distro/debian-common.conf

21:GRUB_BOOTLOADER_INSTALL_amd64 = "grub-efi-amd64-bin"
22:GRUB_BOOTLOADER_INSTALL_i386 = "grub-efi-ia32-bin"
23:GRUB_BOOTLOADER_INSTALL_armhf = "grub-efi-arm-bin"
24:GRUB_BOOTLOADER_INSTALL_arm64 = "grub-efi-arm64-bin"

My guess is that the hint i gave is enough to close this issue. Anyways you should not be using pcbios unless you really really have to deal with such legacy.

@henning-schild
Copy link
Contributor

@ydirson your git describe looks like "master" branch ... in Isar "next" is usually better and not rebasing while "master" is often lacking behind and might have bugs already fixed in "next"

@henning-schild
Copy link
Contributor

And note that i only found this issue because of a cross-post to the Isar mailing-list. Issues on github or not the best channel to get in touch, mailing-list posts would be better because they have a wider reach into the "small" community.

@ydirson
Copy link
Author

ydirson commented Sep 20, 2021

Note(s) taken, thanks!

@ismagulb
Copy link
Contributor

@ydirson, if this worked for you, can we close this issue?

@henning-schild
Copy link
Contributor

@ismagulb what works better is is not too rude is to close saying "please re-open if issue persists"

@ydirson
Copy link
Author

ydirson commented Oct 24, 2021

I understand the difficulty of making this transparent to the user, and since the user will have to take care of this, wouldn't it be better to make this more visible in the doc ?

@ismagulb
Copy link
Contributor

Good point, let me check what we can do. If you have a specific idea, a patch would be great.

@henning-schild
Copy link
Contributor

Concluding that this issue is solved, like already confirmed in #69 (comment)

Possibly needed doc improvements might deserve a new issue.

amikan added a commit that referenced this issue Oct 28, 2021
Include into user_manual.md explanation of imager dependencies should
be added if using wic image type with bootloader-related plugins.

Inspired by issue #69

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
@ydirson
Copy link
Author

ydirson commented Nov 11, 2021

Well it can be kind of found in the examples. IMAGER_INSTALL adds stuff to the buildchroot that is needed for the do_image task. What that will be depends on which image class you choose. In the case of wic it can even differ depending on which plugins you use in your WKS_FILE. When choosing "pcbios" it will be "syslinux", for "efi" it can be "grub" (but can also be empty when using systemd-boot).

Thinking twice about that, we can expand variables inside .wks files. If we use this to specify plugins from a bitbake variable, we could probably use the same variable to trigger the addition of the proper dependencies to IMAGER_INSTALL ?

@jan-kiszka
Copy link
Contributor

Simplifications are always welcome, but I cannot follow yours yet. Can you make a concrete example?

@ydirson
Copy link
Author

ydirson commented Nov 11, 2021

Here is a PoC for just directdisk-isar, with only definitions for the pcbios case. Major problem would probably be that all users would have to change .wks to .wks.in, so maybe both could have to be kept for compatibility (which kinda sucks).

Maybe wic-img.bbclass would be a better fit than image.bbclass, BTW.

diff --git a/meta-isar/scripts/lib/wic/canned-wks/common-isar.wks.inc b/meta-isar/scripts/lib/wic/canned-wks/common-isar.wks.inc
index a65e646..2607edb 100644
--- a/meta-isar/scripts/lib/wic/canned-wks/common-isar.wks.inc
+++ b/meta-isar/scripts/lib/wic/canned-wks/common-isar.wks.inc
@@ -1,3 +1,3 @@
 # This file is included into 3 canned wks files from this directory
-part /boot --source bootimg-pcbios-isar --ondisk sda --label boot --active --align 1024
+part /boot --source ${BOOTIMG_PLUGIN} --ondisk sda --label boot --active --align 1024
 part / --source rootfs --ondisk sda --fstype=ext4 --mkfs-extraopts "-T default" --label platform --align 1024 --exclude-path=boot
diff --git a/meta-isar/scripts/lib/wic/canned-wks/directdisk-isar.wks b/meta-isar/scripts/lib/wic/canned-wks/directdisk-isar.wks.in
similarity index 100%
rename from meta-isar/scripts/lib/wic/canned-wks/directdisk-isar.wks
rename to meta-isar/scripts/lib/wic/canned-wks/directdisk-isar.wks.in
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 5a0f32e..021c860 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -15,6 +15,14 @@ IMAGE_INSTALL += "${@ ("linux-image-" + d.getVar("KERNEL_NAME", True)) if d.getV
 # Name of the image including distro&machine names
 IMAGE_FULLNAME = "${PN}-${DISTRO}-${MACHINE}"
 
+# These variables are used to keep in sync wic plugin and matching dependencies
+BOOTIMG_TYPE ?= "pcbios"
+BOOTIMG_PLUGIN_pcbios = "bootimg-pcbios-isar"
+BOOTIMG_INSTALL_pcbios = "${SYSLINUX_BOOTLOADER_INSTALL}"
+BOOTIMG_PLUGIN = "${BOOTIMG_PLUGIN_${BOOTIMG_TYPE}}"
+IMAGER_INSTALL += "${BOOTIMG_INSTALL_${BOOTIMG_TYPE}}"
+WICVARS_append = " BOOTIMG_PLUGIN"
+
 # These variables are used by wic and start_vm
 KERNEL_IMAGE ?= "${IMAGE_FULLNAME}-${KERNEL_FILE}"
 INITRD_IMAGE ?= "${IMAGE_FULLNAME}-initrd.img"

@jan-kiszka
Copy link
Contributor

And you would BOOTIMG_TYPE basically where you also set WKS_FILE

Yeah, this would be the approach from a technical perspective. From a user perspective, selecting bootimg-* in the wks file should simple do The Right Thing, automatically. Maybe we should parse the selected WKS_FILE for bootimg entries and extend IMAGER_INSTALL automatically? Somewhere in wic-img.bbclass.

@ydirson
Copy link
Author

ydirson commented Nov 11, 2021

I'm not sure it is more "right" to select a particular plugin in the .wks, than to specifiy the intent at a higher level, especially as it would also open the possibility to have a single .wks for both efi and pcbios boot (lifting --source-params into a bb var too). In yocto, as a user I've often felt the .wks as an implementation detail: only when the user wants to tune the partitions is it necessary to dive into wic. I feel it would improve the learning curve to have the parameters most in need of tuning all easily set in local.conf.

And you would BOOTIMG_TYPE basically where you also set WKS_FILE

Yes, and it could also maybe be better named like WKS_FILE_BOOTIMG_TYPE to stress the link. Although I'm not completely sure here: this parameter could also be used in other places, eg. to build a qemu commandline, or even to select alternative bootchains (eg. "legacy" vs. uefi on arm64).

Note: it looks like the renaming to .wks.in may not be needed in the end.

@jan-kiszka
Copy link
Contributor

In the ideal world, the wks file is supposed to describe the partitioning and its parameters, not a higher level.

@ydirson
Copy link
Author

ydirson commented Nov 11, 2021

In the ideal world, the wks file is supposed to describe the partitioning and its parameters, not a higher level.

Not necessarily all of them (especially when other parts of the system depend on the same high-level requirement), and it's probably a good part of why there is variable substitution in there. And I still feel that selecting the target bootchain is more high-level than the specific wic plugin implementing it.

See eg. in meta-rockchip:

bootloader --ptable gpt --append="console=tty1 console=${RK_CONSOLE_DEVICE},${RK_CONSOLE_BAUD}n8 rw rootfstype=ext4 init=/sbin/init"

@jan-kiszka
Copy link
Contributor

I agree for the given type of example. But I disagree about the boot method. You may only see bootimg-* right now, but the boot loader selection is broader and generally influences what needs to be IMAGER_INSTALL (u-boot, efibootguard, etc.).

@ydirson
Copy link
Author

ydirson commented Nov 11, 2021

Maybe then more information could be added together with #69 (reference) ?

@henning-schild
Copy link
Contributor

In fact it remains a hack that we install the bootloader in the buildchroot and not in the target rootfs. A hack that applies to grub and syslinux but not to u-boot or systemd-boot, not sure about efibootguard which only lives in layers but not in isar.

While that keeps the rootfs small it also has issues. i.e. we see problems when doing kernel updates with apt, because we lack grub-mkconfig. And our bill of materials (manifest) does not list bootloaders that are not installed in the rootfs but still make it into the image.

My personal long term plan would be to install the bootloader inside the rootfs, no matter which bootloader that may be.

We have several problems on that front, and the more we discuss it here the more we leave out the mailinglist while discovering more issues/solutions. So again, please close and move to where isar discussions are supposed to happen!

@henning-schild
Copy link
Contributor

amikan added a commit that referenced this issue Dec 23, 2021
Include explanation of imager dependencies into user_manual.md.
Also mention deps should be added if using wic image type with
bootloader-related plugins.

Inspired by issue #69.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants