Conversation
On arm, where FileLoad is a stub, kexec printed only "function not implemented", after an "invalid FDT magic" line from the universal payload probe. Neither says what failed or what to do about it. - Name kexec_file_load and the architecture in the ENOSYS stub error. - When kexec_file_load fails with ENOSYS or ENOEXEC, suggest --loadsyscall. ENOEXEC is what riscv returns for an Image before Linux 6.16. - Only log the universal payload probe failure with -d when the file is not an FDT at all, since that is the common case. - Drop "(default true)" from the -d and -L docs; both default to false. On arm the output goes from Failed to load universalpayload (failed to read fdt file: ... invalid FDT magic, got 0x0000a0e1, expected 0xd00dfeed), try legacy kernel.. function not implemented to SYS_kexec_file_load is not supported on arm: function not implemented; try --loadsyscall to use kexec_load instead Fixes u-root#3384 Signed-off-by: Hung-Chun Tseng <alan.tseng.cs@gmail.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3774 +/- ##
==========================================
- Coverage 61.46% 61.33% -0.14%
==========================================
Files 648 648
Lines 45716 45727 +11
==========================================
- Hits 28099 28045 -54
- Misses 17617 17682 +65
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| return err | ||
| } | ||
| if errors.Is(err, syscall.ENOSYS) || errors.Is(err, syscall.ENOEXEC) { | ||
| return fmt.Errorf("%w; try --loadsyscall to use kexec_load instead", err) |
There was a problem hiding this comment.
I sometimes get complaints from tools that the %w should be last, not first; can you make it last?
| } | ||
| } else { | ||
| // universalpayload package suppresses warning message, we print messages here. | ||
| if warningMsg != nil { |
There was a problem hiding this comment.
I am sad to see packages returning nil pointers, it's going to catch us sooner or later. Any chance of fixing that package?
| // case. Only mention it when debugging. | ||
| linux.Debug("%s is not a universal payload (%v), loading it as a kernel", opts.kernelpath, err) | ||
| } else { | ||
| log.Printf("Failed to load universalpayload (%v), try legacy kernel..", err) |
There was a problem hiding this comment.
if this is an error, shouldn't it just return here? This thicket of if ... else might be reduced a bit.
|
Awesome, thanks for looking into this! I just referenced this PR in the corresponding issue I had filed a good while ago. |
Address review on u-root#3774: - A file that is a universal payload but fails to load or execute now returns the error instead of falling back to loading it as a kernel, which could only fail again with a less useful message. The if/else becomes a switch. - Move %w to the end of the --loadsyscall hint. Signed-off-by: Hung-Chun Tseng <alan.tseng.cs@gmail.com>
|
Thanks. Pushed 1942ca9:
|
Fixes #3384.
On arm, where
FileLoadis a stub,kexecprinted onlyfunction not implemented, after aninvalid FDT magicline from the universal payload probe. Neither says what failed or what to do about it.kexec_file_loadand the architecture in the ENOSYS stub error (still wrapssyscall.ENOSYS).kexec_file_loadfails with ENOSYS or ENOEXEC for a Linux image, suggest--loadsyscall. ENOEXEC is what riscv returns for anImagebefore Linux 6.16 (kexec_file_load error #3236).-dwhen the file is not an FDT at all (ErrFailToReadFdtFile), since that is the common case. A file that is a universal payload but fails to load or exec now returns that error instead of falling back to the legacy path.(default true)from the-dand-Ldocs; both default to false.Before (arm32 vmtest kernel,
kexec -l -c KEXEC=Y /kernel):After:
and on riscv64 with the 6.6 vmtest kernel:
Testing: added
TestLoadError;go test ./cmds/core/kexec ./pkg/boot ./pkg/boot/kexec ./pkg/boot/universalpayloadpasses, and the outputs above are from QEMU runs underrunvmtestwith VMTEST_ARCH=arm and riscv64.