Re: [libvirt] [PATCH 6/6] conf: sync error reporting for object configFile unlinking

On Fri, Jun 24, 2016 at 09:48:12AM -0400, Cole Robinson wrote: > On 06/24/2016 09:38 AM, Ján Tomko wrote: >> On Wed, Jun 22, 2016 at 08:12:16PM -0400, Cole Robinson wrote: >>> The various object implementations for configFile unlinking >>> have subtly different error handling behavior. Sync the impls >>> to use a single error string, and consistently ignore ENOENT, >>> to allow undefining an object whose configFile was deleted >>> behind libvirt's back >>> --- >>> src/conf/domain_conf.c | 7 ++----- >>> src/conf/network_conf.c | 6 ++---- >>> src/conf/nwfilter_conf.c | 6 ++---- >>> src/conf/storage_conf.c | 7 +++---- >>> src/conf/virsecretobj.c | 2 +- >>> 5 files changed, 10 insertions(+), 18 deletions(-) >>> >>> - if (unlink(configFile) < 0 && >>> - errno != ENOENT) { >>> - virReportSystemError(errno, >>> - _("cannot remove config %s"), >>> - configFile); >>> + if (unlink(configFile) < 0 && errno != ENOENT) { >>> + virReportSystemError(errno, _("cannot remove config %s"), >>> configFile); >>> goto cleanup; >> >> Using a helper function that ignores errno and only calls >> virReportSystemError from one place would be even more consistent. >> > > Okay, something like virFileUnlinkSkipMissing in virfile.c ? (I suck at > function names) Or something specific to this driver state handling? if the > latter, I don't know where it should live... > I created virDirOpenIfExists with those semantics recently, but I'm not really proud of the name. virfile.c should be the right place.