| Message ID | 4f5de6852f65c4b7c305b05975a8c4ed05b36387.1466640165.git.crobinso@redhat.com |
|---|---|
| State | New |
| Headers | show |
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(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 75ad03f..9e5af3f 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -23881,11 +23881,8 @@ virDomainDeleteConfig(const char *configDir, >> unlink(autostartLink); >> dom->autostart = 0; >> >> - 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... Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 06/24/2016 09:57 AM, Ján Tomko wrote: > 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. virFileUnlinkIfExists is better than mine anyways, and at least it will be consistent, so I'll go with that Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 75ad03f..9e5af3f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23881,11 +23881,8 @@ virDomainDeleteConfig(const char *configDir, unlink(autostartLink); dom->autostart = 0; - 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; } diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 02b8cd7..4732766 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3319,10 +3319,8 @@ int virNetworkDeleteConfig(const char *configDir, unlink(autostartLink); net->autostart = 0; - if (unlink(configFile) < 0) { - virReportSystemError(errno, - _("cannot remove config file '%s'"), - configFile); + if (unlink(configFile) < 0 && errno != ENOENT) { + virReportSystemError(errno, _("cannot remove config %s"), configFile); goto error; } diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 3f90f65..64dbe8b 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -3283,10 +3283,8 @@ virNWFilterObjDeleteDef(const char *configDir, goto error; } - if (unlink(configFile) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot remove config for %s"), - nwfilter->def->name); + if (unlink(configFile) < 0 && errno != ENOENT) { + virReportSystemError(errno, _("cannot remove config %s"), configFile); goto error; } diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 6932195..e45e197 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2129,10 +2129,9 @@ virStoragePoolObjDeleteDef(virStoragePoolObjPtr pool) return -1; } - if (unlink(pool->configFile) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot remove config for %s"), - pool->def->name); + if (unlink(pool->configFile) < 0 && errno != ENOENT) { + virReportSystemError(errno, _("cannot remove config %s"), + pool->configFile); return -1; } diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index c46d22c..2a5cd31 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -663,7 +663,7 @@ virSecretObjDeleteConfig(virSecretObjPtr secret) { if (!secret->def->isephemeral && unlink(secret->configFile) < 0 && errno != ENOENT) { - virReportSystemError(errno, _("cannot unlink '%s'"), + virReportSystemError(errno, _("cannot remove config %s"), secret->configFile); return -1; }