Re: [libvirt] [PATCH 1/3] Add missing error reporting

On 04/18/2013 04:30 AM, Martin Kletzander wrote: Needs work. Right now, this function intentionally does not report errors; all direct callers (src/util/virlockspace.c and tools/virsh.c) are aware of this convention. On the other hand, I looked through indirect callers of virFileMakePath(), and found at least the following problems: daemon/libvirtd.c: daemonPidFilePath() doesn't report an error src/conf/*_conf.c: 5 callers report error libxl_driver.c: reports errors with VIR_ERROR instead of virReportSystemError, so it gets logged but not reported to caller src/locking/lock_daemon.c: same bug as daemon/libvirt.c I stopped looking here - lots of other callers, and probably a similar mix of functions that report the error themselves, vs. functions that fail to do error reporting. We need to decide which way to go, then audit ALL users to obey that convention. Note that there are cases (such as tools/virsh.c) that intentionally want to suppress errors of EEXIST, except that virFileMakePathHelper already guarantees that EEXIST won't be the cause of failure, so maybe cleaning up ALL callers to expect virutil.c to do error reporting is the way to go. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org