Re: [PATCH 2/2] qemu/osdep: handle sysconf(_SC_OPEN_MAX) return value == -1

[Philippe Mathieu-Daudé]

On 30/8/24 13:57, Clément Léger wrote:
> On 30/08/2024 13:31, Michael Tokarev wrote:
> > 30.08.2024 14:14, Clément Léger wrote:
> > > On some systems (MacOS for instance), sysconf(_SC_OPEN_MAX) can return
> > > -1. In that case we should fallback to using the OPEN_MAX define.
> > > According to "man sysconf", the OPEN_MAX define should be present and
> > > provided by either unistd.h and/or limits.h so include them for that
> > > purpose. For other OSes, just assume a maximum of 1024 files descriptors
> > > as a fallback.
> > >
> > > Fixes: 4ec5ebea078e ("qemu/osdep: Move close_all_open_fds() to oslib-
> > > posix")
> > > Reported-by: Daniel P. Berrangé <berrange@redhat.com>
> > > Signed-off-by: Clément Léger <cleger@rivosinc.com>
> >
> > Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
> >
> > > @@ -928,6 +933,13 @@ static void qemu_close_all_open_fd_fallback(const
> > > int *skip, unsigned int nskip,
> > >    void qemu_close_all_open_fd(const int *skip, unsigned int nskip)
> > >    {
> > >        int open_max = sysconf(_SC_OPEN_MAX);
> > > +    if (open_max == -1) {
> > > +#ifdef CONFIG_DARWIN
> > > +        open_max = OPEN_MAX;

Missing errno check.

> > > +#else
> > > +        open_max = 1024;
> > > +#endif
> >
> > BTW, Can we PLEASE cap this to 1024 in all cases? :)
> > (unrelated to this change but still).
>
> Hi Michael,
>
> Do you mean for all OSes or always using 1024 rather than using the
> sysconf returned value ?

Alternatively add:

  long qemu_sysconf(int name, long unsupported_default);

which returns value, unsupported_default if not supported, or -1.

> In any case, the code now uses close_range() or /proc/self/fd and is
> handling that efficiently.
>
> Thanks,
>
> Clément
>
> > /mjt