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

Re: [libvirt] [PATCH 03/18] Introduce a virFileDeleteTree method

On 04/08/2013 08:04 AM, Daniel P. Berrange wrote:
>> We should use gnulib's LGPL unlinkat.  On capable kernels, it avoids
>> O(n^2) behavior that is inherent in computing filenames in a deep
>> hierarchy.  On less capable kernels, it still makes this code simpler to
>> write (no virAsprintf needed here).
> I looked into unlinkat() but that gnulib module is GPL only.

Oh right.  The gnulib wrappers are nice on Linux (even with older
kernels, they fall back to /proc/self/fd usage for avoidance of
quadratic behavior and no thread-safety issues); but on other platforms
(hello mingw and older BSD), it uses chdir() under the hood, which is
not thread-safe since the current working directory is global state.
Also, the gnulib wrappers currently can call exit() on some extreme
corner cases (primarily related to traversing through searchable but
unreadable directories), and library code should not call exit(), so the
modules were left GPL to avoid using them in a library until someone is
motivated enough to clean up the code to be a bit more robust.

> I also looked at fdopendir() but we'd want gnulib for portability
> of that, and again it is GPL only.

I can make the argument for relaxing its license, if we really want it.
 Given that it is an essential function, and provided by libc (where
glibc is LGPLv2+), I have typically had success with requests like that
in the past; but it's not worth fighting the battle unless we know we
want to use it.

>> Very weak ACK, depending on what you answer to my commentary.
> I would like to make some of the enhancements you suggest, but doing
> so would require we implement a bunch of portability code since all
> the things you suggest are Linux specific and the gnulib modules are
> GPL only.  So I think in the immediate term, we'll just have to go
> with that I have here.

Yeah, that's a pretty convincing argument to use what you have here.
But at least add a comment at the function declaration that we know the
behavior is not optimal and should therefore not be used on deep
hierarchies (where the quadratic behavior would definitely be noticeable).

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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