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

[Cole Robinson]

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