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

Re: [libvirt] [PATCH 1/3] qemuDomainBuildNamespace: Clean up temp files



On 06/15/2017 02:03 PM, John Ferlan wrote:
> 
> 
> On 06/15/2017 04:53 AM, Michal Privoznik wrote:
>> On 06/14/2017 09:50 PM, John Ferlan wrote:
>>>
>>>
>>> On 06/12/2017 11:57 AM, Michal Privoznik wrote:
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1431112
>>>>
>>>> After 290a00e41d we know how to deal with file mount points.
>>>> However, when cleaning up the temporary location for preserved
>>>> mount points we are still calling rmdir(). This won't fly for
>>>> files. We need to call unlink(). Now, since we don't really care
>>>> if the cleanup succeeded or not (it's the best effort anyway), we
>>>> can call both rmdir() and unlink() without need for
>>>> differentiation between files and directories.
>>>>
>>>> Signed-off-by: Michal Privoznik <mprivozn redhat com>
>>>> ---
>>>>  src/qemu/qemu_domain.c | 5 ++++-
>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>
>>> But why call both?
>>
>> I don't think you can call unlink() over a directory, can you? And sure,
>> I could call stat() just to find out if it's a dir or a file and call
>> just one of the pair. Or I can call both and ignore any errors. The
>> result is the same, isn't it?
>>
>> Michal
>>
> 
> From the unlink(3p) man page:
> 
> "The path argument shall not name a directory unless the process has
> appropriate privileges and the implementation supports using unlink() on
> directories."

That's weird. I don't see that in my man page. What I see though is the
following errno code:

 EISDIR pathname refers to a directory.  (This is the non-POSIX value
returned by Linux since 2.1.132.)

The "non-POSIX" bothers me there. But I agree that whole namespaces are
Linux specific, so it doesn't bother me there that much.

> 
> Then a google search on using unlink vs. rmdir uncovers more refs. I
> suppose one could also do the "if file, then unlink else rmdir.

Well, to determine if a path is a file or a dir I'd need to call stat()
which can fail. I'm not a big fan of overcomplicating simple code.

> 
> Just seems "odd" to see both and leaves one wondering why.

Well, there's a comment that says why.

Michal


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