[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v5 1/1] s390x/cpu: expose the guest crash inform

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v5 1/1] s390x/cpu: expose the guest crash information
Date: Tue, 6 Feb 2018 12:55:00 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2

On 02/06/2018 12:21 PM, Christian Borntraeger wrote:

+    CRASH_REASON_UNKNOWN,  /* default value of 0 on reset */

...you have an internal enum for decoding some of those integer values into 
specific human readable strings, but don't expose the enum over QAPI.  Are we 
sure that's the interface we want to go with?  As long as you are okay with 
that, then I can live with the interface change; I just want to make sure that 
you are certain that the machine-based consumer of the QMP command does not 
need to decode crash_reasons because you left it as an internal enum.

We might want to have temporary or intermediate crash reasons (e.g. emulation 
failure or whatever),
so not requiring changes in both components might be less error prone. (the way 
it is today)
But if there is a strong wish for an on-the-wire enum, we could do that as well.

I don't have a strong wish for an on-the-wire enum, so much as a request to at least document in the commit message why you decided one was not needed at this time. And in all reality, would you really have to keep two different enums in sync, or if you expose something over the wire, can't that just be your only enum type? If a temporary or intermediate crash reason is useful enough to give a different string to the human reader, why would it not be useful as also exposing as a different enum?

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

reply via email to

[Prev in Thread] Current Thread [Next in Thread]