On 01/29/2010 11:03 AM, Ales Kozumplik wrote:
> Stop the kernel correctly on HALT (so that we dont see 'kernel panic, they
> killed init' on i386 and on s390 one gets easy access to manual IPL).
>
> Introduce a new reboot method that does allow us to see the backtrace and
> doesn't scroll the screen up with useless unmount info.
> ---
> loader/init.c | 7 +++--
> loader/init.h | 5 +++-
> loader/shutdown.c | 57 +++++++++++++++++++++++++++++++++-------------------
> 4 files changed, 47 insertions(+), 25 deletions(-)
> diff --git a/loader/init.c b/loader/init.c
> index 68a9ea9..9f41850 100644
> --- a/loader/init.c
> +++ b/loader/init.c
> @@ -438,8 +438,8 @@ int main(int argc, char **argv) {
> pid_t installpid, childpid;
> int waitStatus;
> int fd = -1;
> - int doReboot = 0;
> int doShutdown =0;
> + int shutdown_method = HALT;
reboot_action shutdown_method = HALT;
To make it virtually more type safe and more readable?
> int isSerial = 0;
> char * console = NULL;
> int doKill = 1;
> @@ -776,6 +776,7 @@ int main(int argc, char **argv) {
>
> if (!WIFEXITED(waitStatus) ||
> (WIFEXITED(waitStatus) && WEXITSTATUS(waitStatus))) {
> + shutdown_method = VERBOSE_REBOOT;
Introducing this new case at this point seems perfect to me.
So if anaconda or loader crash, also s390 will run into the SIGINT
thingy again? But I guess that's OK, since this case should not happen
unless something is really wrong.
> printf("install exited abnormally [%d/%d] ", WIFEXITED(waitStatus),
> WEXITSTATUS(waitStatus));
> if (WIFSIGNALED(waitStatus)) {
> @@ -783,11 +784,11 @@ int main(int argc, char **argv) {
> }
> printf("\n");
> } else {
> - doReboot = 1;
> + shutdown_method = REBOOT;
> }
>
> expected_exit = 1;
Do you still need this with the fixes in here?
> - shutDown(doKill, doReboot?REBOOT:HALT);
> + shutDown(doKill, shutdown_method);
I suppose there is at least one more call of shutDown that should use
shutdown_method instead of a hardcoded reboot_action:
/* reboot handler */
static void sigintHandler(int signum) {
termReset();
shutDown(getKillPolicy(), REBOOT);
}
REBOOT => shutdown_method
Which requires shutdown_method to be static and global to the compile
unit init.c. Otherwise, I fear, pressing CAD will unconditionally REBOOT
even if shutdown_method was set to VERBOSE_REBOOT above.
>
> return 0;
> }
> diff --git a/loader/init.h b/loader/init.h
> index 733bc8e..c19d188 100644
> --- a/loader/init.h
> +++ b/loader/init.h
> @@ -22,7 +22,10 @@
> typedef enum {
> REBOOT,
> POWEROFF,
> - HALT
> + HALT,
> + /* gives user a chance to read the trace before scrolling the text out
> + with disk unmounting and termination info */
> + VERBOSE_REBOOT
Maybe renaming to DELAYED_REBOOT would be more accurate, since it does
not cause shutdown to output more messages than with a REBOOT, it just
delays the actual reboot until sigint or ctrl-alt-del.
> } reboot_action;
>
> #endif /* INIT_H */
> diff --git a/loader/shutdown.c b/loader/shutdown.c
> index 774e11a..40651f9 100644
> --- a/loader/shutdown.c
> +++ b/loader/shutdown.c
> @@ -62,35 +62,50 @@ static void performUnmounts(void) {
> }
>
> static void performReboot(reboot_action rebootAction) {
> - if (rebootAction == POWEROFF) {
> + switch (rebootAction) {
> + case POWEROFF:
> printf("powering off system\n");
> - sleep(2);
> + sleep(2);
> reboot(RB_POWER_OFF);
> - } else if (rebootAction == REBOOT) {
> - printf("rebooting system\n");
> - sleep(2);
> -
> + break;
> + case REBOOT:
> + printf("rebooting system\n");
> + sleep(2);
> #if USE_MINILIBC
> - reboot(0xfee1dead, 672274793, 0x1234567);
> + reboot(0xfee1dead, 672274793, 0x1234567);
> #else
> - reboot(RB_AUTOBOOT);
> + reboot(RB_AUTOBOOT);
> #endif
> - }
> + break;
> + case HALT:
> + printf("halting system\n");
I'm undecided, if a sleep(2) for consistency with the other cases is a
good or bad idea.
> + reboot(RB_HALT_SYSTEM);
> + break;
> + default:
> + break;
> + }
> }
>
> -void shutDown(int doKill, reboot_action rebootAction) {
> - if (doKill) {
> - performUnmounts();
> - performTerminations();
> - }
> -
> - if ((rebootAction == POWEROFF || rebootAction == REBOOT) && doKill) {
> - performReboot(rebootAction);
> - }
> +static void performVerboseReboot()
> +{
> + printf("the system will be rebooted when you press Ctrl-Alt-Delete.\n");
It should also reboot, when the user presses CTRL+C or sends a SIGINT
(which ctrl-alt-del actually triggers, if I understood the code
correctly). Maybe we should also mention the CTRL+C method, which is
theoretically also possible on s390 as opposed to the CAD key combo.
> + while (1) {
> + sleep(1);
> + }
> +}
>
> - printf("you may safely reboot your system\n");
> - exit(0);
> - return;
> +void shutDown(int doKill, reboot_action rebootAction)
> +{
> + if (rebootAction == VERBOSE_REBOOT) {
> + performVerboseReboot();
Does the SIGINT/CAD to reboot still work for non-s390 now that the
rebootHandler is gone from shutdown.c?
The other sigint handler in init.c only triggers this code path in
shutdown, but then shutdown seems to be trapped in
performVerboseReboot's endless loop.
> + } else if (doKill) {
> + performUnmounts();
> + performTerminations();
> + performReboot(rebootAction);
> + }
> + printf("you may safely reboot your system\n");
> + exit(0);
> + return;
> }
>
> #ifdef AS_SHUTDOWN
All of this should also go into rhel6-branch because of bug 533198.
Master and rhel6-branch would additionally need the following:
+ a new option -H for the main() of AS_SHUTDOWN to HALT
+ changing linuxrc.s390 to exec "/sbin/shutdown -H" in doshutdown()
That's all I can think of now.
Thanks a lot for fixing this!
Steffen
Linux on System z Development
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
