Re: [qemu-s390x] [PATCH v3 1/1] s390x/cpu: expose the guest crash information

[Christian Borntraeger]

On 02/05/2018 01:04 PM, Cornelia Huck wrote:

> You're doing the crash_reason -> reason mapping here and also below.
> Maybe introduce a helper for it?
> 
[....]
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index 8736001156..c6a23262a8 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -1568,15 +1568,32 @@ static int handle_instruction(S390CPU *cpu, struct 
>> kvm_run *run)
>>      return r;
>>  }
>>  
>> -static void unmanageable_intercept(S390CPU *cpu, const char *str, int 
>> pswoffset)
>> +static void unmanageable_intercept(S390CPU *cpu, enum crash_reasons reason,
>> +                                   int pswoffset)
>>  {
>>      CPUState *cs = CPU(cpu);
>> +    const char *str;
>>  
>> +    switch (reason) {
>> +    case CRASH_REASON_PGM:
>> +        str = "program interrupt loop";
>> +        break;
>> +    case CRASH_REASON_EXT:
>> +        str = "external interrupt loop";
>> +        break;
>> +    case CRASH_REASON_OPEREXC:
>> +        str = "operation exception loop";
>> +        break;
>> +    default:
>> +        str = "unknown crash reason";
>> +        break;
>> +    }
>>      error_report("Unmanageable %s! CPU%i new PSW: 0x%016lx:%016lx",
> 
> "Unmanageable unknown crash reason!" looks a bit odd. In this case,
> "Unmanageable intercept!" would actually look a bit saner (but you
> would not be able to use a common converter in that case). We can also
> just simply keep it :)

We could maybe just drop this print in kvm.c. qemu_system_guest_panicked below 
will
trigger some logging as well (if enabled) and it will also notify libvirt about
that. a future libvirt code will print something like
panic s390: psw-mask='0x0000000000000000', psw-addr='0x0000000000000002', crash 
reason: operation exception loop
anyway in the log file.

That would also address your concern from above.