Re: [Xen-devel] [PATCH RFC] hvm: Allow triple fault to imply crash rather than reboot

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.02.13 at 15:25, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> While the triple fault action on native hardware will result in a system
> reset, any modern operating system can and will make use of less violent
> reboot methods.  As a result, the most likely cause of a triple fault is a
> fatal software bug.
> 
> This patch allows the toolstack to indicate that a triple fault should mean 
> a
> crash rather than a reboot.  The default of reboot still remains the same.

Makes sense to me; minor nits below (no need to resend just
because of that, but would be nice to be addressed if you had
to rev the patch anyway).

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1233,9 +1233,14 @@ void hvm_hlt(unsigned long rflags)
>  void hvm_triple_fault(void)
>  {
>      struct vcpu *v = current;
> +    struct domain * d = v->domain;

Stray blank.

> +    u8 reason = d->arch.hvm_domain.params[HVM_PARAM_TRIPLE_FAULT_CRASH]
> +        ? SHUTDOWN_crash : SHUTDOWN_reboot;
> +
>      gdprintk(XENLOG_INFO, "Triple fault on VCPU%d - "
> -             "invoking HVM system reset.\n", v->vcpu_id);
> -    domain_shutdown(v->domain, SHUTDOWN_reboot);
> +             "invoking HVM system %s.\n", v->vcpu_id,
> +             reason == SHUTDOWN_crash ? "crash" : "reboot");
> +    domain_shutdown(v->domain, reason);

So you have d cached in a local variable now, yet you still use
v->domain here?

Also, I'd prefer for the message to continue to say "reset".

Jan


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