Re: [PATCH 3/4] vircommand: Make sysconf(_SC_OPEN_MAX) failure non-fatal

On Wed, Aug 28, 2024 at 02:16:16PM +0200, Michal Privoznik wrote: > The point of calling sysconf(_SC_OPEN_MAX) is to allocate big > enough bitmap so that subsequent call to > virCommandMassCloseGetFDsDir() can just set the bit instead of > expanding memory (this code runs in a forked off child and thus > using async-signal-unsafe functions like malloc() is a bit > tricky). > > But on some systems the limit for opened FDs is virtually > non-existent (typically macOS Ventura started reporting EINVAL). > > But with both glibc and musl using malloc() after fork() is safe. > And with sufficiently new glib too, as it's using malloc() with > newer releases instead of their own allocator. > > Therefore, pick a sufficiently large value (glibc falls back to > 256, [1], so 1024 should be good enough) to fall back to and make > the error non-fatal. > > 1: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/... > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > src/util/vircommand.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/src/util/vircommand.c b/src/util/vircommand.c > index c0aab85c53..e48f361114 100644 > --- a/src/util/vircommand.c > +++ b/src/util/vircommand.c > @@ -493,7 +493,7 @@ virCommandMassCloseGetFDsDir(virBitmap *fds, > return -1; > } > > - ignore_value(virBitmapSetBit(fds, fd)); > + virBitmapSetBitExpand(fds, fd); > } > > if (rc < 0) > @@ -537,10 +537,8 @@ virCommandMassCloseFrom(virCommand *cmd, > * Therefore we can safely allocate memory here (and transitively call > * opendir/readdir) without a deadlock. */ > > t - if (openmax < 0) { > - virReportSystemError(errno, "%s", _("sysconf(_SC_OPEN_MAX) failed")); > - return -1; > - } > + if (openmax <= 0) > + openmax = 1024; Darwin has a OPEN_MAX constant that is x10 bigger than this: https://github.com/apple/darwin-xnu/blob/main/bsd/sys/syslimits.h#L104 IMHO we should be using that instead