[PATCH] kexec/noreboot: Don't kexec_crash() if noreboot has been requested.

[PATCH] kexec/noreboot: Don't kexec_crash() if noreboot has been requested.

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 205 bytes --]

This issue came up when debugging pcpu linked list corruption (patches
for that issue to follow).

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com


[-- Attachment #2: noreboot.patch --]
[-- Type: text/x-patch, Size: 840 bytes --]

# HG changeset patch
# Parent 1126b3079bef37e1bb5a97b90c14a51d4e1c91c3
kexec/noreboot: Don't kexec_crash() if noreboot has been requested.

The noreboot option is for debugging, to be able to see the dying output.  To
that end, continuing and possibly booting into the crash kernel defeats one of
the purposes of noreboot.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r 1126b3079bef xen/drivers/char/console.c
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -963,16 +963,15 @@ void panic(const char *fmt, ...)
 
     debugger_trap_immediate();
 
-#ifdef CONFIG_KEXEC
-    kexec_crash();
-#endif
-
     if ( opt_noreboot )
     {
         machine_halt();
     }
     else
     {
+#ifdef CONFIG_KEXEC
+        kexec_crash();
+#endif
         watchdog_disable();
         machine_restart(5000);
     }

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] kexec/noreboot: Don't kexec_crash() if noreboot has been requested.

[Jan Beulich]

>>> On 07.09.12 at 15:05, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> This issue came up when debugging pcpu linked list corruption (patches
> for that issue to follow).

Hmm, that's a matter of taste of course. Nor is running kdump
really a (direct) reboot action. Both ways have their reasoning
imo, so I'm not sure whether keeping things as is or applying the
patch is the better thing.

Jan

Re: [PATCH] kexec/noreboot: Don't kexec_crash() if noreboot has been requested.

[Andrew Cooper]

On 07/09/12 14:16, Jan Beulich wrote:
>>>> On 07.09.12 at 15:05, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> This issue came up when debugging pcpu linked list corruption (patches
>> for that issue to follow).
> Hmm, that's a matter of taste of course. Nor is running kdump
> really a (direct) reboot action. Both ways have their reasoning
> imo, so I'm not sure whether keeping things as is or applying the
> patch is the better thing.
>
> Jan

Hmm yes - I guess its not as clean cut as I thought, but I would still
argue that this patch is the lest surprising alternative.

One tweak I am thinking of working on soon is a "pause-on-reboot"
feature which is a bit like noreboot, but will accept a passphrase and
continue with the reboot, so you can 'pause', see the dying panic then
reboot without needing to invoke the server power management.

(This may have been as a result of being given details for the wrong
server power management interface IP address when debugging a server in
a data center 7 time zones away, and getting rather confused when it
didn't make any attempt to reboot.  I imagine someone else might get
equally irate if their server was rebooting without warning.)

~Andrew

>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com

Re: [PATCH] kexec/noreboot: Don't kexec_crash() if noreboot has been requested.

[Keir Fraser]

On 07/09/2012 14:16, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 07.09.12 at 15:05, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> This issue came up when debugging pcpu linked list corruption (patches
>> for that issue to follow).
> 
> Hmm, that's a matter of taste of course. Nor is running kdump
> really a (direct) reboot action. Both ways have their reasoning
> imo, so I'm not sure whether keeping things as is or applying the
> patch is the better thing.

I note though that kexec_crash() should be stubbed out in the header file
when !CONFIG_KEXEC, rather than ifdef at the caller. By the by.

 -- Keir

> Jan
> 

Re: [PATCH] kexec/noreboot: Don't kexec_crash() if noreboot has been requested.

[Andrew Cooper]

On 07/09/12 16:03, Keir Fraser wrote:
> On 07/09/2012 14:16, "Jan Beulich" <JBeulich@suse.com> wrote:
>
>>>>> On 07.09.12 at 15:05, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>> This issue came up when debugging pcpu linked list corruption (patches
>>> for that issue to follow).
>> Hmm, that's a matter of taste of course. Nor is running kdump
>> really a (direct) reboot action. Both ways have their reasoning
>> imo, so I'm not sure whether keeping things as is or applying the
>> patch is the better thing.
> I note though that kexec_crash() should be stubbed out in the header file
> when !CONFIG_KEXEC, rather than ifdef at the caller. By the by.
>
>  -- Keir

Agreed - I will put it on my todo list to fix.  (I think its elsewhere
in the codebase as well)

~Andrew

>
>> Jan
>>
>

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com