diff mbox series

[v8,4/5] qemu/osdep: Add excluded fd parameter to qemu_close_all_open_fd()

Message ID 20240802145423.3232974-5-cleger@rivosinc.com (mailing list archive)
State New, archived
Headers show
Series qemu/osdep: add a qemu_close_all_open_fd() helper | expand

Commit Message

Clément Léger Aug. 2, 2024, 2:54 p.m. UTC
In order for this function to be usable by tap.c code, add a list of
file descriptors that should not be closed.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 include/qemu/osdep.h    |   8 ++-
 system/async-teardown.c |   2 +-
 util/oslib-posix.c      | 106 ++++++++++++++++++++++++++++++++++------
 3 files changed, 97 insertions(+), 19 deletions(-)

Comments

Richard Henderson Aug. 5, 2024, 12:33 a.m. UTC | #1
On 8/3/24 00:54, Clément Léger wrote:
> In order for this function to be usable by tap.c code, add a list of
> file descriptors that should not be closed.
> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>   include/qemu/osdep.h    |   8 ++-
>   system/async-teardown.c |   2 +-
>   util/oslib-posix.c      | 106 ++++++++++++++++++++++++++++++++++------
>   3 files changed, 97 insertions(+), 19 deletions(-)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index de77c5c254..4cc4c32b14 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -760,9 +760,13 @@ int qemu_fdatasync(int fd);
>   /**
>    * qemu_close_all_open_fd:
>    *
> - * Close all open file descriptors
> + * Close all open file descriptors except the ones supplied in the @skip array
> + *
> + * @skip: ordered array of distinct file descriptors that should not be closed
> + *        if any, or NULL.
> + * @nskip: number of entries in the @skip array or 0 if @skip is NULL.
>    */
> -void qemu_close_all_open_fd(void);
> +void qemu_close_all_open_fd(const int *skip, unsigned int nskip);
>   
>   /**
>    * Sync changes made to the memory mapped file back to the backing
> diff --git a/system/async-teardown.c b/system/async-teardown.c
> index edf49e1007..9148ee8d04 100644
> --- a/system/async-teardown.c
> +++ b/system/async-teardown.c
> @@ -52,7 +52,7 @@ static int async_teardown_fn(void *arg)
>        * Close all file descriptors that might have been inherited from the
>        * main qemu process when doing clone, needed to make libvirt happy.
>        */
> -    qemu_close_all_open_fd();
> +    qemu_close_all_open_fd(NULL, 0);
>   
>       /* Set up a handler for SIGHUP and unblock SIGHUP. */
>       sigaction(SIGHUP, &sa, NULL);
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 9b79fc7cff..2636dcd1e0 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -808,11 +808,12 @@ int qemu_msync(void *addr, size_t length, int fd)
>       return msync(addr, length, MS_SYNC);
>   }
>   
> -static bool qemu_close_all_open_fd_proc(void)
> +static bool qemu_close_all_open_fd_proc(const int *skip, unsigned int nskip)
>   {
>       struct dirent *de;
>       int fd, dfd;
>       DIR *dir;
> +    unsigned int skip_start = 0, skip_end = nskip;
>   
>       dir = opendir("/proc/self/fd");
>       if (!dir) {
> @@ -823,8 +824,33 @@ static bool qemu_close_all_open_fd_proc(void)
>       dfd = dirfd(dir);
>   
>       for (de = readdir(dir); de; de = readdir(dir)) {
> +        bool close_fd = true;
> +
> +        if (de->d_name[0] == '.') {
> +            continue;
> +        }
>           fd = atoi(de->d_name);
> -        if (fd != dfd) {
> +        if (fd == dfd) {
> +            continue;
> +        }
> +
> +        for (unsigned int i = skip_start; i < skip_end; i++) {
> +            if (fd < skip[i]) {
> +                /* We are below the next skipped fd, break */
> +                break;
> +            } else if (fd == skip[i]) {
> +                close_fd = false;
> +                /* Restrict the range as we found fds matching start/end */
> +                if (i == skip_start) {
> +                    skip_start++;
> +                } else if (i == skip_end) {
> +                    skip_end--;
> +                }
> +                break;
> +            }
> +        }
> +
> +        if (close_fd) {
>               close(fd);
>           }
>       }
> @@ -833,24 +859,68 @@ static bool qemu_close_all_open_fd_proc(void)
>       return true;
>   }
>   
> -static bool qemu_close_all_open_fd_close_range(void)
> +static bool qemu_close_all_open_fd_close_range(const int *skip,
> +                                               unsigned int nskip,
> +                                               int open_max)
>   {
>   #ifdef CONFIG_CLOSE_RANGE
> -    int r = close_range(0, ~0U, 0);
> -    if (!r) {
> -        /* Success, no need to try other ways. */
> -        return true;
> -    }
> -#endif
> +    int first = 0, last = open_max - 1;
> +    unsigned int cur_skip = 0;
> +    int ret;
> +
> +    do {
> +        /* Find the start boundary of the range to close */
> +        while (cur_skip < nskip && first == skip[cur_skip]) {
> +            cur_skip++;
> +            first++;
> +        }
> +
> +        /* Find the upper boundary of the range to close */
> +        if (cur_skip < nskip) {
> +            last = skip[cur_skip] - 1;
> +        }
> +        /*
> +         * Adjust the maximum fd to close if it's above what the system
> +         * supports
> +         */
> +        if (last > open_max) {
> +            last = open_max;

You wanted open_max - 1 here as well.

I have fixed this up, and tidied a bit more while queuing for pull.


r~
diff mbox series

Patch

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index de77c5c254..4cc4c32b14 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -760,9 +760,13 @@  int qemu_fdatasync(int fd);
 /**
  * qemu_close_all_open_fd:
  *
- * Close all open file descriptors
+ * Close all open file descriptors except the ones supplied in the @skip array
+ *
+ * @skip: ordered array of distinct file descriptors that should not be closed
+ *        if any, or NULL.
+ * @nskip: number of entries in the @skip array or 0 if @skip is NULL.
  */
-void qemu_close_all_open_fd(void);
+void qemu_close_all_open_fd(const int *skip, unsigned int nskip);
 
 /**
  * Sync changes made to the memory mapped file back to the backing
diff --git a/system/async-teardown.c b/system/async-teardown.c
index edf49e1007..9148ee8d04 100644
--- a/system/async-teardown.c
+++ b/system/async-teardown.c
@@ -52,7 +52,7 @@  static int async_teardown_fn(void *arg)
      * Close all file descriptors that might have been inherited from the
      * main qemu process when doing clone, needed to make libvirt happy.
      */
-    qemu_close_all_open_fd();
+    qemu_close_all_open_fd(NULL, 0);
 
     /* Set up a handler for SIGHUP and unblock SIGHUP. */
     sigaction(SIGHUP, &sa, NULL);
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 9b79fc7cff..2636dcd1e0 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -808,11 +808,12 @@  int qemu_msync(void *addr, size_t length, int fd)
     return msync(addr, length, MS_SYNC);
 }
 
-static bool qemu_close_all_open_fd_proc(void)
+static bool qemu_close_all_open_fd_proc(const int *skip, unsigned int nskip)
 {
     struct dirent *de;
     int fd, dfd;
     DIR *dir;
+    unsigned int skip_start = 0, skip_end = nskip;
 
     dir = opendir("/proc/self/fd");
     if (!dir) {
@@ -823,8 +824,33 @@  static bool qemu_close_all_open_fd_proc(void)
     dfd = dirfd(dir);
 
     for (de = readdir(dir); de; de = readdir(dir)) {
+        bool close_fd = true;
+
+        if (de->d_name[0] == '.') {
+            continue;
+        }
         fd = atoi(de->d_name);
-        if (fd != dfd) {
+        if (fd == dfd) {
+            continue;
+        }
+
+        for (unsigned int i = skip_start; i < skip_end; i++) {
+            if (fd < skip[i]) {
+                /* We are below the next skipped fd, break */
+                break;
+            } else if (fd == skip[i]) {
+                close_fd = false;
+                /* Restrict the range as we found fds matching start/end */
+                if (i == skip_start) {
+                    skip_start++;
+                } else if (i == skip_end) {
+                    skip_end--;
+                }
+                break;
+            }
+        }
+
+        if (close_fd) {
             close(fd);
         }
     }
@@ -833,24 +859,68 @@  static bool qemu_close_all_open_fd_proc(void)
     return true;
 }
 
-static bool qemu_close_all_open_fd_close_range(void)
+static bool qemu_close_all_open_fd_close_range(const int *skip,
+                                               unsigned int nskip,
+                                               int open_max)
 {
 #ifdef CONFIG_CLOSE_RANGE
-    int r = close_range(0, ~0U, 0);
-    if (!r) {
-        /* Success, no need to try other ways. */
-        return true;
-    }
-#endif
+    int first = 0, last = open_max - 1;
+    unsigned int cur_skip = 0;
+    int ret;
+
+    do {
+        /* Find the start boundary of the range to close */
+        while (cur_skip < nskip && first == skip[cur_skip]) {
+            cur_skip++;
+            first++;
+        }
+
+        /* Find the upper boundary of the range to close */
+        if (cur_skip < nskip) {
+            last = skip[cur_skip] - 1;
+        }
+        /*
+         * Adjust the maximum fd to close if it's above what the system
+         * supports
+         */
+        if (last > open_max) {
+            last = open_max;
+            /*
+             * We can directly skip the remaining skip fds since the current
+             * one is already above the maximum supported one.
+             */
+            cur_skip = nskip;
+        }
+        /* If last was adjusted, we might be > first, bail out */
+        if (first > last) {
+            break;
+        }
+
+        ret = close_range(first, last, 0);
+        if (ret < 0) {
+            return false;
+        }
+        first = last + 1;
+        last = open_max;
+    } while (cur_skip < nskip);
+
+    return true;
+#else
     return false;
+#endif
 }
 
-static void qemu_close_all_open_fd_fallback(void)
+static void qemu_close_all_open_fd_fallback(const int *skip, unsigned int nskip,
+                                            int open_max)
 {
-    int open_max = sysconf(_SC_OPEN_MAX), i;
+    unsigned int cur_skip = 0;
 
     /* Fallback */
-    for (i = 0; i < open_max; i++) {
+    for (int i = 0; i < open_max; i++) {
+        if (cur_skip < nskip && i == skip[cur_skip]) {
+            cur_skip++;
+            continue;
+        }
         close(i);
     }
 }
@@ -858,10 +928,14 @@  static void qemu_close_all_open_fd_fallback(void)
 /*
  * Close all open file descriptors.
  */
-void qemu_close_all_open_fd(void)
+void qemu_close_all_open_fd(const int *skip, unsigned int nskip)
 {
-    if (!qemu_close_all_open_fd_close_range() &&
-        !qemu_close_all_open_fd_proc()) {
-        qemu_close_all_open_fd_fallback();
+    int open_max = sysconf(_SC_OPEN_MAX);
+
+    assert(skip != NULL || nskip == 0);
+
+    if (!qemu_close_all_open_fd_close_range(skip, nskip, open_max) &&
+        !qemu_close_all_open_fd_proc(skip, nskip)) {
+        qemu_close_all_open_fd_fallback(skip, nskip, open_max);
     }
 }