Re: [Qemu-devel] [libvirt] [PATCHv2] Don't log an internal error when the guest hasn't updated balloon stats

[Ján Tomko]

On 05/15/2014 11:19 PM, Eric Blake wrote:
> On 05/15/2014 01:22 AM, Ján Tomko wrote:
>> If virDomainMemoryStats is called too soon after domain startup,
>> QEMU returns:
>> "error":{"class":"GenericError","desc":"guest hasn't updated any stats yet"}
>> when we try to query balloon stats.
>>
>> Check for this reply and log it as OPERATION_INVALID instead of
>> INTERNAL_ERROR. This means the daemon only logs it at the debug level,
>> without polluting system logs.
>>
>> Reported by Laszlo Pal:
>> https://www.redhat.com/archives/libvirt-users/2014-May/msg00023.html
>> ---
>> v1: https://www.redhat.com/archives/libvir-list/2014-May/msg00420.html
>> v2:
>>   return 0 in this case - even though balloon stats are not yet available,
>>     we can still return 'rss' in qemuDomainMemoryStats
>>   jump to cleanup if CheckError returns < 0
>>
>>  src/qemu/qemu_monitor_json.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
>> +    if ((data = virJSONValueObjectGet(reply, "error"))) {
>> +        const char *klass = virJSONValueObjectGetString(data, "class");
>> +        const char *desc = virJSONValueObjectGetString(data, "desc");
>>  
>> -    if (ret < 0)
>> +        if (STREQ_NULLABLE(klass, "GenericError") &&
>> +            STREQ_NULLABLE(desc, "guest hasn't updated any stats yet")) {
> 
> Adding qemu.  Uggh - the qemu documentation of QMP states:
> 
> - The "desc" member is a human-readable error message. Clients should
>   not attempt to parse this message.
> 
> because the contents of that field are NOT guaranteed to be stable.
> We're stuck parsing that field for old versions of qemu, but this is one
> case where upstream qemu (for future versions) should change the "class"
> member of that particular error case to a distinct value other than
> GenericError so that it is trivially obvious when this particular
> condition has occurred, since it is a case where libvirt wants to treat
> it as a non-error.
> 
> reluctant ACK, while hoping that we can do something more reliable in
> the future.
> 

I have pushed the patch now.

The qemu patch reporting empty stats instead of this error should be on its way:
https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04295.html

Jan

signature.asc
Description: OpenPGP digital signature