Re: [Qemu-devel] [RESEND][PATCH 2/2 v3] deal with guest panicked event

[Anthony Liguori <anthony@codemonkey.ws>]

On 03/09/2012 04:22 PM, Marcelo Tosatti wrote:
> On Thu, Mar 08, 2012 at 11:56:56AM +0000, Daniel P. Berrange wrote:
>> On Thu, Mar 08, 2012 at 01:52:45PM +0200, Avi Kivity wrote:
>>> On 03/08/2012 01:36 PM, Daniel P. Berrange wrote:
>>>> On Thu, Mar 08, 2012 at 01:28:56PM +0200, Avi Kivity wrote:
>>>>> On 03/08/2012 12:15 PM, Wen Congyang wrote:
>>>>>> When the host knows the guest is panicked, it will set
>>>>>> exit_reason to KVM_EXIT_GUEST_PANICKED. So if qemu receive
>>>>>> this exit_reason, we can send a event to tell management
>>>>>> application that the guest is panicked and set the guest
>>>>>> status to RUN_STATE_PANICKED.
>>>>>>
>>>>>> Signed-off-by: Wen Congyang<wency@cn.fujitsu.com>
>>>>>> ---
>>>>>>   kvm-all.c        |    5 +++++
>>>>>>   monitor.c        |    3 +++
>>>>>>   monitor.h        |    1 +
>>>>>>   qapi-schema.json |    2 +-
>>>>>>   qmp.c            |    3 ++-
>>>>>>   vl.c             |    1 +
>>>>>>   6 files changed, 13 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>>>> index 77eadf6..b3c9a83 100644
>>>>>> --- a/kvm-all.c
>>>>>> +++ b/kvm-all.c
>>>>>> @@ -1290,6 +1290,11 @@ int kvm_cpu_exec(CPUState *env)
>>>>>>                       (uint64_t)run->hw.hardware_exit_reason);
>>>>>>               ret = -1;
>>>>>>               break;
>>>>>> +        case KVM_EXIT_GUEST_PANICKED:
>>>>>> +            monitor_protocol_event(QEVENT_GUEST_PANICKED, NULL);
>>>>>> +            vm_stop(RUN_STATE_PANICKED);
>>>>>> +            ret = -1;
>>>>>> +            break;
>>>>>>
>>>>>
>>>>> If the management application is not aware of this event, then it will
>>>>> never resume the guest, so it will appear hung.
>>>>
>>>> Even if the mgmt app doesn't know about the QEVENT_GUEST_PANICKED, it should
>>>> still see a QEVENT_STOP event emitted by vm_stop() surely ? So it will
>>>> know the guest CPUs have been stopped, even if it isn't aware of the
>>>> reason why, which seems fine to me.
>>>
>>> No.  The guest is stopped, and there's no reason to suppose that the
>>> management app will restart it.  Behaviour has changed.
>>>
>>> Suppose the guest has reboot_on_panic set; now the behaviour change is
>>> even more visible - service will stop completely instead of being
>>> interrupted for a bit while the guest reboots.
>>
>> Hmm, so this calls for a new command line argument to control behaviour,
>> similar to what we do for disk werror, eg something like
>>
>>    --onpanic "report|pause|stop|..."
>>
>> where
>>
>>   report - emit QEVENT_GUEST_PANICKED only
>
> Should be the default.

Should we just have a mechanism to stop the guest on certain types of QMP 
events?  For instance:

-stop-on guest-panicked,block-ioerror

Likewise, we could have a:

-quit-on guest-panicked.

In the very least, we should make what we use for rerror,werror an enumeration 
that's shared here.

Regards,

Anthony Liguori

>
>>   pause  - emit QEVENT_GUEST_PANICKED and pause VM
>>   stop   - emit QEVENT_GUEST_PANICKED and quit VM
>
> "quit" is a better name than "stop".
>
>> This would map fairly well into libvirt, where we already have config
>> parameters for controlling what todo with a guest when it panics.
>>
>> Regards,
>> Daniel
>
>