intended behaviour of noreboot

intended behaviour of noreboot

[Muli Ben-Yehuda]

Hi,

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.

Cheers,
Muli

Re: intended behaviour of noreboot

[Keir Fraser]

On 6/8/06 7:50 am, "Muli Ben-Yehuda" <muli@il.ibm.com> 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.

 -- Keir

Re: intended behaviour of noreboot

[Muli Ben-Yehuda]

On Mon, Aug 07, 2006 at 08:17:44AM +0100, Keir Fraser wrote:
> On 6/8/06 7:50 am, "Muli Ben-Yehuda" <muli@il.ibm.com> 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.

Ok, good. I'll whip up a patch later today.

Cheers,
Muli

[PATCH RFC] fix noreboot behaviour on clean reboot (was: intended behaviour of noreboot)

[Muli Ben-Yehuda]

On Mon, Aug 07, 2006 at 08:17:44AM +0100, Keir Fraser wrote:
> On 6/8/06 7:50 am, "Muli Ben-Yehuda" <muli@il.ibm.com> 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 <muli@il.ibm.com>

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 <xen/sched-if.h>
 
 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 <asm/current.h>
 #include <asm/hcalls.h>
 
+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 <asm/msr.h>
 
 /* 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 <xen/guest_access.h>
 #include <xen/hypercall.h>
 #include <xen/delay.h>
+#include <xen/reboot.h>
 #include <asm/debugger.h>
 #include <public/dom0_ops.h>
 #include <public/sched.h>
@@ -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 <xen/mm.h>
 #include <xen/delay.h>
 #include <xen/guest_access.h>
+#include <xen/reboot.h>
 #include <asm/current.h>
 #include <asm/debugger.h>
 #include <asm/io.h>
@@ -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);

Re: [PATCH RFC] fix noreboot behaviour on clean reboot (was: intended behaviour of noreb

[Jan Beulich]

>>> Muli Ben-Yehuda <muli@il.ibm.com> 07.08.06 14:13 >>>
>On Mon, Aug 07, 2006 at 08:17:44AM +0100, Keir Fraser wrote:
>> On 6/8/06 7:50 am, "Muli Ben-Yehuda" <muli@il.ibm.com> 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!

Yes, please!

Jan

Re: [Xen-devel] [PATCH RFC] fix noreboot behaviour on clean reboot(was: intended behaviour of noreboot)

[Akio Takebe]

Hi,

I tested Muil's patch on ia64.

The below is test results.
- Before patch
    reboot  ----> reboot
    halt    ----> reboot
    panic   ----> reboot

- After patch
    reboot  ----> reboot
    halt    ----> reboot
    panic   ----> reboot
    
This results is caused by calling EFI_RESET_SHUTDOWN in ia64's 
machine_halt(). Ia64's machine_halt require system reboot because
EFI_RESET_SHUTDOWN (used by ia64's machine_halt()) enter ACPI G2/S5
 or G3 state.

This is ia64 issue, I think Muli's patch is good. :-)

Best Regards,

Akio Takebe

>On Mon, Aug 07, 2006 at 08:17:44AM +0100, Keir Fraser wrote:
>> On 6/8/06 7:50 am, "Muli Ben-Yehuda" <muli@il.ibm.com> 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
>