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

Re: [libvirt] [PATCH 1/4] qemu: make qemuDomainHelperGetVcpus silent



On 09/19/2014 04:57 AM, Peter Krempa wrote:
> On 09/19/14 12:29, Daniel P. Berrange wrote:
>> On Fri, Sep 19, 2014 at 11:44:20AM +0200, Francesco Romani wrote:
>>> The commit 74c066df4d8 introduced an helper to factor a code path
>>> which is shared between the existing API and the new bulk stats API.
>>> In the bulk stats path errors must be silenced unless critical
>>> (e.g. memory allocation failure).
>>>
>>> To address this need, this patch adds an argument to disable error reporting.
>>
>> I'm really don't like the approach of adding 'bool reportError' flags to
>> internal functions as it doesn't scale. These flags end up creeping out
>> across more & more of the code base and different callers are going to
>> disagree about which errors should be ignored and which not. Having the
>> callers use virResetError is preferrable IMHO as it avoids polluting
> 
> In Eric's last review he advised to change from virResetError to the
> boolean flag so that logs are not polluted. Without this patch, the code
> is in the state as you suggest, but it contradicts Eric's stance.
> 
>> countless internal functions with ill defined args.

I think it all boils down to a question of how frequently will these
functions be called, and how frequently will the logs be populated with
messages about an ignored failure.  Is this a case where we are better
off waiting until the complaints happen?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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