Re: [PATCH 2/2] util: command: improve generic mass close of fds
On Wed, 19 Aug 2020 11:55:06 +0100
Daniel P. Berrangé <berrange(a)redhat.com> wrote:
On Wed, Aug 19, 2020 at 12:03:41PM +0200, Natanael Copa wrote:
> Add a portable generic implementation of virMassClose as fallback on
> non-FreeBSD and non-glibc.
>
> This implementation uses poll(2) to look for open files to keep
> performance reasonable while not using any mallocs.
The patch isn't avoiding malloc - the virReportSystemError calls
all trigger mallocs in the error reporting code, as well as
triggering the logging code which mallocs.
Even if it does not avoid all malloc's it does avoid the allocation and
use of virBitmap and made my system usable again, probably because
there was nothing to report.
I also consider use of malloc in virReportSystemError a different
problem and should probably be dealth with separately. (how would you
report out-of-memory if the error reporting needs allocate memory?)
I suppose the commit message needs to be improved.
The idea of using poll() as a fallback still makes sense as a
general feature though.
Credit for the idea goes to Rich Felker.
On a side note, it is theoretically possible to open a large number of
files, then set max fds to a lower number and you'll have open file
handles above sysconf(_SC_OPEN_MAX) that will not be closed. The
current fallback implementation also has this problem.
>
> This solves a deadlock with musl libc.
>
> Signed-off-by: Natanael Copa <ncopa(a)alpinelinux.org>
> ---
> src/util/vircommand.c | 76 +++++++++++++++++++++++++++++++++----------
> 1 file changed, 58 insertions(+), 18 deletions(-)
>
> diff --git a/src/util/vircommand.c b/src/util/vircommand.c
> index 17e5bb00d3..06579cfb44 100644
> --- a/src/util/vircommand.c
> +++ b/src/util/vircommand.c
...
> @@ -546,7 +536,7 @@ virCommandMassClose(virCommandPtr cmd,
> return 0;
> }
>
> -# else /* ! __FreeBSD__ */
> +# elif defined(__GLIBC_) /* ! __FreeBSD__ */
I did a typo here. __GLIBC_ instead of __GLIBC__
-nc