From: Muli Ben-Yehuda Subject: [PATCH RFC] fix noreboot behaviour on clean reboot (was: intended behaviour of noreboot) Date: Mon, 7 Aug 2006 15:13:28 +0300 Message-ID: <20060807121328.GD3802@rhun.haifa.ibm.com> References: <20060806065053.GB3447@rhun.haifa.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: xen-devel List-Id: xen-devel@lists.xenproject.org On Mon, Aug 07, 2006 at 08:17:44AM +0100, Keir Fraser wrote: > On 6/8/06 7:50 am, "Muli Ben-Yehuda" wrote: > > > Setting 'noreboot' on the Xen command line stops reboot(1) from > > working in dom0. Is this the intended behaviour? I would expect > > noreboot to only apply to reboots that are trigerred as a result of > > kernel panis or oopses. > > Clean shutdown of domain0 could clear the noreboot flag in Xen. Itt's a bit too hacky that machine_restart() either restarts or halts, depends on opt_noreboot. How about something like this: - move code specific to dom0 out of domain_shutdown() into dom0_shutdown() - dom0_shutdown() has the following semantics: if reason is SHUTDOWN_poweroff: halt if reason is SHUTDOWN_crash: halt if opt_noreboot is set, reboot otherwise. if reason is SHUTDOWN_reboot: reboot (regardless of noreboot) if reason is anything else: halt if opt_noreboot is set, reboot otherwise (preserve backward compatible behaviour). - I checked and PV Linux i386/x86-64 uses SHUTDOWN_crash/SHUTDOWN_reboot appropriately to give the above semantics. - since opt_noreboot is now referenced in common code, add definitions for it for ia64 and ppc as well. Hooking it in in arch code is simple - just parse the command line options and set opt_noreboot appropriately. Tested on x86-64, comments appreciated! Cheers, Muli Signed-off-by: Muli Ben-Yehuda diff -r ffa5b2975dff xen/arch/ia64/xen/domain.c --- a/xen/arch/ia64/xen/domain.c Fri Aug 04 20:34:44 2006 +0100 +++ b/xen/arch/ia64/xen/domain.c Mon Aug 07 13:07:05 2006 +0300 @@ -77,6 +77,8 @@ DEFINE_PER_CPU(uint8_t *, current_psr_i_ DEFINE_PER_CPU(uint8_t *, current_psr_i_addr); DEFINE_PER_CPU(int *, current_psr_ic_addr); +int opt_noreboot = 0; + #include static void flush_vtlb_for_context_switch(struct vcpu* vcpu) diff -r ffa5b2975dff xen/arch/powerpc/domain.c --- a/xen/arch/powerpc/domain.c Fri Aug 04 20:34:44 2006 +0100 +++ b/xen/arch/powerpc/domain.c Mon Aug 07 13:07:27 2006 +0300 @@ -31,6 +31,8 @@ #include #include +int opt_noreboot = 0; + extern void idle_loop(void); #define next_arg(fmt, args) ({ \ diff -r ffa5b2975dff xen/arch/x86/shutdown.c --- a/xen/arch/x86/shutdown.c Fri Aug 04 20:34:44 2006 +0100 +++ b/xen/arch/x86/shutdown.c Mon Aug 07 13:09:40 2006 +0300 @@ -22,7 +22,7 @@ #include /* opt_noreboot: If true, machine will need manual reset on error. */ -static int opt_noreboot = 0; +int opt_noreboot = 0; boolean_param("noreboot", opt_noreboot); /* reboot_str: comma-separated list of reboot options. */ @@ -204,12 +204,6 @@ void machine_restart(char * __unused) { int i; - if ( opt_noreboot ) - { - printk("Reboot disabled on cmdline: require manual reset\n"); - machine_halt(); - } - watchdog_disable(); console_start_sync(); diff -r ffa5b2975dff xen/common/domain.c --- a/xen/common/domain.c Fri Aug 04 20:34:44 2006 +0100 +++ b/xen/common/domain.c Mon Aug 07 13:08:47 2006 +0300 @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -282,35 +283,58 @@ static __init int domain_shutdown_finali } __initcall(domain_shutdown_finaliser_init); - -void domain_shutdown(struct domain *d, u8 reason) -{ - struct vcpu *v; - - if ( d->domain_id == 0 ) - { - extern void machine_restart(char *); - extern void machine_halt(void); - - debugger_trap_immediate(); - - if ( reason == SHUTDOWN_poweroff ) +static void dom0_shutdown(u8 reason) +{ + debugger_trap_immediate(); + switch ( reason) { + case ( SHUTDOWN_poweroff ): + { + printk("Domain 0 halted: halting machine.\n"); + machine_halt(); + break; /* not reached */ + } + case ( SHUTDOWN_crash ): + { + if ( opt_noreboot ) { - printk("Domain 0 halted: halting machine.\n"); + printk("Domain 0 crashed: 'noreboot' set - not rebooting.\n"); + watchdog_disable(); machine_halt(); } - else if ( reason == SHUTDOWN_crash ) + else { printk("Domain 0 crashed: rebooting machine in 5 seconds.\n"); watchdog_disable(); mdelay(5000); - machine_restart(0); + machine_restart(NULL); } + break; /* not reached */ + } + case ( SHUTDOWN_reboot ): + { + printk("Domain 0 shutdown: rebooting machine.\n"); + machine_restart(NULL); + break; /* not reached */ + } + default: + { + printk("Domain 0 shutdown (reason %u): rebooting machine.\n", + reason); + if ( opt_noreboot ) + machine_halt(); else - { - printk("Domain 0 shutdown: rebooting machine.\n"); - machine_restart(0); - } + machine_restart(NULL); + } + } +} + +void domain_shutdown(struct domain *d, u8 reason) +{ + struct vcpu *v; + + if ( d->domain_id == 0 ) + { + dom0_shutdown(reason); } /* Mark the domain as shutting down. */ diff -r ffa5b2975dff xen/drivers/char/console.c --- a/xen/drivers/char/console.c Fri Aug 04 20:34:44 2006 +0100 +++ b/xen/drivers/char/console.c Mon Aug 07 13:09:12 2006 +0300 @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -737,7 +738,10 @@ void panic(const char *fmt, ...) watchdog_disable(); mdelay(5000); - machine_restart(0); + if ( opt_noreboot ) + machine_halt(); + else + machine_restart(NULL); } void __bug(char *file, int line) diff -r ffa5b2975dff xen/include/xen/reboot.h --- a/xen/include/xen/reboot.h Fri Aug 04 20:34:44 2006 +0100 +++ b/xen/include/xen/reboot.h Mon Aug 07 13:06:19 2006 +0300 @@ -1,5 +1,9 @@ #ifndef _LINUX_REBOOT_H #define _LINUX_REBOOT_H + +/* opt_noreboot: If true, machine will need manual reset on error. */ +/* only implemented on i386/x86-64 for the moment */ +extern int opt_noreboot; extern void machine_restart(char *cmd); extern void machine_halt(void);