[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] storage: silently ignore missing files on pool refresh



On 07/10/2013 06:07 PM, Daniel P. Berrange wrote:
> On Wed, Jul 10, 2013 at 05:00:59PM +0200, Michal Privoznik wrote:
>> On 10.07.2013 16:57, Ján Tomko wrote:
>>> From: Wei Zhou <w zhou leaseweb com>
>>>
>>> Make virStorageBackendVolOpenCheckMode return -2 instead of
>>> -1 if volume file is missing.
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=977706
>>> ---
>>>  src/storage/storage_backend.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
>>> index e2527c9..f063601 100644
>>> --- a/src/storage/storage_backend.c
>>> +++ b/src/storage/storage_backend.c
>>> @@ -1084,7 +1084,7 @@ virStorageBackendForType(int type)
>>>   * Allows caller to silently ignore files with improper mode
>>>   *
>>>   * Returns -1 on error, -2 if file mode is unexpected or the
>>> - * volume is a dangling symbolic link.
>>> + * volume is a dangling symbolic link or file is missing.
>>>   */
>>>  int
>>>  virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags)
>>> @@ -1097,7 +1097,7 @@ virStorageBackendVolOpenCheckMode(const char *path, unsigned int flags)
>>>          virReportSystemError(errno,
>>>                               _("cannot stat file '%s'"),
>>>                               path);
>>
>> If we want to *silently* ignore missing file, why do we
>> virReportSystemError() here?
>>
>>> -        return -1;
>>> +        return -2;
>>>      }
> 
> Well returning -1 vs -2 from this function isn't ignoring the
> error. It is just providing the caller a way to detect a specific
> error scenario. Thus if the caller decides to ignore the error
> when ret == -2, then the caller should call virResetLastError()
> to clear it.

Does this imply a NACK to v2 [1], where I changed it to VIR_WARN to match the
other cases of returning -2?

Resetting the error won't unlog it. On the other hand, there are paths (like
in virStorageBackendFileSystemVolRefresh) where the return value of '-2' would
get propagated without setting an error (which is pre-existing for the other
cases, but could be a problem in this case).

Jan

[1] https://www.redhat.com/archives/libvir-list/2013-July/msg00639.html


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]