Re: [PATCH 2/3] vircommand: Utilize close_range()

On Wed, Jun 21, 2023 at 04:09:10PM +0200, Michal Privoznik wrote: > As of commit v5.9-rc1~160^2~3 the Linux kernel has close_range() > syscall, which closes not just one FD but whole range. In glibc > this is exposed by automatically generated wrapper of the same > name. In musl, this is not exposed, yet, but we can call the > syscall() directly. In either case, we have to deal with a > situation, when the kernel we're running under does not have the > syscall as glibc deliberately does not implement fallback. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > meson.build | 1 + > src/util/vircommand.c | 117 +++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 117 insertions(+), 1 deletion(-) > > diff --git a/meson.build b/meson.build > index a4b52b6156..ecfc1b6bdf 100644 > --- a/meson.build > +++ b/meson.build > @@ -573,6 +573,7 @@ libvirt_export_dynamic = cc.first_supported_link_argument([ > # check availability of various common functions (non-fatal if missing) > > functions = [ > + 'close_range', > 'closefrom', > 'elf_aux_info', > 'explicit_bzero', > diff --git a/src/util/vircommand.c b/src/util/vircommand.c > index b8b8d48f92..e826f5f348 100644 > --- a/src/util/vircommand.c > +++ b/src/util/vircommand.c > @@ -479,13 +479,128 @@ virExecCommon(virCommand *cmd, gid_t *groups, int ngroups) > return 0; > } > > -# ifdef WITH_CLOSEFROM > +# if defined(WITH_CLOSE_RANGE) || \ > + (defined(WITH_SYS_SYSCALL_H) && defined(SYS_close_range)) > +# define USE_CLOSE_RANGE > +# elif defined(WITH_CLOSEFROM) > # define USE_CLOSEFROM > # else > # define USE_GENERIC > # endif Although you've optimized to take advantage of using close_range to deal with gaps, in the interests of code brevity, I wonder if we shouldn't simplify this patch down to merely: #ifndef WITH_CLOSEFROM # ifdef WITH_CLOSE_RANGE # define closefrom(n) closerange(n, ~0U, 0) # define WITH_CLOSEFROM # else if defined(WITH_SYS_SYSCALL_H) && defined(SYS_close_range) # define closefrom(n) syscall(SYS_close_range, n, ~0U, 0) # define WITH_CLOSEFROM # endif #endif thus just using the existing closefrom() impl (assuming you adjust the previous patch to check return status for ENOSYS and do legacy fallback. Not convinced it is worth worrying about the FD gaps ?