Bootloaders: Support for U-boot in extlinux.conf compatibility mode #2256
Conversation
The NVidia Jetson NX comes with u-boot installed in syslinux compatible mode. It reads from `/boot/extlinux/extlinux.conf` to know what to boot. In my setup I set `/boot/extlinux/extlinux.conf` to be a symlink pointing at `../loader/syslinux.cfg` to take advantage of the ostree syslinux support. This commit improves compatibility by also checking for `extlinux.conf` in addition to `syslinux.cfg` when trying to locate the existing configuration.
The NVidia Jetson NX comes with u-boot installed in syslinux compatible mode. It reads from `/boot/extlinux/extlinux.conf` to know what to boot. It's quite picky about what format it will accept. It crashes if `MENU LABEL` is missing or `KERNEL` is present rather than `LINUX`. This commit improves compatibility by conforming to what u-boot accepts. In theory it shouldn't affect real syslinux systems because the documentation says: > ### LINUX > > You can use this, instead of using KERNEL file to boot a linux kernel > image. although I don't have a real syslinux system to test it on.
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wmanley The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| @@ -124,7 +133,19 @@ _ostree_bootloader_syslinux_write_config (OstreeBootloader *bootloader, | |||
| glnx_file_get_contents_utf8_at (self->sysroot->sysroot_fd, syslinux_config_path, NULL, | |||
| cancellable, error); | |||
| if (!config_contents) | |||
| return FALSE; | |||
| { | |||
| if (errno != ENOENT) | |||
There was a problem hiding this comment.
Having errno be set after a function returns a GError is...somewhat undefined behavior. It'd be better to do:
if (g_error_matches (temp_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND))
which is a pattern you can find all over the codebase (xref https://docs.rs/openat-ext/0.1.9/openat_ext/trait.OpenatDirExt.html#tymethod.open_file_optional )
| @@ -84,12 +92,13 @@ append_config_from_loader_entries (OstreeBootloaderSyslinux *self, | |||
| if (regenerate_default && i == 0) | |||
| g_ptr_array_add (new_lines, g_strdup_printf ("DEFAULT %s", val)); | |||
|
|
|||
| g_ptr_array_add (new_lines, g_strdup_printf ("LABEL %s", val)); | |||
| g_ptr_array_add (new_lines, g_strdup_printf ("\nLABEL %s", val)); | |||
There was a problem hiding this comment.
The way we're adding newlines into what is intended to be an "array of lines" feels...slightly wrong though it's clearly going to work. But WDYT about:
g_ptr_array_add (new_lines, g_strdup (""));
g_ptr_array_add (new_lines, g_strdup_printf ("LABEL %s", val));
instead?
|
@wmanley: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Supports the bootloader that comes with the NVidia Tegra TK-1 and NX.
The NVidia Jetson NX comes with u-boot installed in syslinux compatible
mode. It reads from
/boot/extlinux/extlinux.confto know what to boot.In my setup I set
/boot/extlinux/extlinux.confto be a symlink pointingat
../loader/syslinux.cfgto take advantage of the ostree syslinuxsupport.
This PR improves compatibility by also checking for
extlinux.confinaddition to
syslinux.cfgwhen trying to locate the existingconfiguration and by conforming to what u-boot accepts.
In theory it shouldn't affect real syslinux systems because the
documentation says:
although I don't have a real syslinux system to test it on.
We've been using this in production for some time and it's been working well.
See also: #2255
TODO: