* [PATCH] [RFC] Add exitcode support
@ 2015-08-18 17:30 dann frazier
2015-08-18 18:03 ` Ben Hildred
0 siblings, 1 reply; 8+ messages in thread
From: dann frazier @ 2015-08-18 17:30 UTC (permalink / raw)
To: grub-devel
Some platforms are capable of changing behavior based on the bootloader
exit code. In particular, UEFI boot managers use this code to determine
if they should try the next boot entry or not. I'd like to use this as
a way to make my PXE server tell clients to attempt to boot from their
next entry (presumably an on-disk OS), providing something akin to
pxelinux's "localboot".
The implementation here is to add a new optional argument to the exit
command. The platform-specific grub_exit() implementations can then
translate it into a platform-appropriate exit code.
Signed-off-by: dann frazier <dann.frazier@canonical.com>
---
grub-core/commands/minicmd.c | 12 ++++++++----
grub-core/kern/efi/efi.c | 20 ++++++++++++++++++--
grub-core/kern/emu/misc.c | 3 ++-
grub-core/kern/i386/coreboot/init.c | 3 ++-
grub-core/kern/i386/qemu/init.c | 3 ++-
grub-core/kern/ieee1275/init.c | 3 ++-
grub-core/kern/mips/arc/init.c | 3 ++-
grub-core/kern/mips/loongson/init.c | 3 ++-
grub-core/kern/mips/qemu_mips/init.c | 3 ++-
grub-core/kern/misc.c | 3 ++-
grub-core/kern/uboot/init.c | 5 +++--
grub-core/kern/xen/init.c | 2 +-
include/grub/exitcode.h | 30 ++++++++++++++++++++++++++++++
include/grub/misc.h | 3 ++-
14 files changed, 78 insertions(+), 18 deletions(-)
create mode 100644 include/grub/exitcode.h
diff --git a/grub-core/commands/minicmd.c b/grub-core/commands/minicmd.c
index a3a1182..d67cdf1 100644
--- a/grub-core/commands/minicmd.c
+++ b/grub-core/commands/minicmd.c
@@ -28,6 +28,7 @@
#include <grub/loader.h>
#include <grub/command.h>
#include <grub/i18n.h>
+#include <grub/exitcode.h>
GRUB_MOD_LICENSE ("GPLv3+");
@@ -178,10 +179,13 @@ grub_mini_cmd_lsmod (struct grub_command *cmd __attribute__ ((unused)),
/* exit */
static grub_err_t __attribute__ ((noreturn))
grub_mini_cmd_exit (struct grub_command *cmd __attribute__ ((unused)),
- int argc __attribute__ ((unused)),
- char *argv[] __attribute__ ((unused)))
+ int argc, char *argv[])
+
{
- grub_exit ();
+ grub_exitcode_t exitcode;
+
+ exitcode = argc ? grub_strtoul(argv[0], 0, 10) : GRUB_EXITCODE_NONE;
+ grub_exit (exitcode);
/* Not reached. */
}
@@ -207,7 +211,7 @@ GRUB_MOD_INIT(minicmd)
0, N_("Show loaded modules."));
cmd_exit =
grub_register_command ("exit", grub_mini_cmd_exit,
- 0, N_("Exit from GRUB."));
+ N_("[EXITCODE]"), N_("Exit from GRUB."));
}
GRUB_MOD_FINI(minicmd)
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index caf9bcc..33fb737 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -28,6 +28,7 @@
#include <grub/kernel.h>
#include <grub/mm.h>
#include <grub/loader.h>
+#include <grub/exitcode.h>
/* The handle of GRUB itself. Filled in by the startup code. */
grub_efi_handle_t grub_efi_image_handle;
@@ -155,11 +156,26 @@ grub_efi_get_loaded_image (grub_efi_handle_t image_handle)
}
void
-grub_exit (void)
+grub_exit (grub_exitcode_t exitcode)
{
+ grub_efi_status_t efi_status;
+
grub_machine_fini (GRUB_LOADER_FLAG_NORETURN);
+
+ /* Map the GRUB exitcode to a suitable EFI status */
+ switch (exitcode)
+ {
+ case GRUB_EXITCODE_NEXT:
+ /* Anything other than EFI_SUCCESS will do (UEFI 2.5 section 3.1.1) */
+ efi_status = GRUB_EFI_LOAD_ERROR;
+ break;
+ default:
+ efi_status = GRUB_EFI_SUCCESS;
+ break;
+ }
+
efi_call_4 (grub_efi_system_table->boot_services->exit,
- grub_efi_image_handle, GRUB_EFI_SUCCESS, 0, 0);
+ grub_efi_image_handle, efi_status, 0, 0);
for (;;) ;
}
diff --git a/grub-core/kern/emu/misc.c b/grub-core/kern/emu/misc.c
index bb606da..4f345d6 100644
--- a/grub-core/kern/emu/misc.c
+++ b/grub-core/kern/emu/misc.c
@@ -35,6 +35,7 @@
#include <grub/i18n.h>
#include <grub/time.h>
#include <grub/emu/misc.h>
+#include <grub/exitcode.h>
int verbosity;
@@ -135,7 +136,7 @@ xasprintf (const char *fmt, ...)
#endif
void
-grub_exit (void)
+grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
{
exit (1);
}
diff --git a/grub-core/kern/i386/coreboot/init.c b/grub-core/kern/i386/coreboot/init.c
index 3314f02..07294a1 100644
--- a/grub-core/kern/i386/coreboot/init.c
+++ b/grub-core/kern/i386/coreboot/init.c
@@ -35,13 +35,14 @@
#include <grub/cpu/floppy.h>
#include <grub/cpu/tsc.h>
#include <grub/video.h>
+#include <grub/exitcode.h>
extern grub_uint8_t _start[];
extern grub_uint8_t _end[];
extern grub_uint8_t _edata[];
void __attribute__ ((noreturn))
-grub_exit (void)
+grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
{
/* We can't use grub_fatal() in this function. This would create an infinite
loop, since grub_fatal() calls grub_abort() which in turn calls grub_exit(). */
diff --git a/grub-core/kern/i386/qemu/init.c b/grub-core/kern/i386/qemu/init.c
index 271b6fb..67e155c 100644
--- a/grub-core/kern/i386/qemu/init.c
+++ b/grub-core/kern/i386/qemu/init.c
@@ -36,13 +36,14 @@
#include <grub/cpu/tsc.h>
#include <grub/machine/kernel.h>
#include <grub/pci.h>
+#include <grub/exitcode.h>
extern grub_uint8_t _start[];
extern grub_uint8_t _end[];
extern grub_uint8_t _edata[];
void __attribute__ ((noreturn))
-grub_exit (void)
+grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
{
/* We can't use grub_fatal() in this function. This would create an infinite
loop, since grub_fatal() calls grub_abort() which in turn calls grub_exit(). */
diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index d5bd74d..996663f 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -35,6 +35,7 @@
#include <grub/offsets.h>
#include <grub/memory.h>
#include <grub/loader.h>
+#include <grub/exitcode.h>
#ifdef __i386__
#include <grub/cpu/tsc.h>
#endif
@@ -60,7 +61,7 @@ grub_addr_t grub_ieee1275_original_stack;
#endif
void
-grub_exit (void)
+grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
{
grub_ieee1275_exit ();
}
diff --git a/grub-core/kern/mips/arc/init.c b/grub-core/kern/mips/arc/init.c
index 3834a14..28189b4 100644
--- a/grub-core/kern/mips/arc/init.c
+++ b/grub-core/kern/mips/arc/init.c
@@ -36,6 +36,7 @@
#include <grub/i18n.h>
#include <grub/disk.h>
#include <grub/partition.h>
+#include <grub/exitcode.h>
const char *type_names[] = {
#ifdef GRUB_CPU_WORDS_BIGENDIAN
@@ -276,7 +277,7 @@ grub_halt (void)
}
void
-grub_exit (void)
+grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
{
GRUB_ARC_FIRMWARE_VECTOR->exit ();
diff --git a/grub-core/kern/mips/loongson/init.c b/grub-core/kern/mips/loongson/init.c
index 7b96531..5958653 100644
--- a/grub-core/kern/mips/loongson/init.c
+++ b/grub-core/kern/mips/loongson/init.c
@@ -38,6 +38,7 @@
#include <grub/serial.h>
#include <grub/loader.h>
#include <grub/at_keyboard.h>
+#include <grub/exitcode.h>
grub_err_t
grub_machine_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
@@ -304,7 +305,7 @@ grub_halt (void)
}
void
-grub_exit (void)
+grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
{
grub_halt ();
}
diff --git a/grub-core/kern/mips/qemu_mips/init.c b/grub-core/kern/mips/qemu_mips/init.c
index be88b77..b807b78 100644
--- a/grub-core/kern/mips/qemu_mips/init.c
+++ b/grub-core/kern/mips/qemu_mips/init.c
@@ -17,6 +17,7 @@
#include <grub/serial.h>
#include <grub/loader.h>
#include <grub/at_keyboard.h>
+#include <grub/exitcode.h>
static inline int
probe_mem (grub_addr_t addr)
@@ -75,7 +76,7 @@ grub_machine_fini (int flags __attribute__ ((unused)))
}
void
-grub_exit (void)
+grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
{
grub_halt ();
}
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index 906d2c2..0db98cc 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -24,6 +24,7 @@
#include <grub/term.h>
#include <grub/env.h>
#include <grub/i18n.h>
+#include <grub/exitcode.h>
union printf_arg
{
@@ -1087,7 +1088,7 @@ grub_abort (void)
grub_getkey ();
}
- grub_exit ();
+ grub_exit (GRUB_EXITCODE_NONE);
}
void
diff --git a/grub-core/kern/uboot/init.c b/grub-core/kern/uboot/init.c
index 5dcc106..1f27fa9 100644
--- a/grub-core/kern/uboot/init.c
+++ b/grub-core/kern/uboot/init.c
@@ -32,6 +32,7 @@
#include <grub/uboot/api_public.h>
#include <grub/cpu/system.h>
#include <grub/cache.h>
+#include <grub/exitcode.h>
extern char __bss_start[];
extern char _end[];
@@ -43,7 +44,7 @@ extern grub_uint32_t grub_uboot_machine_type;
extern grub_addr_t grub_uboot_boot_data;
void
-grub_exit (void)
+grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
{
grub_uboot_return (0);
}
@@ -94,7 +95,7 @@ grub_machine_init (void)
if (!ver)
{
/* Don't even have a console to log errors to... */
- grub_exit ();
+ grub_exit (GRUB_EXITCODE_NONE);
}
else if (ver > API_SIG_VERSION)
{
diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
index 0559c03..0fc3723 100644
--- a/grub-core/kern/xen/init.c
+++ b/grub-core/kern/xen/init.c
@@ -549,7 +549,7 @@ grub_machine_init (void)
}
void
-grub_exit (void)
+grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
{
struct sched_shutdown arg;
diff --git a/include/grub/exitcode.h b/include/grub/exitcode.h
new file mode 100644
index 0000000..7f179f4
--- /dev/null
+++ b/include/grub/exitcode.h
@@ -0,0 +1,30 @@
+/* exitcode.h - declare exitcode types */
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2015 Free Software Foundation, Inc.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_EXITCODE_HEADER
+#define GRUB_EXITCODE_HEADER 1
+
+typedef enum
+ {
+ GRUB_EXITCODE_NONE = 0,
+ GRUB_EXITCODE_NEXT,
+ }
+grub_exitcode_t;
+
+#endif /* ! GRUB_EXITCODE_HEADER */
diff --git a/include/grub/misc.h b/include/grub/misc.h
index 2a9f87c..18dc77c 100644
--- a/include/grub/misc.h
+++ b/include/grub/misc.h
@@ -26,6 +26,7 @@
#include <grub/err.h>
#include <grub/i18n.h>
#include <grub/compiler.h>
+#include <grub/exitcode.h>
#define ALIGN_UP(addr, align) \
((addr + (typeof (addr)) align - 1) & ~((typeof (addr)) align - 1))
@@ -334,7 +335,7 @@ int EXPORT_FUNC(grub_vsnprintf) (char *str, grub_size_t n, const char *fmt,
char *EXPORT_FUNC(grub_xasprintf) (const char *fmt, ...)
__attribute__ ((format (GNU_PRINTF, 1, 2))) WARN_UNUSED_RESULT;
char *EXPORT_FUNC(grub_xvasprintf) (const char *fmt, va_list args) WARN_UNUSED_RESULT;
-void EXPORT_FUNC(grub_exit) (void) __attribute__ ((noreturn));
+void EXPORT_FUNC(grub_exit) (grub_exitcode_t exitcode) __attribute__ ((noreturn));
grub_uint64_t EXPORT_FUNC(grub_divmod64) (grub_uint64_t n,
grub_uint64_t d,
grub_uint64_t *r);
--
2.5.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] [RFC] Add exitcode support
2015-08-18 17:30 [PATCH] [RFC] Add exitcode support dann frazier
@ 2015-08-18 18:03 ` Ben Hildred
2015-08-18 18:56 ` Dann Frazier
0 siblings, 1 reply; 8+ messages in thread
From: Ben Hildred @ 2015-08-18 18:03 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 13519 bytes --]
I only briefly scanned this, but this is cool (at least the idea is
something I want to see implemented everywhere it makes sense to do so).
Just one humdinger of a question: does efi assume nonzero is an error? Does
platform X assume a nonzero exit is an error? Do we want to assume nonzero
is an error? What do we do when these assumptions do not agree?
My off the cuff recommendation is for grub to make no assumptions about how
exit's value will be used in a general case but to instead have a per
platform predefined variable (may EXITTYPE) which would provide to the
script the semantics of the exit value. I would propose that the first
three possible values be 'ignored', 'errorlevel', and 'zeroerror'. This
would handle all current models that I am aware of. specifically it gives
known true and false values everywhere.
On Tue, Aug 18, 2015 at 11:30 AM, dann frazier <dann.frazier@canonical.com>
wrote:
> Some platforms are capable of changing behavior based on the bootloader
> exit code. In particular, UEFI boot managers use this code to determine
> if they should try the next boot entry or not. I'd like to use this as
> a way to make my PXE server tell clients to attempt to boot from their
> next entry (presumably an on-disk OS), providing something akin to
> pxelinux's "localboot".
>
> The implementation here is to add a new optional argument to the exit
> command. The platform-specific grub_exit() implementations can then
> translate it into a platform-appropriate exit code.
>
> Signed-off-by: dann frazier <dann.frazier@canonical.com>
> ---
> grub-core/commands/minicmd.c | 12 ++++++++----
> grub-core/kern/efi/efi.c | 20 ++++++++++++++++++--
> grub-core/kern/emu/misc.c | 3 ++-
> grub-core/kern/i386/coreboot/init.c | 3 ++-
> grub-core/kern/i386/qemu/init.c | 3 ++-
> grub-core/kern/ieee1275/init.c | 3 ++-
> grub-core/kern/mips/arc/init.c | 3 ++-
> grub-core/kern/mips/loongson/init.c | 3 ++-
> grub-core/kern/mips/qemu_mips/init.c | 3 ++-
> grub-core/kern/misc.c | 3 ++-
> grub-core/kern/uboot/init.c | 5 +++--
> grub-core/kern/xen/init.c | 2 +-
> include/grub/exitcode.h | 30 ++++++++++++++++++++++++++++++
> include/grub/misc.h | 3 ++-
> 14 files changed, 78 insertions(+), 18 deletions(-)
> create mode 100644 include/grub/exitcode.h
>
> diff --git a/grub-core/commands/minicmd.c b/grub-core/commands/minicmd.c
> index a3a1182..d67cdf1 100644
> --- a/grub-core/commands/minicmd.c
> +++ b/grub-core/commands/minicmd.c
> @@ -28,6 +28,7 @@
> #include <grub/loader.h>
> #include <grub/command.h>
> #include <grub/i18n.h>
> +#include <grub/exitcode.h>
>
> GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -178,10 +179,13 @@ grub_mini_cmd_lsmod (struct grub_command *cmd
> __attribute__ ((unused)),
> /* exit */
> static grub_err_t __attribute__ ((noreturn))
> grub_mini_cmd_exit (struct grub_command *cmd __attribute__ ((unused)),
> - int argc __attribute__ ((unused)),
> - char *argv[] __attribute__ ((unused)))
> + int argc, char *argv[])
> +
> {
> - grub_exit ();
> + grub_exitcode_t exitcode;
> +
> + exitcode = argc ? grub_strtoul(argv[0], 0, 10) : GRUB_EXITCODE_NONE;
> + grub_exit (exitcode);
> /* Not reached. */
> }
>
> @@ -207,7 +211,7 @@ GRUB_MOD_INIT(minicmd)
> 0, N_("Show loaded modules."));
> cmd_exit =
> grub_register_command ("exit", grub_mini_cmd_exit,
> - 0, N_("Exit from GRUB."));
> + N_("[EXITCODE]"), N_("Exit from GRUB."));
> }
>
> GRUB_MOD_FINI(minicmd)
> diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> index caf9bcc..33fb737 100644
> --- a/grub-core/kern/efi/efi.c
> +++ b/grub-core/kern/efi/efi.c
> @@ -28,6 +28,7 @@
> #include <grub/kernel.h>
> #include <grub/mm.h>
> #include <grub/loader.h>
> +#include <grub/exitcode.h>
>
> /* The handle of GRUB itself. Filled in by the startup code. */
> grub_efi_handle_t grub_efi_image_handle;
> @@ -155,11 +156,26 @@ grub_efi_get_loaded_image (grub_efi_handle_t
> image_handle)
> }
>
> void
> -grub_exit (void)
> +grub_exit (grub_exitcode_t exitcode)
> {
> + grub_efi_status_t efi_status;
> +
> grub_machine_fini (GRUB_LOADER_FLAG_NORETURN);
> +
> + /* Map the GRUB exitcode to a suitable EFI status */
> + switch (exitcode)
> + {
> + case GRUB_EXITCODE_NEXT:
> + /* Anything other than EFI_SUCCESS will do (UEFI 2.5 section 3.1.1)
> */
> + efi_status = GRUB_EFI_LOAD_ERROR;
> + break;
> + default:
> + efi_status = GRUB_EFI_SUCCESS;
> + break;
> + }
> +
> efi_call_4 (grub_efi_system_table->boot_services->exit,
> - grub_efi_image_handle, GRUB_EFI_SUCCESS, 0, 0);
> + grub_efi_image_handle, efi_status, 0, 0);
> for (;;) ;
> }
>
> diff --git a/grub-core/kern/emu/misc.c b/grub-core/kern/emu/misc.c
> index bb606da..4f345d6 100644
> --- a/grub-core/kern/emu/misc.c
> +++ b/grub-core/kern/emu/misc.c
> @@ -35,6 +35,7 @@
> #include <grub/i18n.h>
> #include <grub/time.h>
> #include <grub/emu/misc.h>
> +#include <grub/exitcode.h>
>
> int verbosity;
>
> @@ -135,7 +136,7 @@ xasprintf (const char *fmt, ...)
> #endif
>
> void
> -grub_exit (void)
> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> {
> exit (1);
> }
> diff --git a/grub-core/kern/i386/coreboot/init.c
> b/grub-core/kern/i386/coreboot/init.c
> index 3314f02..07294a1 100644
> --- a/grub-core/kern/i386/coreboot/init.c
> +++ b/grub-core/kern/i386/coreboot/init.c
> @@ -35,13 +35,14 @@
> #include <grub/cpu/floppy.h>
> #include <grub/cpu/tsc.h>
> #include <grub/video.h>
> +#include <grub/exitcode.h>
>
> extern grub_uint8_t _start[];
> extern grub_uint8_t _end[];
> extern grub_uint8_t _edata[];
>
> void __attribute__ ((noreturn))
> -grub_exit (void)
> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> {
> /* We can't use grub_fatal() in this function. This would create an
> infinite
> loop, since grub_fatal() calls grub_abort() which in turn calls
> grub_exit(). */
> diff --git a/grub-core/kern/i386/qemu/init.c
> b/grub-core/kern/i386/qemu/init.c
> index 271b6fb..67e155c 100644
> --- a/grub-core/kern/i386/qemu/init.c
> +++ b/grub-core/kern/i386/qemu/init.c
> @@ -36,13 +36,14 @@
> #include <grub/cpu/tsc.h>
> #include <grub/machine/kernel.h>
> #include <grub/pci.h>
> +#include <grub/exitcode.h>
>
> extern grub_uint8_t _start[];
> extern grub_uint8_t _end[];
> extern grub_uint8_t _edata[];
>
> void __attribute__ ((noreturn))
> -grub_exit (void)
> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> {
> /* We can't use grub_fatal() in this function. This would create an
> infinite
> loop, since grub_fatal() calls grub_abort() which in turn calls
> grub_exit(). */
> diff --git a/grub-core/kern/ieee1275/init.c
> b/grub-core/kern/ieee1275/init.c
> index d5bd74d..996663f 100644
> --- a/grub-core/kern/ieee1275/init.c
> +++ b/grub-core/kern/ieee1275/init.c
> @@ -35,6 +35,7 @@
> #include <grub/offsets.h>
> #include <grub/memory.h>
> #include <grub/loader.h>
> +#include <grub/exitcode.h>
> #ifdef __i386__
> #include <grub/cpu/tsc.h>
> #endif
> @@ -60,7 +61,7 @@ grub_addr_t grub_ieee1275_original_stack;
> #endif
>
> void
> -grub_exit (void)
> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> {
> grub_ieee1275_exit ();
> }
> diff --git a/grub-core/kern/mips/arc/init.c
> b/grub-core/kern/mips/arc/init.c
> index 3834a14..28189b4 100644
> --- a/grub-core/kern/mips/arc/init.c
> +++ b/grub-core/kern/mips/arc/init.c
> @@ -36,6 +36,7 @@
> #include <grub/i18n.h>
> #include <grub/disk.h>
> #include <grub/partition.h>
> +#include <grub/exitcode.h>
>
> const char *type_names[] = {
> #ifdef GRUB_CPU_WORDS_BIGENDIAN
> @@ -276,7 +277,7 @@ grub_halt (void)
> }
>
> void
> -grub_exit (void)
> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> {
> GRUB_ARC_FIRMWARE_VECTOR->exit ();
>
> diff --git a/grub-core/kern/mips/loongson/init.c
> b/grub-core/kern/mips/loongson/init.c
> index 7b96531..5958653 100644
> --- a/grub-core/kern/mips/loongson/init.c
> +++ b/grub-core/kern/mips/loongson/init.c
> @@ -38,6 +38,7 @@
> #include <grub/serial.h>
> #include <grub/loader.h>
> #include <grub/at_keyboard.h>
> +#include <grub/exitcode.h>
>
> grub_err_t
> grub_machine_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
> @@ -304,7 +305,7 @@ grub_halt (void)
> }
>
> void
> -grub_exit (void)
> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> {
> grub_halt ();
> }
> diff --git a/grub-core/kern/mips/qemu_mips/init.c
> b/grub-core/kern/mips/qemu_mips/init.c
> index be88b77..b807b78 100644
> --- a/grub-core/kern/mips/qemu_mips/init.c
> +++ b/grub-core/kern/mips/qemu_mips/init.c
> @@ -17,6 +17,7 @@
> #include <grub/serial.h>
> #include <grub/loader.h>
> #include <grub/at_keyboard.h>
> +#include <grub/exitcode.h>
>
> static inline int
> probe_mem (grub_addr_t addr)
> @@ -75,7 +76,7 @@ grub_machine_fini (int flags __attribute__ ((unused)))
> }
>
> void
> -grub_exit (void)
> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> {
> grub_halt ();
> }
> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
> index 906d2c2..0db98cc 100644
> --- a/grub-core/kern/misc.c
> +++ b/grub-core/kern/misc.c
> @@ -24,6 +24,7 @@
> #include <grub/term.h>
> #include <grub/env.h>
> #include <grub/i18n.h>
> +#include <grub/exitcode.h>
>
> union printf_arg
> {
> @@ -1087,7 +1088,7 @@ grub_abort (void)
> grub_getkey ();
> }
>
> - grub_exit ();
> + grub_exit (GRUB_EXITCODE_NONE);
> }
>
> void
> diff --git a/grub-core/kern/uboot/init.c b/grub-core/kern/uboot/init.c
> index 5dcc106..1f27fa9 100644
> --- a/grub-core/kern/uboot/init.c
> +++ b/grub-core/kern/uboot/init.c
> @@ -32,6 +32,7 @@
> #include <grub/uboot/api_public.h>
> #include <grub/cpu/system.h>
> #include <grub/cache.h>
> +#include <grub/exitcode.h>
>
> extern char __bss_start[];
> extern char _end[];
> @@ -43,7 +44,7 @@ extern grub_uint32_t grub_uboot_machine_type;
> extern grub_addr_t grub_uboot_boot_data;
>
> void
> -grub_exit (void)
> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> {
> grub_uboot_return (0);
> }
> @@ -94,7 +95,7 @@ grub_machine_init (void)
> if (!ver)
> {
> /* Don't even have a console to log errors to... */
> - grub_exit ();
> + grub_exit (GRUB_EXITCODE_NONE);
> }
> else if (ver > API_SIG_VERSION)
> {
> diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
> index 0559c03..0fc3723 100644
> --- a/grub-core/kern/xen/init.c
> +++ b/grub-core/kern/xen/init.c
> @@ -549,7 +549,7 @@ grub_machine_init (void)
> }
>
> void
> -grub_exit (void)
> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> {
> struct sched_shutdown arg;
>
> diff --git a/include/grub/exitcode.h b/include/grub/exitcode.h
> new file mode 100644
> index 0000000..7f179f4
> --- /dev/null
> +++ b/include/grub/exitcode.h
> @@ -0,0 +1,30 @@
> +/* exitcode.h - declare exitcode types */
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2015 Free Software Foundation, Inc.
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef GRUB_EXITCODE_HEADER
> +#define GRUB_EXITCODE_HEADER 1
> +
> +typedef enum
> + {
> + GRUB_EXITCODE_NONE = 0,
> + GRUB_EXITCODE_NEXT,
> + }
> +grub_exitcode_t;
> +
> +#endif /* ! GRUB_EXITCODE_HEADER */
> diff --git a/include/grub/misc.h b/include/grub/misc.h
> index 2a9f87c..18dc77c 100644
> --- a/include/grub/misc.h
> +++ b/include/grub/misc.h
> @@ -26,6 +26,7 @@
> #include <grub/err.h>
> #include <grub/i18n.h>
> #include <grub/compiler.h>
> +#include <grub/exitcode.h>
>
> #define ALIGN_UP(addr, align) \
> ((addr + (typeof (addr)) align - 1) & ~((typeof (addr)) align - 1))
> @@ -334,7 +335,7 @@ int EXPORT_FUNC(grub_vsnprintf) (char *str,
> grub_size_t n, const char *fmt,
> char *EXPORT_FUNC(grub_xasprintf) (const char *fmt, ...)
> __attribute__ ((format (GNU_PRINTF, 1, 2))) WARN_UNUSED_RESULT;
> char *EXPORT_FUNC(grub_xvasprintf) (const char *fmt, va_list args)
> WARN_UNUSED_RESULT;
> -void EXPORT_FUNC(grub_exit) (void) __attribute__ ((noreturn));
> +void EXPORT_FUNC(grub_exit) (grub_exitcode_t exitcode) __attribute__
> ((noreturn));
> grub_uint64_t EXPORT_FUNC(grub_divmod64) (grub_uint64_t n,
> grub_uint64_t d,
> grub_uint64_t *r);
> --
> 2.5.0
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
--
--
Ben Hildred
Automation Support Services
303 815 6721
[-- Attachment #2: Type: text/html, Size: 16337 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [RFC] Add exitcode support
2015-08-18 18:03 ` Ben Hildred
@ 2015-08-18 18:56 ` Dann Frazier
2015-08-18 19:17 ` Ben Hildred
0 siblings, 1 reply; 8+ messages in thread
From: Dann Frazier @ 2015-08-18 18:56 UTC (permalink / raw)
To: The development of GNU GRUB
On Tue, Aug 18, 2015 at 12:03 PM, Ben Hildred <42656e@gmail.com> wrote:
> I only briefly scanned this, but this is cool (at least the idea is
> something I want to see implemented everywhere it makes sense to do so).
Cool, thanks for the feedback :)
> Just one humdinger of a question: does efi assume nonzero is an error? Does
> platform X assume a nonzero exit is an error? Do we want to assume nonzero
> is an error? What do we do when these assumptions do not agree?
This is the reason for the GRUB_EXITCODE abstraction. I thought we
could abstract out the behaviors users would like to request (e.g.,
try the next boot device, halt, report an error, etc), and then map
those to the proper behavior for the underlying platform (if
supported). Right now I've implemented two behaviors:
- GRUB_EXITCODE_NONE, which retains existing behavior
- GRUB_EXITCODE_NEXT (aka exit 1), meaning try the next
firmware-defined boot device.
Perhaps this should actually be string arguments to exit ('exit next')
instead of forcing the user to lookup integer codes. And perhaps it'd
be more honest to call them "hints" instead of "codes".
> My off the cuff recommendation is for grub to make no assumptions about how
> exit's value will be used in a general case but to instead have a per
> platform predefined variable (may EXITTYPE) which would provide to the
> script the semantics of the exit value. I would propose that the first three
> possible values be 'ignored', 'errorlevel', and 'zeroerror'. This would
> handle all current models that I am aware of. specifically it gives known
> true and false values everywhere.
I think we're generally on the same page. I do think I prefer passing
down an abstract request to the low level grub_exit() function vs. a
series of platform-specific variables because it gives the platform
code the ability to execute additional code if necessary - e.g.
setting fw environment variable.
-dann
> On Tue, Aug 18, 2015 at 11:30 AM, dann frazier <dann.frazier@canonical.com>
> wrote:
>>
>> Some platforms are capable of changing behavior based on the bootloader
>> exit code. In particular, UEFI boot managers use this code to determine
>> if they should try the next boot entry or not. I'd like to use this as
>> a way to make my PXE server tell clients to attempt to boot from their
>> next entry (presumably an on-disk OS), providing something akin to
>> pxelinux's "localboot".
>>
>> The implementation here is to add a new optional argument to the exit
>> command. The platform-specific grub_exit() implementations can then
>> translate it into a platform-appropriate exit code.
>>
>> Signed-off-by: dann frazier <dann.frazier@canonical.com>
>> ---
>> grub-core/commands/minicmd.c | 12 ++++++++----
>> grub-core/kern/efi/efi.c | 20 ++++++++++++++++++--
>> grub-core/kern/emu/misc.c | 3 ++-
>> grub-core/kern/i386/coreboot/init.c | 3 ++-
>> grub-core/kern/i386/qemu/init.c | 3 ++-
>> grub-core/kern/ieee1275/init.c | 3 ++-
>> grub-core/kern/mips/arc/init.c | 3 ++-
>> grub-core/kern/mips/loongson/init.c | 3 ++-
>> grub-core/kern/mips/qemu_mips/init.c | 3 ++-
>> grub-core/kern/misc.c | 3 ++-
>> grub-core/kern/uboot/init.c | 5 +++--
>> grub-core/kern/xen/init.c | 2 +-
>> include/grub/exitcode.h | 30 ++++++++++++++++++++++++++++++
>> include/grub/misc.h | 3 ++-
>> 14 files changed, 78 insertions(+), 18 deletions(-)
>> create mode 100644 include/grub/exitcode.h
>>
>> diff --git a/grub-core/commands/minicmd.c b/grub-core/commands/minicmd.c
>> index a3a1182..d67cdf1 100644
>> --- a/grub-core/commands/minicmd.c
>> +++ b/grub-core/commands/minicmd.c
>> @@ -28,6 +28,7 @@
>> #include <grub/loader.h>
>> #include <grub/command.h>
>> #include <grub/i18n.h>
>> +#include <grub/exitcode.h>
>>
>> GRUB_MOD_LICENSE ("GPLv3+");
>>
>> @@ -178,10 +179,13 @@ grub_mini_cmd_lsmod (struct grub_command *cmd
>> __attribute__ ((unused)),
>> /* exit */
>> static grub_err_t __attribute__ ((noreturn))
>> grub_mini_cmd_exit (struct grub_command *cmd __attribute__ ((unused)),
>> - int argc __attribute__ ((unused)),
>> - char *argv[] __attribute__ ((unused)))
>> + int argc, char *argv[])
>> +
>> {
>> - grub_exit ();
>> + grub_exitcode_t exitcode;
>> +
>> + exitcode = argc ? grub_strtoul(argv[0], 0, 10) : GRUB_EXITCODE_NONE;
>> + grub_exit (exitcode);
>> /* Not reached. */
>> }
>>
>> @@ -207,7 +211,7 @@ GRUB_MOD_INIT(minicmd)
>> 0, N_("Show loaded modules."));
>> cmd_exit =
>> grub_register_command ("exit", grub_mini_cmd_exit,
>> - 0, N_("Exit from GRUB."));
>> + N_("[EXITCODE]"), N_("Exit from GRUB."));
>> }
>>
>> GRUB_MOD_FINI(minicmd)
>> diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
>> index caf9bcc..33fb737 100644
>> --- a/grub-core/kern/efi/efi.c
>> +++ b/grub-core/kern/efi/efi.c
>> @@ -28,6 +28,7 @@
>> #include <grub/kernel.h>
>> #include <grub/mm.h>
>> #include <grub/loader.h>
>> +#include <grub/exitcode.h>
>>
>> /* The handle of GRUB itself. Filled in by the startup code. */
>> grub_efi_handle_t grub_efi_image_handle;
>> @@ -155,11 +156,26 @@ grub_efi_get_loaded_image (grub_efi_handle_t
>> image_handle)
>> }
>>
>> void
>> -grub_exit (void)
>> +grub_exit (grub_exitcode_t exitcode)
>> {
>> + grub_efi_status_t efi_status;
>> +
>> grub_machine_fini (GRUB_LOADER_FLAG_NORETURN);
>> +
>> + /* Map the GRUB exitcode to a suitable EFI status */
>> + switch (exitcode)
>> + {
>> + case GRUB_EXITCODE_NEXT:
>> + /* Anything other than EFI_SUCCESS will do (UEFI 2.5 section 3.1.1)
>> */
>> + efi_status = GRUB_EFI_LOAD_ERROR;
>> + break;
>> + default:
>> + efi_status = GRUB_EFI_SUCCESS;
>> + break;
>> + }
>> +
>> efi_call_4 (grub_efi_system_table->boot_services->exit,
>> - grub_efi_image_handle, GRUB_EFI_SUCCESS, 0, 0);
>> + grub_efi_image_handle, efi_status, 0, 0);
>> for (;;) ;
>> }
>>
>> diff --git a/grub-core/kern/emu/misc.c b/grub-core/kern/emu/misc.c
>> index bb606da..4f345d6 100644
>> --- a/grub-core/kern/emu/misc.c
>> +++ b/grub-core/kern/emu/misc.c
>> @@ -35,6 +35,7 @@
>> #include <grub/i18n.h>
>> #include <grub/time.h>
>> #include <grub/emu/misc.h>
>> +#include <grub/exitcode.h>
>>
>> int verbosity;
>>
>> @@ -135,7 +136,7 @@ xasprintf (const char *fmt, ...)
>> #endif
>>
>> void
>> -grub_exit (void)
>> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> {
>> exit (1);
>> }
>> diff --git a/grub-core/kern/i386/coreboot/init.c
>> b/grub-core/kern/i386/coreboot/init.c
>> index 3314f02..07294a1 100644
>> --- a/grub-core/kern/i386/coreboot/init.c
>> +++ b/grub-core/kern/i386/coreboot/init.c
>> @@ -35,13 +35,14 @@
>> #include <grub/cpu/floppy.h>
>> #include <grub/cpu/tsc.h>
>> #include <grub/video.h>
>> +#include <grub/exitcode.h>
>>
>> extern grub_uint8_t _start[];
>> extern grub_uint8_t _end[];
>> extern grub_uint8_t _edata[];
>>
>> void __attribute__ ((noreturn))
>> -grub_exit (void)
>> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> {
>> /* We can't use grub_fatal() in this function. This would create an
>> infinite
>> loop, since grub_fatal() calls grub_abort() which in turn calls
>> grub_exit(). */
>> diff --git a/grub-core/kern/i386/qemu/init.c
>> b/grub-core/kern/i386/qemu/init.c
>> index 271b6fb..67e155c 100644
>> --- a/grub-core/kern/i386/qemu/init.c
>> +++ b/grub-core/kern/i386/qemu/init.c
>> @@ -36,13 +36,14 @@
>> #include <grub/cpu/tsc.h>
>> #include <grub/machine/kernel.h>
>> #include <grub/pci.h>
>> +#include <grub/exitcode.h>
>>
>> extern grub_uint8_t _start[];
>> extern grub_uint8_t _end[];
>> extern grub_uint8_t _edata[];
>>
>> void __attribute__ ((noreturn))
>> -grub_exit (void)
>> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> {
>> /* We can't use grub_fatal() in this function. This would create an
>> infinite
>> loop, since grub_fatal() calls grub_abort() which in turn calls
>> grub_exit(). */
>> diff --git a/grub-core/kern/ieee1275/init.c
>> b/grub-core/kern/ieee1275/init.c
>> index d5bd74d..996663f 100644
>> --- a/grub-core/kern/ieee1275/init.c
>> +++ b/grub-core/kern/ieee1275/init.c
>> @@ -35,6 +35,7 @@
>> #include <grub/offsets.h>
>> #include <grub/memory.h>
>> #include <grub/loader.h>
>> +#include <grub/exitcode.h>
>> #ifdef __i386__
>> #include <grub/cpu/tsc.h>
>> #endif
>> @@ -60,7 +61,7 @@ grub_addr_t grub_ieee1275_original_stack;
>> #endif
>>
>> void
>> -grub_exit (void)
>> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> {
>> grub_ieee1275_exit ();
>> }
>> diff --git a/grub-core/kern/mips/arc/init.c
>> b/grub-core/kern/mips/arc/init.c
>> index 3834a14..28189b4 100644
>> --- a/grub-core/kern/mips/arc/init.c
>> +++ b/grub-core/kern/mips/arc/init.c
>> @@ -36,6 +36,7 @@
>> #include <grub/i18n.h>
>> #include <grub/disk.h>
>> #include <grub/partition.h>
>> +#include <grub/exitcode.h>
>>
>> const char *type_names[] = {
>> #ifdef GRUB_CPU_WORDS_BIGENDIAN
>> @@ -276,7 +277,7 @@ grub_halt (void)
>> }
>>
>> void
>> -grub_exit (void)
>> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> {
>> GRUB_ARC_FIRMWARE_VECTOR->exit ();
>>
>> diff --git a/grub-core/kern/mips/loongson/init.c
>> b/grub-core/kern/mips/loongson/init.c
>> index 7b96531..5958653 100644
>> --- a/grub-core/kern/mips/loongson/init.c
>> +++ b/grub-core/kern/mips/loongson/init.c
>> @@ -38,6 +38,7 @@
>> #include <grub/serial.h>
>> #include <grub/loader.h>
>> #include <grub/at_keyboard.h>
>> +#include <grub/exitcode.h>
>>
>> grub_err_t
>> grub_machine_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
>> @@ -304,7 +305,7 @@ grub_halt (void)
>> }
>>
>> void
>> -grub_exit (void)
>> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> {
>> grub_halt ();
>> }
>> diff --git a/grub-core/kern/mips/qemu_mips/init.c
>> b/grub-core/kern/mips/qemu_mips/init.c
>> index be88b77..b807b78 100644
>> --- a/grub-core/kern/mips/qemu_mips/init.c
>> +++ b/grub-core/kern/mips/qemu_mips/init.c
>> @@ -17,6 +17,7 @@
>> #include <grub/serial.h>
>> #include <grub/loader.h>
>> #include <grub/at_keyboard.h>
>> +#include <grub/exitcode.h>
>>
>> static inline int
>> probe_mem (grub_addr_t addr)
>> @@ -75,7 +76,7 @@ grub_machine_fini (int flags __attribute__ ((unused)))
>> }
>>
>> void
>> -grub_exit (void)
>> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> {
>> grub_halt ();
>> }
>> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
>> index 906d2c2..0db98cc 100644
>> --- a/grub-core/kern/misc.c
>> +++ b/grub-core/kern/misc.c
>> @@ -24,6 +24,7 @@
>> #include <grub/term.h>
>> #include <grub/env.h>
>> #include <grub/i18n.h>
>> +#include <grub/exitcode.h>
>>
>> union printf_arg
>> {
>> @@ -1087,7 +1088,7 @@ grub_abort (void)
>> grub_getkey ();
>> }
>>
>> - grub_exit ();
>> + grub_exit (GRUB_EXITCODE_NONE);
>> }
>>
>> void
>> diff --git a/grub-core/kern/uboot/init.c b/grub-core/kern/uboot/init.c
>> index 5dcc106..1f27fa9 100644
>> --- a/grub-core/kern/uboot/init.c
>> +++ b/grub-core/kern/uboot/init.c
>> @@ -32,6 +32,7 @@
>> #include <grub/uboot/api_public.h>
>> #include <grub/cpu/system.h>
>> #include <grub/cache.h>
>> +#include <grub/exitcode.h>
>>
>> extern char __bss_start[];
>> extern char _end[];
>> @@ -43,7 +44,7 @@ extern grub_uint32_t grub_uboot_machine_type;
>> extern grub_addr_t grub_uboot_boot_data;
>>
>> void
>> -grub_exit (void)
>> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> {
>> grub_uboot_return (0);
>> }
>> @@ -94,7 +95,7 @@ grub_machine_init (void)
>> if (!ver)
>> {
>> /* Don't even have a console to log errors to... */
>> - grub_exit ();
>> + grub_exit (GRUB_EXITCODE_NONE);
>> }
>> else if (ver > API_SIG_VERSION)
>> {
>> diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
>> index 0559c03..0fc3723 100644
>> --- a/grub-core/kern/xen/init.c
>> +++ b/grub-core/kern/xen/init.c
>> @@ -549,7 +549,7 @@ grub_machine_init (void)
>> }
>>
>> void
>> -grub_exit (void)
>> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> {
>> struct sched_shutdown arg;
>>
>> diff --git a/include/grub/exitcode.h b/include/grub/exitcode.h
>> new file mode 100644
>> index 0000000..7f179f4
>> --- /dev/null
>> +++ b/include/grub/exitcode.h
>> @@ -0,0 +1,30 @@
>> +/* exitcode.h - declare exitcode types */
>> +/*
>> + * GRUB -- GRand Unified Bootloader
>> + * Copyright (C) 2015 Free Software Foundation, Inc.
>> + *
>> + * GRUB is free software: you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, either version 3 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * GRUB is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef GRUB_EXITCODE_HEADER
>> +#define GRUB_EXITCODE_HEADER 1
>> +
>> +typedef enum
>> + {
>> + GRUB_EXITCODE_NONE = 0,
>> + GRUB_EXITCODE_NEXT,
>> + }
>> +grub_exitcode_t;
>> +
>> +#endif /* ! GRUB_EXITCODE_HEADER */
>> diff --git a/include/grub/misc.h b/include/grub/misc.h
>> index 2a9f87c..18dc77c 100644
>> --- a/include/grub/misc.h
>> +++ b/include/grub/misc.h
>> @@ -26,6 +26,7 @@
>> #include <grub/err.h>
>> #include <grub/i18n.h>
>> #include <grub/compiler.h>
>> +#include <grub/exitcode.h>
>>
>> #define ALIGN_UP(addr, align) \
>> ((addr + (typeof (addr)) align - 1) & ~((typeof (addr)) align -
>> 1))
>> @@ -334,7 +335,7 @@ int EXPORT_FUNC(grub_vsnprintf) (char *str,
>> grub_size_t n, const char *fmt,
>> char *EXPORT_FUNC(grub_xasprintf) (const char *fmt, ...)
>> __attribute__ ((format (GNU_PRINTF, 1, 2))) WARN_UNUSED_RESULT;
>> char *EXPORT_FUNC(grub_xvasprintf) (const char *fmt, va_list args)
>> WARN_UNUSED_RESULT;
>> -void EXPORT_FUNC(grub_exit) (void) __attribute__ ((noreturn));
>> +void EXPORT_FUNC(grub_exit) (grub_exitcode_t exitcode) __attribute__
>> ((noreturn));
>> grub_uint64_t EXPORT_FUNC(grub_divmod64) (grub_uint64_t n,
>> grub_uint64_t d,
>> grub_uint64_t *r);
>> --
>> 2.5.0
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>
>
>
>
> --
> --
> Ben Hildred
> Automation Support Services
> 303 815 6721
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [RFC] Add exitcode support
2015-08-18 18:56 ` Dann Frazier
@ 2015-08-18 19:17 ` Ben Hildred
2015-08-18 22:00 ` Dann Frazier
2016-01-22 19:06 ` Vladimir 'φ-coder/phcoder' Serbinenko
0 siblings, 2 replies; 8+ messages in thread
From: Ben Hildred @ 2015-08-18 19:17 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 17257 bytes --]
On Tue, Aug 18, 2015 at 12:56 PM, Dann Frazier <dann.frazier@canonical.com>
wrote:
> On Tue, Aug 18, 2015 at 12:03 PM, Ben Hildred <42656e@gmail.com> wrote:
> > I only briefly scanned this, but this is cool (at least the idea is
> > something I want to see implemented everywhere it makes sense to do so).
>
> Cool, thanks for the feedback :)
>
> > Just one humdinger of a question: does efi assume nonzero is an error?
> Does
> > platform X assume a nonzero exit is an error? Do we want to assume
> nonzero
> > is an error? What do we do when these assumptions do not agree?
>
> This is the reason for the GRUB_EXITCODE abstraction. I thought we
> could abstract out the behaviors users would like to request (e.g.,
> try the next boot device, halt, report an error, etc), and then map
> those to the proper behavior for the underlying platform (if
> supported). Right now I've implemented two behaviors:
>
> - GRUB_EXITCODE_NONE, which retains existing behavior
>
Is this true or false in a boolean context?
> - GRUB_EXITCODE_NEXT (aka exit 1), meaning try the next
> firmware-defined boot device.
>
Is this true or false in a boolean context?
>
> Perhaps this should actually be string arguments to exit ('exit next')
> instead of forcing the user to lookup integer codes. And perhaps it'd
> be more honest to call them "hints" instead of "codes".
>
> Let's assume for a minute that I have compiled grub as a multiboot image
and have called it from another bootloader, say iPXE.Now iPXE assumes that
any false return is an error. What happens when grub returns with exit
next, does iPXE get a true or false? What about exit fred where fred is not
defined by any platform? What if I do an exit config which is only defined
for coreboot?
> > My off the cuff recommendation is for grub to make no assumptions about
> how
> > exit's value will be used in a general case but to instead have a per
> > platform predefined variable (may EXITTYPE) which would provide to the
> > script the semantics of the exit value. I would propose that the first
> three
> > possible values be 'ignored', 'errorlevel', and 'zeroerror'. This would
> > handle all current models that I am aware of. specifically it gives known
> > true and false values everywhere.
>
> I think we're generally on the same page. I do think I prefer passing
> down an abstract request to the low level grub_exit() function vs. a
> series of platform-specific variables because it gives the platform
> code the ability to execute additional code if necessary - e.g.
> setting fw environment variable.
>
> I prefer an integer value being passed to exit, with a hint to tell us how
to interpret the value specifically indicating if zero is true (like shell)
or false (like perl). I would not be specifically opposed to string exits
(aside from code size issues), but string to number mapping may introduce
error conditions, and what do you do if exit fails?
-dann
>
> > On Tue, Aug 18, 2015 at 11:30 AM, dann frazier <
> dann.frazier@canonical.com>
> > wrote:
> >>
> >> Some platforms are capable of changing behavior based on the bootloader
> >> exit code. In particular, UEFI boot managers use this code to determine
> >> if they should try the next boot entry or not. I'd like to use this as
> >> a way to make my PXE server tell clients to attempt to boot from their
> >> next entry (presumably an on-disk OS), providing something akin to
> >> pxelinux's "localboot".
> >>
> >> The implementation here is to add a new optional argument to the exit
> >> command. The platform-specific grub_exit() implementations can then
> >> translate it into a platform-appropriate exit code.
> >>
> >> Signed-off-by: dann frazier <dann.frazier@canonical.com>
> >> ---
> >> grub-core/commands/minicmd.c | 12 ++++++++----
> >> grub-core/kern/efi/efi.c | 20 ++++++++++++++++++--
> >> grub-core/kern/emu/misc.c | 3 ++-
> >> grub-core/kern/i386/coreboot/init.c | 3 ++-
> >> grub-core/kern/i386/qemu/init.c | 3 ++-
> >> grub-core/kern/ieee1275/init.c | 3 ++-
> >> grub-core/kern/mips/arc/init.c | 3 ++-
> >> grub-core/kern/mips/loongson/init.c | 3 ++-
> >> grub-core/kern/mips/qemu_mips/init.c | 3 ++-
> >> grub-core/kern/misc.c | 3 ++-
> >> grub-core/kern/uboot/init.c | 5 +++--
> >> grub-core/kern/xen/init.c | 2 +-
> >> include/grub/exitcode.h | 30
> ++++++++++++++++++++++++++++++
> >> include/grub/misc.h | 3 ++-
> >> 14 files changed, 78 insertions(+), 18 deletions(-)
> >> create mode 100644 include/grub/exitcode.h
> >>
> >> diff --git a/grub-core/commands/minicmd.c b/grub-core/commands/minicmd.c
> >> index a3a1182..d67cdf1 100644
> >> --- a/grub-core/commands/minicmd.c
> >> +++ b/grub-core/commands/minicmd.c
> >> @@ -28,6 +28,7 @@
> >> #include <grub/loader.h>
> >> #include <grub/command.h>
> >> #include <grub/i18n.h>
> >> +#include <grub/exitcode.h>
> >>
> >> GRUB_MOD_LICENSE ("GPLv3+");
> >>
> >> @@ -178,10 +179,13 @@ grub_mini_cmd_lsmod (struct grub_command *cmd
> >> __attribute__ ((unused)),
> >> /* exit */
> >> static grub_err_t __attribute__ ((noreturn))
> >> grub_mini_cmd_exit (struct grub_command *cmd __attribute__ ((unused)),
> >> - int argc __attribute__ ((unused)),
> >> - char *argv[] __attribute__ ((unused)))
> >> + int argc, char *argv[])
> >> +
> >> {
> >> - grub_exit ();
> >> + grub_exitcode_t exitcode;
> >> +
> >> + exitcode = argc ? grub_strtoul(argv[0], 0, 10) : GRUB_EXITCODE_NONE;
> >> + grub_exit (exitcode);
> >> /* Not reached. */
> >> }
> >>
> >> @@ -207,7 +211,7 @@ GRUB_MOD_INIT(minicmd)
> >> 0, N_("Show loaded modules."));
> >> cmd_exit =
> >> grub_register_command ("exit", grub_mini_cmd_exit,
> >> - 0, N_("Exit from GRUB."));
> >> + N_("[EXITCODE]"), N_("Exit from GRUB."));
> >> }
> >>
> >> GRUB_MOD_FINI(minicmd)
> >> diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> >> index caf9bcc..33fb737 100644
> >> --- a/grub-core/kern/efi/efi.c
> >> +++ b/grub-core/kern/efi/efi.c
> >> @@ -28,6 +28,7 @@
> >> #include <grub/kernel.h>
> >> #include <grub/mm.h>
> >> #include <grub/loader.h>
> >> +#include <grub/exitcode.h>
> >>
> >> /* The handle of GRUB itself. Filled in by the startup code. */
> >> grub_efi_handle_t grub_efi_image_handle;
> >> @@ -155,11 +156,26 @@ grub_efi_get_loaded_image (grub_efi_handle_t
> >> image_handle)
> >> }
> >>
> >> void
> >> -grub_exit (void)
> >> +grub_exit (grub_exitcode_t exitcode)
> >> {
> >> + grub_efi_status_t efi_status;
> >> +
> >> grub_machine_fini (GRUB_LOADER_FLAG_NORETURN);
> >> +
> >> + /* Map the GRUB exitcode to a suitable EFI status */
> >> + switch (exitcode)
> >> + {
> >> + case GRUB_EXITCODE_NEXT:
> >> + /* Anything other than EFI_SUCCESS will do (UEFI 2.5 section
> 3.1.1)
> >> */
> >> + efi_status = GRUB_EFI_LOAD_ERROR;
> >> + break;
> >> + default:
> >> + efi_status = GRUB_EFI_SUCCESS;
> >> + break;
> >> + }
> >> +
> >> efi_call_4 (grub_efi_system_table->boot_services->exit,
> >> - grub_efi_image_handle, GRUB_EFI_SUCCESS, 0, 0);
> >> + grub_efi_image_handle, efi_status, 0, 0);
> >> for (;;) ;
> >> }
> >>
> >> diff --git a/grub-core/kern/emu/misc.c b/grub-core/kern/emu/misc.c
> >> index bb606da..4f345d6 100644
> >> --- a/grub-core/kern/emu/misc.c
> >> +++ b/grub-core/kern/emu/misc.c
> >> @@ -35,6 +35,7 @@
> >> #include <grub/i18n.h>
> >> #include <grub/time.h>
> >> #include <grub/emu/misc.h>
> >> +#include <grub/exitcode.h>
> >>
> >> int verbosity;
> >>
> >> @@ -135,7 +136,7 @@ xasprintf (const char *fmt, ...)
> >> #endif
> >>
> >> void
> >> -grub_exit (void)
> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> >> {
> >> exit (1);
> >> }
> >> diff --git a/grub-core/kern/i386/coreboot/init.c
> >> b/grub-core/kern/i386/coreboot/init.c
> >> index 3314f02..07294a1 100644
> >> --- a/grub-core/kern/i386/coreboot/init.c
> >> +++ b/grub-core/kern/i386/coreboot/init.c
> >> @@ -35,13 +35,14 @@
> >> #include <grub/cpu/floppy.h>
> >> #include <grub/cpu/tsc.h>
> >> #include <grub/video.h>
> >> +#include <grub/exitcode.h>
> >>
> >> extern grub_uint8_t _start[];
> >> extern grub_uint8_t _end[];
> >> extern grub_uint8_t _edata[];
> >>
> >> void __attribute__ ((noreturn))
> >> -grub_exit (void)
> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> >> {
> >> /* We can't use grub_fatal() in this function. This would create an
> >> infinite
> >> loop, since grub_fatal() calls grub_abort() which in turn calls
> >> grub_exit(). */
> >> diff --git a/grub-core/kern/i386/qemu/init.c
> >> b/grub-core/kern/i386/qemu/init.c
> >> index 271b6fb..67e155c 100644
> >> --- a/grub-core/kern/i386/qemu/init.c
> >> +++ b/grub-core/kern/i386/qemu/init.c
> >> @@ -36,13 +36,14 @@
> >> #include <grub/cpu/tsc.h>
> >> #include <grub/machine/kernel.h>
> >> #include <grub/pci.h>
> >> +#include <grub/exitcode.h>
> >>
> >> extern grub_uint8_t _start[];
> >> extern grub_uint8_t _end[];
> >> extern grub_uint8_t _edata[];
> >>
> >> void __attribute__ ((noreturn))
> >> -grub_exit (void)
> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> >> {
> >> /* We can't use grub_fatal() in this function. This would create an
> >> infinite
> >> loop, since grub_fatal() calls grub_abort() which in turn calls
> >> grub_exit(). */
> >> diff --git a/grub-core/kern/ieee1275/init.c
> >> b/grub-core/kern/ieee1275/init.c
> >> index d5bd74d..996663f 100644
> >> --- a/grub-core/kern/ieee1275/init.c
> >> +++ b/grub-core/kern/ieee1275/init.c
> >> @@ -35,6 +35,7 @@
> >> #include <grub/offsets.h>
> >> #include <grub/memory.h>
> >> #include <grub/loader.h>
> >> +#include <grub/exitcode.h>
> >> #ifdef __i386__
> >> #include <grub/cpu/tsc.h>
> >> #endif
> >> @@ -60,7 +61,7 @@ grub_addr_t grub_ieee1275_original_stack;
> >> #endif
> >>
> >> void
> >> -grub_exit (void)
> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> >> {
> >> grub_ieee1275_exit ();
> >> }
> >> diff --git a/grub-core/kern/mips/arc/init.c
> >> b/grub-core/kern/mips/arc/init.c
> >> index 3834a14..28189b4 100644
> >> --- a/grub-core/kern/mips/arc/init.c
> >> +++ b/grub-core/kern/mips/arc/init.c
> >> @@ -36,6 +36,7 @@
> >> #include <grub/i18n.h>
> >> #include <grub/disk.h>
> >> #include <grub/partition.h>
> >> +#include <grub/exitcode.h>
> >>
> >> const char *type_names[] = {
> >> #ifdef GRUB_CPU_WORDS_BIGENDIAN
> >> @@ -276,7 +277,7 @@ grub_halt (void)
> >> }
> >>
> >> void
> >> -grub_exit (void)
> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> >> {
> >> GRUB_ARC_FIRMWARE_VECTOR->exit ();
> >>
> >> diff --git a/grub-core/kern/mips/loongson/init.c
> >> b/grub-core/kern/mips/loongson/init.c
> >> index 7b96531..5958653 100644
> >> --- a/grub-core/kern/mips/loongson/init.c
> >> +++ b/grub-core/kern/mips/loongson/init.c
> >> @@ -38,6 +38,7 @@
> >> #include <grub/serial.h>
> >> #include <grub/loader.h>
> >> #include <grub/at_keyboard.h>
> >> +#include <grub/exitcode.h>
> >>
> >> grub_err_t
> >> grub_machine_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
> >> @@ -304,7 +305,7 @@ grub_halt (void)
> >> }
> >>
> >> void
> >> -grub_exit (void)
> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> >> {
> >> grub_halt ();
> >> }
> >> diff --git a/grub-core/kern/mips/qemu_mips/init.c
> >> b/grub-core/kern/mips/qemu_mips/init.c
> >> index be88b77..b807b78 100644
> >> --- a/grub-core/kern/mips/qemu_mips/init.c
> >> +++ b/grub-core/kern/mips/qemu_mips/init.c
> >> @@ -17,6 +17,7 @@
> >> #include <grub/serial.h>
> >> #include <grub/loader.h>
> >> #include <grub/at_keyboard.h>
> >> +#include <grub/exitcode.h>
> >>
> >> static inline int
> >> probe_mem (grub_addr_t addr)
> >> @@ -75,7 +76,7 @@ grub_machine_fini (int flags __attribute__ ((unused)))
> >> }
> >>
> >> void
> >> -grub_exit (void)
> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> >> {
> >> grub_halt ();
> >> }
> >> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
> >> index 906d2c2..0db98cc 100644
> >> --- a/grub-core/kern/misc.c
> >> +++ b/grub-core/kern/misc.c
> >> @@ -24,6 +24,7 @@
> >> #include <grub/term.h>
> >> #include <grub/env.h>
> >> #include <grub/i18n.h>
> >> +#include <grub/exitcode.h>
> >>
> >> union printf_arg
> >> {
> >> @@ -1087,7 +1088,7 @@ grub_abort (void)
> >> grub_getkey ();
> >> }
> >>
> >> - grub_exit ();
> >> + grub_exit (GRUB_EXITCODE_NONE);
> >> }
> >>
> >> void
> >> diff --git a/grub-core/kern/uboot/init.c b/grub-core/kern/uboot/init.c
> >> index 5dcc106..1f27fa9 100644
> >> --- a/grub-core/kern/uboot/init.c
> >> +++ b/grub-core/kern/uboot/init.c
> >> @@ -32,6 +32,7 @@
> >> #include <grub/uboot/api_public.h>
> >> #include <grub/cpu/system.h>
> >> #include <grub/cache.h>
> >> +#include <grub/exitcode.h>
> >>
> >> extern char __bss_start[];
> >> extern char _end[];
> >> @@ -43,7 +44,7 @@ extern grub_uint32_t grub_uboot_machine_type;
> >> extern grub_addr_t grub_uboot_boot_data;
> >>
> >> void
> >> -grub_exit (void)
> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> >> {
> >> grub_uboot_return (0);
> >> }
> >> @@ -94,7 +95,7 @@ grub_machine_init (void)
> >> if (!ver)
> >> {
> >> /* Don't even have a console to log errors to... */
> >> - grub_exit ();
> >> + grub_exit (GRUB_EXITCODE_NONE);
> >> }
> >> else if (ver > API_SIG_VERSION)
> >> {
> >> diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
> >> index 0559c03..0fc3723 100644
> >> --- a/grub-core/kern/xen/init.c
> >> +++ b/grub-core/kern/xen/init.c
> >> @@ -549,7 +549,7 @@ grub_machine_init (void)
> >> }
> >>
> >> void
> >> -grub_exit (void)
> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
> >> {
> >> struct sched_shutdown arg;
> >>
> >> diff --git a/include/grub/exitcode.h b/include/grub/exitcode.h
> >> new file mode 100644
> >> index 0000000..7f179f4
> >> --- /dev/null
> >> +++ b/include/grub/exitcode.h
> >> @@ -0,0 +1,30 @@
> >> +/* exitcode.h - declare exitcode types */
> >> +/*
> >> + * GRUB -- GRand Unified Bootloader
> >> + * Copyright (C) 2015 Free Software Foundation, Inc.
> >> + *
> >> + * GRUB is free software: you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License as published
> by
> >> + * the Free Software Foundation, either version 3 of the License, or
> >> + * (at your option) any later version.
> >> + *
> >> + * GRUB is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> >> + */
> >> +
> >> +#ifndef GRUB_EXITCODE_HEADER
> >> +#define GRUB_EXITCODE_HEADER 1
> >> +
> >> +typedef enum
> >> + {
> >> + GRUB_EXITCODE_NONE = 0,
> >> + GRUB_EXITCODE_NEXT,
> >> + }
> >> +grub_exitcode_t;
> >> +
> >> +#endif /* ! GRUB_EXITCODE_HEADER */
> >> diff --git a/include/grub/misc.h b/include/grub/misc.h
> >> index 2a9f87c..18dc77c 100644
> >> --- a/include/grub/misc.h
> >> +++ b/include/grub/misc.h
> >> @@ -26,6 +26,7 @@
> >> #include <grub/err.h>
> >> #include <grub/i18n.h>
> >> #include <grub/compiler.h>
> >> +#include <grub/exitcode.h>
> >>
> >> #define ALIGN_UP(addr, align) \
> >> ((addr + (typeof (addr)) align - 1) & ~((typeof (addr)) align -
> >> 1))
> >> @@ -334,7 +335,7 @@ int EXPORT_FUNC(grub_vsnprintf) (char *str,
> >> grub_size_t n, const char *fmt,
> >> char *EXPORT_FUNC(grub_xasprintf) (const char *fmt, ...)
> >> __attribute__ ((format (GNU_PRINTF, 1, 2))) WARN_UNUSED_RESULT;
> >> char *EXPORT_FUNC(grub_xvasprintf) (const char *fmt, va_list args)
> >> WARN_UNUSED_RESULT;
> >> -void EXPORT_FUNC(grub_exit) (void) __attribute__ ((noreturn));
> >> +void EXPORT_FUNC(grub_exit) (grub_exitcode_t exitcode) __attribute__
> >> ((noreturn));
> >> grub_uint64_t EXPORT_FUNC(grub_divmod64) (grub_uint64_t n,
> >> grub_uint64_t d,
> >> grub_uint64_t *r);
> >> --
> >> 2.5.0
> >>
> >>
> >> _______________________________________________
> >> Grub-devel mailing list
> >> Grub-devel@gnu.org
> >> https://lists.gnu.org/mailman/listinfo/grub-devel
> >
> >
> >
> >
> > --
> > --
> > Ben Hildred
> > Automation Support Services
> > 303 815 6721
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
--
--
Ben Hildred
Automation Support Services
303 815 6721
[-- Attachment #2: Type: text/html, Size: 23973 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [RFC] Add exitcode support
2015-08-18 19:17 ` Ben Hildred
@ 2015-08-18 22:00 ` Dann Frazier
2016-01-22 19:06 ` Vladimir 'φ-coder/phcoder' Serbinenko
1 sibling, 0 replies; 8+ messages in thread
From: Dann Frazier @ 2015-08-18 22:00 UTC (permalink / raw)
To: The development of GNU GRUB
On Tue, Aug 18, 2015 at 1:17 PM, Ben Hildred <42656e@gmail.com> wrote:
>
>
> On Tue, Aug 18, 2015 at 12:56 PM, Dann Frazier <dann.frazier@canonical.com>
> wrote:
>>
>> On Tue, Aug 18, 2015 at 12:03 PM, Ben Hildred <42656e@gmail.com> wrote:
>> > I only briefly scanned this, but this is cool (at least the idea is
>> > something I want to see implemented everywhere it makes sense to do so).
>>
>> Cool, thanks for the feedback :)
>>
>> > Just one humdinger of a question: does efi assume nonzero is an error?
>> > Does
>> > platform X assume a nonzero exit is an error? Do we want to assume
>> > nonzero
>> > is an error? What do we do when these assumptions do not agree?
>>
>> This is the reason for the GRUB_EXITCODE abstraction. I thought we
>> could abstract out the behaviors users would like to request (e.g.,
>> try the next boot device, halt, report an error, etc), and then map
>> those to the proper behavior for the underlying platform (if
>> supported). Right now I've implemented two behaviors:
>>
>> - GRUB_EXITCODE_NONE, which retains existing behavior
>
> Is this true or false in a boolean context?
>>
>> - GRUB_EXITCODE_NEXT (aka exit 1), meaning try the next
>> firmware-defined boot device.
>
> Is this true or false in a boolean context?
I'm not sure a boolean context makes sense here. Any return from a
bootloader could be interpreted as some kind of error, so what would a
generic success imply?. What I really want is a way to give hints to
the platform for what to do next, if the platform supports them. And
that has me now leaning more towards flags (see below).
>>
>> Perhaps this should actually be string arguments to exit ('exit next')
>> instead of forcing the user to lookup integer codes. And perhaps it'd
>> be more honest to call them "hints" instead of "codes".
>>
> Let's assume for a minute that I have compiled grub as a multiboot image and
> have called it from another bootloader, say iPXE.Now iPXE assumes that any
> false return is an error. What happens when grub returns with exit next,
> does iPXE get a true or false?
In this architecture, that'd be up to the platform grub_exit()
implementation. If
that code knows the platform does not support a "next" equivalent, it would
presumably return an error (false).
> What about exit fred where fred is not
> defined by any platform? What if I do an exit config which is only defined
> for coreboot?
If the platform doesn't support that feature, it should return a
reasonable value for that platform. Presumably an error, if the
platform supports them.
>>
>> > My off the cuff recommendation is for grub to make no assumptions about
>> > how
>> > exit's value will be used in a general case but to instead have a per
>> > platform predefined variable (may EXITTYPE) which would provide to the
>> > script the semantics of the exit value. I would propose that the first
>> > three
>> > possible values be 'ignored', 'errorlevel', and 'zeroerror'. This would
>> > handle all current models that I am aware of. specifically it gives
>> > known
>> > true and false values everywhere.
>>
>> I think we're generally on the same page. I do think I prefer passing
>> down an abstract request to the low level grub_exit() function vs. a
>> series of platform-specific variables because it gives the platform
>> code the ability to execute additional code if necessary - e.g.
>> setting fw environment variable.
>>
> I prefer an integer value being passed to exit, with a hint to tell us how
> to interpret the value specifically indicating if zero is true (like shell)
> or false (like perl).
>
> I would not be specifically opposed to string exits
> (aside from code size issues), but string to number mapping may introduce
> error conditions, and what do you do if exit fails?
Perhaps this would be better handled as one or more grub variables
that can be set before exit; e.g. 'set exit_try_next=1'. That would
also allow for multiple hints to be set if that ever became
interesting.
-dann
>> -dann
>>
>> > On Tue, Aug 18, 2015 at 11:30 AM, dann frazier
>> > <dann.frazier@canonical.com>
>> > wrote:
>> >>
>> >> Some platforms are capable of changing behavior based on the bootloader
>> >> exit code. In particular, UEFI boot managers use this code to determine
>> >> if they should try the next boot entry or not. I'd like to use this as
>> >> a way to make my PXE server tell clients to attempt to boot from their
>> >> next entry (presumably an on-disk OS), providing something akin to
>> >> pxelinux's "localboot".
>> >>
>> >> The implementation here is to add a new optional argument to the exit
>> >> command. The platform-specific grub_exit() implementations can then
>> >> translate it into a platform-appropriate exit code.
>> >>
>> >> Signed-off-by: dann frazier <dann.frazier@canonical.com>
>> >> ---
>> >> grub-core/commands/minicmd.c | 12 ++++++++----
>> >> grub-core/kern/efi/efi.c | 20 ++++++++++++++++++--
>> >> grub-core/kern/emu/misc.c | 3 ++-
>> >> grub-core/kern/i386/coreboot/init.c | 3 ++-
>> >> grub-core/kern/i386/qemu/init.c | 3 ++-
>> >> grub-core/kern/ieee1275/init.c | 3 ++-
>> >> grub-core/kern/mips/arc/init.c | 3 ++-
>> >> grub-core/kern/mips/loongson/init.c | 3 ++-
>> >> grub-core/kern/mips/qemu_mips/init.c | 3 ++-
>> >> grub-core/kern/misc.c | 3 ++-
>> >> grub-core/kern/uboot/init.c | 5 +++--
>> >> grub-core/kern/xen/init.c | 2 +-
>> >> include/grub/exitcode.h | 30
>> >> ++++++++++++++++++++++++++++++
>> >> include/grub/misc.h | 3 ++-
>> >> 14 files changed, 78 insertions(+), 18 deletions(-)
>> >> create mode 100644 include/grub/exitcode.h
>> >>
>> >> diff --git a/grub-core/commands/minicmd.c
>> >> b/grub-core/commands/minicmd.c
>> >> index a3a1182..d67cdf1 100644
>> >> --- a/grub-core/commands/minicmd.c
>> >> +++ b/grub-core/commands/minicmd.c
>> >> @@ -28,6 +28,7 @@
>> >> #include <grub/loader.h>
>> >> #include <grub/command.h>
>> >> #include <grub/i18n.h>
>> >> +#include <grub/exitcode.h>
>> >>
>> >> GRUB_MOD_LICENSE ("GPLv3+");
>> >>
>> >> @@ -178,10 +179,13 @@ grub_mini_cmd_lsmod (struct grub_command *cmd
>> >> __attribute__ ((unused)),
>> >> /* exit */
>> >> static grub_err_t __attribute__ ((noreturn))
>> >> grub_mini_cmd_exit (struct grub_command *cmd __attribute__ ((unused)),
>> >> - int argc __attribute__ ((unused)),
>> >> - char *argv[] __attribute__ ((unused)))
>> >> + int argc, char *argv[])
>> >> +
>> >> {
>> >> - grub_exit ();
>> >> + grub_exitcode_t exitcode;
>> >> +
>> >> + exitcode = argc ? grub_strtoul(argv[0], 0, 10) : GRUB_EXITCODE_NONE;
>> >> + grub_exit (exitcode);
>> >> /* Not reached. */
>> >> }
>> >>
>> >> @@ -207,7 +211,7 @@ GRUB_MOD_INIT(minicmd)
>> >> 0, N_("Show loaded modules."));
>> >> cmd_exit =
>> >> grub_register_command ("exit", grub_mini_cmd_exit,
>> >> - 0, N_("Exit from GRUB."));
>> >> + N_("[EXITCODE]"), N_("Exit from GRUB."));
>> >> }
>> >>
>> >> GRUB_MOD_FINI(minicmd)
>> >> diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
>> >> index caf9bcc..33fb737 100644
>> >> --- a/grub-core/kern/efi/efi.c
>> >> +++ b/grub-core/kern/efi/efi.c
>> >> @@ -28,6 +28,7 @@
>> >> #include <grub/kernel.h>
>> >> #include <grub/mm.h>
>> >> #include <grub/loader.h>
>> >> +#include <grub/exitcode.h>
>> >>
>> >> /* The handle of GRUB itself. Filled in by the startup code. */
>> >> grub_efi_handle_t grub_efi_image_handle;
>> >> @@ -155,11 +156,26 @@ grub_efi_get_loaded_image (grub_efi_handle_t
>> >> image_handle)
>> >> }
>> >>
>> >> void
>> >> -grub_exit (void)
>> >> +grub_exit (grub_exitcode_t exitcode)
>> >> {
>> >> + grub_efi_status_t efi_status;
>> >> +
>> >> grub_machine_fini (GRUB_LOADER_FLAG_NORETURN);
>> >> +
>> >> + /* Map the GRUB exitcode to a suitable EFI status */
>> >> + switch (exitcode)
>> >> + {
>> >> + case GRUB_EXITCODE_NEXT:
>> >> + /* Anything other than EFI_SUCCESS will do (UEFI 2.5 section
>> >> 3.1.1)
>> >> */
>> >> + efi_status = GRUB_EFI_LOAD_ERROR;
>> >> + break;
>> >> + default:
>> >> + efi_status = GRUB_EFI_SUCCESS;
>> >> + break;
>> >> + }
>> >> +
>> >> efi_call_4 (grub_efi_system_table->boot_services->exit,
>> >> - grub_efi_image_handle, GRUB_EFI_SUCCESS, 0, 0);
>> >> + grub_efi_image_handle, efi_status, 0, 0);
>> >> for (;;) ;
>> >> }
>> >>
>> >> diff --git a/grub-core/kern/emu/misc.c b/grub-core/kern/emu/misc.c
>> >> index bb606da..4f345d6 100644
>> >> --- a/grub-core/kern/emu/misc.c
>> >> +++ b/grub-core/kern/emu/misc.c
>> >> @@ -35,6 +35,7 @@
>> >> #include <grub/i18n.h>
>> >> #include <grub/time.h>
>> >> #include <grub/emu/misc.h>
>> >> +#include <grub/exitcode.h>
>> >>
>> >> int verbosity;
>> >>
>> >> @@ -135,7 +136,7 @@ xasprintf (const char *fmt, ...)
>> >> #endif
>> >>
>> >> void
>> >> -grub_exit (void)
>> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> >> {
>> >> exit (1);
>> >> }
>> >> diff --git a/grub-core/kern/i386/coreboot/init.c
>> >> b/grub-core/kern/i386/coreboot/init.c
>> >> index 3314f02..07294a1 100644
>> >> --- a/grub-core/kern/i386/coreboot/init.c
>> >> +++ b/grub-core/kern/i386/coreboot/init.c
>> >> @@ -35,13 +35,14 @@
>> >> #include <grub/cpu/floppy.h>
>> >> #include <grub/cpu/tsc.h>
>> >> #include <grub/video.h>
>> >> +#include <grub/exitcode.h>
>> >>
>> >> extern grub_uint8_t _start[];
>> >> extern grub_uint8_t _end[];
>> >> extern grub_uint8_t _edata[];
>> >>
>> >> void __attribute__ ((noreturn))
>> >> -grub_exit (void)
>> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> >> {
>> >> /* We can't use grub_fatal() in this function. This would create an
>> >> infinite
>> >> loop, since grub_fatal() calls grub_abort() which in turn calls
>> >> grub_exit(). */
>> >> diff --git a/grub-core/kern/i386/qemu/init.c
>> >> b/grub-core/kern/i386/qemu/init.c
>> >> index 271b6fb..67e155c 100644
>> >> --- a/grub-core/kern/i386/qemu/init.c
>> >> +++ b/grub-core/kern/i386/qemu/init.c
>> >> @@ -36,13 +36,14 @@
>> >> #include <grub/cpu/tsc.h>
>> >> #include <grub/machine/kernel.h>
>> >> #include <grub/pci.h>
>> >> +#include <grub/exitcode.h>
>> >>
>> >> extern grub_uint8_t _start[];
>> >> extern grub_uint8_t _end[];
>> >> extern grub_uint8_t _edata[];
>> >>
>> >> void __attribute__ ((noreturn))
>> >> -grub_exit (void)
>> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> >> {
>> >> /* We can't use grub_fatal() in this function. This would create an
>> >> infinite
>> >> loop, since grub_fatal() calls grub_abort() which in turn calls
>> >> grub_exit(). */
>> >> diff --git a/grub-core/kern/ieee1275/init.c
>> >> b/grub-core/kern/ieee1275/init.c
>> >> index d5bd74d..996663f 100644
>> >> --- a/grub-core/kern/ieee1275/init.c
>> >> +++ b/grub-core/kern/ieee1275/init.c
>> >> @@ -35,6 +35,7 @@
>> >> #include <grub/offsets.h>
>> >> #include <grub/memory.h>
>> >> #include <grub/loader.h>
>> >> +#include <grub/exitcode.h>
>> >> #ifdef __i386__
>> >> #include <grub/cpu/tsc.h>
>> >> #endif
>> >> @@ -60,7 +61,7 @@ grub_addr_t grub_ieee1275_original_stack;
>> >> #endif
>> >>
>> >> void
>> >> -grub_exit (void)
>> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> >> {
>> >> grub_ieee1275_exit ();
>> >> }
>> >> diff --git a/grub-core/kern/mips/arc/init.c
>> >> b/grub-core/kern/mips/arc/init.c
>> >> index 3834a14..28189b4 100644
>> >> --- a/grub-core/kern/mips/arc/init.c
>> >> +++ b/grub-core/kern/mips/arc/init.c
>> >> @@ -36,6 +36,7 @@
>> >> #include <grub/i18n.h>
>> >> #include <grub/disk.h>
>> >> #include <grub/partition.h>
>> >> +#include <grub/exitcode.h>
>> >>
>> >> const char *type_names[] = {
>> >> #ifdef GRUB_CPU_WORDS_BIGENDIAN
>> >> @@ -276,7 +277,7 @@ grub_halt (void)
>> >> }
>> >>
>> >> void
>> >> -grub_exit (void)
>> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> >> {
>> >> GRUB_ARC_FIRMWARE_VECTOR->exit ();
>> >>
>> >> diff --git a/grub-core/kern/mips/loongson/init.c
>> >> b/grub-core/kern/mips/loongson/init.c
>> >> index 7b96531..5958653 100644
>> >> --- a/grub-core/kern/mips/loongson/init.c
>> >> +++ b/grub-core/kern/mips/loongson/init.c
>> >> @@ -38,6 +38,7 @@
>> >> #include <grub/serial.h>
>> >> #include <grub/loader.h>
>> >> #include <grub/at_keyboard.h>
>> >> +#include <grub/exitcode.h>
>> >>
>> >> grub_err_t
>> >> grub_machine_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
>> >> @@ -304,7 +305,7 @@ grub_halt (void)
>> >> }
>> >>
>> >> void
>> >> -grub_exit (void)
>> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> >> {
>> >> grub_halt ();
>> >> }
>> >> diff --git a/grub-core/kern/mips/qemu_mips/init.c
>> >> b/grub-core/kern/mips/qemu_mips/init.c
>> >> index be88b77..b807b78 100644
>> >> --- a/grub-core/kern/mips/qemu_mips/init.c
>> >> +++ b/grub-core/kern/mips/qemu_mips/init.c
>> >> @@ -17,6 +17,7 @@
>> >> #include <grub/serial.h>
>> >> #include <grub/loader.h>
>> >> #include <grub/at_keyboard.h>
>> >> +#include <grub/exitcode.h>
>> >>
>> >> static inline int
>> >> probe_mem (grub_addr_t addr)
>> >> @@ -75,7 +76,7 @@ grub_machine_fini (int flags __attribute__
>> >> ((unused)))
>> >> }
>> >>
>> >> void
>> >> -grub_exit (void)
>> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> >> {
>> >> grub_halt ();
>> >> }
>> >> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
>> >> index 906d2c2..0db98cc 100644
>> >> --- a/grub-core/kern/misc.c
>> >> +++ b/grub-core/kern/misc.c
>> >> @@ -24,6 +24,7 @@
>> >> #include <grub/term.h>
>> >> #include <grub/env.h>
>> >> #include <grub/i18n.h>
>> >> +#include <grub/exitcode.h>
>> >>
>> >> union printf_arg
>> >> {
>> >> @@ -1087,7 +1088,7 @@ grub_abort (void)
>> >> grub_getkey ();
>> >> }
>> >>
>> >> - grub_exit ();
>> >> + grub_exit (GRUB_EXITCODE_NONE);
>> >> }
>> >>
>> >> void
>> >> diff --git a/grub-core/kern/uboot/init.c b/grub-core/kern/uboot/init.c
>> >> index 5dcc106..1f27fa9 100644
>> >> --- a/grub-core/kern/uboot/init.c
>> >> +++ b/grub-core/kern/uboot/init.c
>> >> @@ -32,6 +32,7 @@
>> >> #include <grub/uboot/api_public.h>
>> >> #include <grub/cpu/system.h>
>> >> #include <grub/cache.h>
>> >> +#include <grub/exitcode.h>
>> >>
>> >> extern char __bss_start[];
>> >> extern char _end[];
>> >> @@ -43,7 +44,7 @@ extern grub_uint32_t grub_uboot_machine_type;
>> >> extern grub_addr_t grub_uboot_boot_data;
>> >>
>> >> void
>> >> -grub_exit (void)
>> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> >> {
>> >> grub_uboot_return (0);
>> >> }
>> >> @@ -94,7 +95,7 @@ grub_machine_init (void)
>> >> if (!ver)
>> >> {
>> >> /* Don't even have a console to log errors to... */
>> >> - grub_exit ();
>> >> + grub_exit (GRUB_EXITCODE_NONE);
>> >> }
>> >> else if (ver > API_SIG_VERSION)
>> >> {
>> >> diff --git a/grub-core/kern/xen/init.c b/grub-core/kern/xen/init.c
>> >> index 0559c03..0fc3723 100644
>> >> --- a/grub-core/kern/xen/init.c
>> >> +++ b/grub-core/kern/xen/init.c
>> >> @@ -549,7 +549,7 @@ grub_machine_init (void)
>> >> }
>> >>
>> >> void
>> >> -grub_exit (void)
>> >> +grub_exit (grub_exitcode_t exitcode __attribute__ ((unused)))
>> >> {
>> >> struct sched_shutdown arg;
>> >>
>> >> diff --git a/include/grub/exitcode.h b/include/grub/exitcode.h
>> >> new file mode 100644
>> >> index 0000000..7f179f4
>> >> --- /dev/null
>> >> +++ b/include/grub/exitcode.h
>> >> @@ -0,0 +1,30 @@
>> >> +/* exitcode.h - declare exitcode types */
>> >> +/*
>> >> + * GRUB -- GRand Unified Bootloader
>> >> + * Copyright (C) 2015 Free Software Foundation, Inc.
>> >> + *
>> >> + * GRUB is free software: you can redistribute it and/or modify
>> >> + * it under the terms of the GNU General Public License as published
>> >> by
>> >> + * the Free Software Foundation, either version 3 of the License, or
>> >> + * (at your option) any later version.
>> >> + *
>> >> + * GRUB is distributed in the hope that it will be useful,
>> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> >> + * GNU General Public License for more details.
>> >> + *
>> >> + * You should have received a copy of the GNU General Public License
>> >> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
>> >> + */
>> >> +
>> >> +#ifndef GRUB_EXITCODE_HEADER
>> >> +#define GRUB_EXITCODE_HEADER 1
>> >> +
>> >> +typedef enum
>> >> + {
>> >> + GRUB_EXITCODE_NONE = 0,
>> >> + GRUB_EXITCODE_NEXT,
>> >> + }
>> >> +grub_exitcode_t;
>> >> +
>> >> +#endif /* ! GRUB_EXITCODE_HEADER */
>> >> diff --git a/include/grub/misc.h b/include/grub/misc.h
>> >> index 2a9f87c..18dc77c 100644
>> >> --- a/include/grub/misc.h
>> >> +++ b/include/grub/misc.h
>> >> @@ -26,6 +26,7 @@
>> >> #include <grub/err.h>
>> >> #include <grub/i18n.h>
>> >> #include <grub/compiler.h>
>> >> +#include <grub/exitcode.h>
>> >>
>> >> #define ALIGN_UP(addr, align) \
>> >> ((addr + (typeof (addr)) align - 1) & ~((typeof (addr)) align -
>> >> 1))
>> >> @@ -334,7 +335,7 @@ int EXPORT_FUNC(grub_vsnprintf) (char *str,
>> >> grub_size_t n, const char *fmt,
>> >> char *EXPORT_FUNC(grub_xasprintf) (const char *fmt, ...)
>> >> __attribute__ ((format (GNU_PRINTF, 1, 2))) WARN_UNUSED_RESULT;
>> >> char *EXPORT_FUNC(grub_xvasprintf) (const char *fmt, va_list args)
>> >> WARN_UNUSED_RESULT;
>> >> -void EXPORT_FUNC(grub_exit) (void) __attribute__ ((noreturn));
>> >> +void EXPORT_FUNC(grub_exit) (grub_exitcode_t exitcode) __attribute__
>> >> ((noreturn));
>> >> grub_uint64_t EXPORT_FUNC(grub_divmod64) (grub_uint64_t n,
>> >> grub_uint64_t d,
>> >> grub_uint64_t *r);
>> >> --
>> >> 2.5.0
>> >>
>> >>
>> >> _______________________________________________
>> >> Grub-devel mailing list
>> >> Grub-devel@gnu.org
>> >> https://lists.gnu.org/mailman/listinfo/grub-devel
>> >
>> >
>> >
>> >
>> > --
>> > --
>> > Ben Hildred
>> > Automation Support Services
>> > 303 815 6721
>> >
>> > _______________________________________________
>> > Grub-devel mailing list
>> > Grub-devel@gnu.org
>> > https://lists.gnu.org/mailman/listinfo/grub-devel
>> >
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>
>
>
>
> --
> --
> Ben Hildred
> Automation Support Services
> 303 815 6721
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [RFC] Add exitcode support
2015-08-18 19:17 ` Ben Hildred
2015-08-18 22:00 ` Dann Frazier
@ 2016-01-22 19:06 ` Vladimir 'φ-coder/phcoder' Serbinenko
2016-01-23 8:26 ` Andrei Borzenkov
1 sibling, 1 reply; 8+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2016-01-22 19:06 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 1330 bytes --]
On 18.08.2015 21:17, Ben Hildred wrote:
> Let's assume for a minute that I have compiled grub as a multiboot image
> and have called it from another bootloader, say iPXE.Now iPXE assumes
> that any false return is an error. What happens when grub returns with
> exit next, does iPXE get a true or false? What about exit fred where
> fred is not defined by any platform? What if I do an exit config which
> is only defined for coreboot?
Neither multiboot nor coreboot have any return semantics. The situation
with current platforms is as follows:
No return/exit semantics at all or machine shutdown:
i386_coreboot, i386_qemu, i386_multiboot, mips_qemu_mips, mips_loongson
no-args exit:
*-ieee1275, i386-pc, mips-arc
Xen semantics (crash vs poweroff):
*-xen
EFI semantics:
*-efi
Unix-like semantics:
arm-uboot, emu
Emu is of no real interest and I have no idea what Uboot does with
return code but I suppose nothing.
This leaves us only with xen and EFI semantics. Xen is enough of outlier
to handle it separately. So only EFI is remaining.
On i386-pc the default behaviour of exit is to try next boot entry. EFI
should probably do the same. What is the current behaviour of grub_exit
and what is the value in returning EFI_SUCCESS ?
Can we have returncode-aware command to be EFI-specific?
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [RFC] Add exitcode support
2016-01-22 19:06 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2016-01-23 8:26 ` Andrei Borzenkov
2016-01-25 19:15 ` Vladimir 'phcoder' Serbinenko
0 siblings, 1 reply; 8+ messages in thread
From: Andrei Borzenkov @ 2016-01-23 8:26 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1: Type: text/plain, Size: 1951 bytes --]
22.01.2016 22:06, Vladimir 'φ-coder/phcoder' Serbinenko пишет:
> On 18.08.2015 21:17, Ben Hildred wrote:
>> Let's assume for a minute that I have compiled grub as a multiboot image
>> and have called it from another bootloader, say iPXE.Now iPXE assumes
>> that any false return is an error. What happens when grub returns with
>> exit next, does iPXE get a true or false? What about exit fred where
>> fred is not defined by any platform? What if I do an exit config which
>> is only defined for coreboot?
> Neither multiboot nor coreboot have any return semantics. The situation
> with current platforms is as follows:
> No return/exit semantics at all or machine shutdown:
> i386_coreboot, i386_qemu, i386_multiboot, mips_qemu_mips, mips_loongson
> no-args exit:
> *-ieee1275, i386-pc, mips-arc
> Xen semantics (crash vs poweroff):
> *-xen
> EFI semantics:
> *-efi
> Unix-like semantics:
> arm-uboot, emu
>
> Emu is of no real interest and I have no idea what Uboot does with
> return code but I suppose nothing.
>
> This leaves us only with xen and EFI semantics. Xen is enough of outlier
> to handle it separately. So only EFI is remaining.
>
> On i386-pc the default behaviour of exit is to try next boot entry. EFI
> should probably do the same. What is the current behaviour of grub_exit
> and what is the value in returning EFI_SUCCESS ?
Currently it returns EFI_SUCCESS that tells firmware boot manager to
return to menu and stop attempting to load further boot options. I.e.
this is directly opposite to what i386-pc.
OTOH I still find it useful to be able to explicitly return to boot menu
from GRUB (or EFI Shell for that matter).
> Can we have returncode-aware command to be EFI-specific?
>
I'm fine with changing default EFI exit code to be not EFI_SUCCESS (but
this should be commented in sources) and to have fwmenu (similar to
fwsetup) that explicitly returns EFI_SUCCESS.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] [RFC] Add exitcode support
2016-01-23 8:26 ` Andrei Borzenkov
@ 2016-01-25 19:15 ` Vladimir 'phcoder' Serbinenko
0 siblings, 0 replies; 8+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2016-01-25 19:15 UTC (permalink / raw)
To: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 2270 bytes --]
Agreed
сб, 23 янв. 2016, 9:26, Andrei Borzenkov <arvidjaar@gmail.com>:
> 22.01.2016 22:06, Vladimir 'φ-coder/phcoder' Serbinenko пишет:
> > On 18.08.2015 21:17, Ben Hildred wrote:
> >> Let's assume for a minute that I have compiled grub as a multiboot image
> >> and have called it from another bootloader, say iPXE.Now iPXE assumes
> >> that any false return is an error. What happens when grub returns with
> >> exit next, does iPXE get a true or false? What about exit fred where
> >> fred is not defined by any platform? What if I do an exit config which
> >> is only defined for coreboot?
> > Neither multiboot nor coreboot have any return semantics. The situation
> > with current platforms is as follows:
> > No return/exit semantics at all or machine shutdown:
> > i386_coreboot, i386_qemu, i386_multiboot, mips_qemu_mips, mips_loongson
> > no-args exit:
> > *-ieee1275, i386-pc, mips-arc
> > Xen semantics (crash vs poweroff):
> > *-xen
> > EFI semantics:
> > *-efi
> > Unix-like semantics:
> > arm-uboot, emu
> >
> > Emu is of no real interest and I have no idea what Uboot does with
> > return code but I suppose nothing.
> >
> > This leaves us only with xen and EFI semantics. Xen is enough of outlier
> > to handle it separately. So only EFI is remaining.
> >
> > On i386-pc the default behaviour of exit is to try next boot entry. EFI
> > should probably do the same. What is the current behaviour of grub_exit
> > and what is the value in returning EFI_SUCCESS ?
>
> Currently it returns EFI_SUCCESS that tells firmware boot manager to
> return to menu and stop attempting to load further boot options. I.e.
> this is directly opposite to what i386-pc.
>
> OTOH I still find it useful to be able to explicitly return to boot menu
> from GRUB (or EFI Shell for that matter).
>
> > Can we have returncode-aware command to be EFI-specific?
> >
>
> I'm fine with changing default EFI exit code to be not EFI_SUCCESS (but
> this should be commented in sources) and to have fwmenu (similar to
> fwsetup) that explicitly returns EFI_SUCCESS.
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
[-- Attachment #2: Type: text/html, Size: 2886 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-01-25 19:15 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-18 17:30 [PATCH] [RFC] Add exitcode support dann frazier
2015-08-18 18:03 ` Ben Hildred
2015-08-18 18:56 ` Dann Frazier
2015-08-18 19:17 ` Ben Hildred
2015-08-18 22:00 ` Dann Frazier
2016-01-22 19:06 ` Vladimir 'φ-coder/phcoder' Serbinenko
2016-01-23 8:26 ` Andrei Borzenkov
2016-01-25 19:15 ` Vladimir 'phcoder' Serbinenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.