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
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list