[PATCH] Revert "vircommand: Parse /dev/fd on *BSD-like systems when looking for opened FDs"
Unfortunately, devfs on FreeBSD (accessible via /dev/fd) exposes only those FDs which can be represented as a file. To cite manpage [1]: The files /dev/fd/0 through /dev/fd/# refer to file descriptors which can be accessed through the file system. This means FDs representing pipes and/or unnamed sockets are not visible by default. To expose all FDs a slightly different filesystem must be mounted [2]: mount -t fdescfs none /dev/fd Apparently, on my test machine fdescfs is mounted by default and thus I haven't seen any problem. Only after aforementioned patch was merged our CI started reporting problems. While we could try to figure out whether correct FS is mounted, it's a needless micro optimization. Just revert the code to the state it was before I touched it. 1: https://man.freebsd.org/cgi/man.cgi?query=fd&sektion=4&manpath=freebsd-release-ports 2: https://man.freebsd.org/cgi/man.cgi?query=fdescfs&sektion=5&n=1 This reverts commit 308ec0fb2c77f4867179f00c628f05d1d784f370. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/vircommand.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 969d4c28ef..ea52acfbb8 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -472,7 +472,7 @@ virExecCommon(virCommand *cmd, gid_t *groups, int ngroups) return 0; } -# if defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__) +# ifdef __linux__ static int virCommandMassCloseGetFDsDir(virBitmap *fds, const char *dirName) @@ -502,7 +502,7 @@ virCommandMassCloseGetFDsDir(virBitmap *fds, return 0; } -# endif /* defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__) */ +# endif /* __linux__ */ static int virCommandMassCloseGetFDs(virBitmap *fds) @@ -513,8 +513,6 @@ virCommandMassCloseGetFDs(virBitmap *fds) * onto child process (well, the one we will exec soon since this * is called from the child). */ return virCommandMassCloseGetFDsDir(fds, "/proc/self/fd"); -# elif defined(__APPLE__) || defined(__FreeBSD__) - return virCommandMassCloseGetFDsDir(fds, "/dev/fd"); # else virBitmapSetAll(fds); return 0; -- 2.44.2
On Mon, Sep 16, 2024 at 11:00:04AM +0200, Michal Privoznik wrote:
Unfortunately, devfs on FreeBSD (accessible via /dev/fd) exposes only those FDs which can be represented as a file. To cite manpage [1]:
The files /dev/fd/0 through /dev/fd/# refer to file descriptors which can be accessed through the file system.
This means FDs representing pipes and/or unnamed sockets are not visible by default. To expose all FDs a slightly different filesystem must be mounted [2]:
mount -t fdescfs none /dev/fd
Apparently, on my test machine fdescfs is mounted by default and thus I haven't seen any problem. Only after aforementioned patch was merged our CI started reporting problems. While we could try to figure out whether correct FS is mounted, it's a needless micro optimization. Just revert the code to the state it was before I touched it.
1: https://man.freebsd.org/cgi/man.cgi?query=fd&sektion=4&manpath=freebsd-release-ports 2: https://man.freebsd.org/cgi/man.cgi?query=fdescfs&sektion=5&n=1
This reverts commit 308ec0fb2c77f4867179f00c628f05d1d784f370.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
--- src/util/vircommand.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 969d4c28ef..ea52acfbb8 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -472,7 +472,7 @@ virExecCommon(virCommand *cmd, gid_t *groups, int ngroups) return 0; }
-# if defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__) +# ifdef __linux__ static int virCommandMassCloseGetFDsDir(virBitmap *fds, const char *dirName) @@ -502,7 +502,7 @@ virCommandMassCloseGetFDsDir(virBitmap *fds,
return 0; } -# endif /* defined(__linux__) || defined(__APPLE__) || defined(__FreeBSD__) */ +# endif /* __linux__ */
static int virCommandMassCloseGetFDs(virBitmap *fds) @@ -513,8 +513,6 @@ virCommandMassCloseGetFDs(virBitmap *fds) * onto child process (well, the one we will exec soon since this * is called from the child). */ return virCommandMassCloseGetFDsDir(fds, "/proc/self/fd"); -# elif defined(__APPLE__) || defined(__FreeBSD__) - return virCommandMassCloseGetFDsDir(fds, "/dev/fd"); # else virBitmapSetAll(fds); return 0; -- 2.44.2
participants (2)
-
Martin Kletzander -
Michal Privoznik