Re: [PATCH v3 5/6] util/async-teardown: Use qemu_close_range() to close fds

2023-06-19 Thread Claudio Imbrenda
On Sat, 17 Jun 2023 13:36:20 +0800
Bin Meng  wrote:

> From: Zhangjin Wu 
> 
> Based on the old close_all_open_fd() of util/async-teardown.c, a new
> generic qemu_close_range() has been added in osdep.c.
> 
> Now, let's switch over to use the generic qemu_close_range().
> 
> Signed-off-by: Zhangjin Wu 
> Signed-off-by: Bin Meng 
> 
> ---
> 
> Changes in v3:
> - limit the last_fd of qemu_close_range() to sysconf(_SC_OPEN_MAX)
> 
> Changes in v2:
> - new patch: "util/async-teardown: Use qemu_close_range() to close fds"
> 
>  util/async-teardown.c | 42 ++
>  1 file changed, 2 insertions(+), 40 deletions(-)
> 
> diff --git a/util/async-teardown.c b/util/async-teardown.c
> index 7e0177a8da..e102912f3f 100644
> --- a/util/async-teardown.c
> +++ b/util/async-teardown.c
> @@ -29,44 +29,6 @@
>  
>  static pid_t the_ppid;
>  
> -/*
> - * Close all open file descriptors.
> - */
> -static void close_all_open_fd(void)
> -{
> -struct dirent *de;
> -int fd, dfd;
> -DIR *dir;
> -
> -#ifdef CONFIG_CLOSE_RANGE
> -int r = close_range(0, ~0U, 0);
> -if (!r) {
> -/* Success, no need to try other ways. */
> -return;
> -}
> -#endif
> -
> -dir = opendir("/proc/self/fd");
> -if (!dir) {
> -/* If /proc is not mounted, close fds one by one. */
> -int open_max = sysconf(_SC_OPEN_MAX), i;
> -for (i = 0; i < open_max; i++) {
> -close(i);
> -}
> -return;
> -}
> -/* Avoid closing the directory. */
> -dfd = dirfd(dir);
> -
> -for (de = readdir(dir); de; de = readdir(dir)) {
> -fd = atoi(de->d_name);
> -if (fd != dfd) {
> -close(fd);
> -}
> -}
> -closedir(dir);
> -}
> -
>  static void hup_handler(int signal)
>  {
>  /* Check every second if this process has been reparented. */
> @@ -84,6 +46,7 @@ static int async_teardown_fn(void *arg)
>  struct sigaction sa = { .sa_handler = hup_handler };
>  sigset_t hup_signal;
>  char name[16];
> +int open_max = sysconf(_SC_OPEN_MAX);
>  
>  /* Set a meaningful name for this process. */
>  snprintf(name, 16, "cleanup/%d", the_ppid);
> @@ -92,9 +55,8 @@ 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.
> - * Not using close_range for increased compatibility with older kernels.
>   */
> -close_all_open_fd();
> +qemu_close_range(0, open_max - 1);

I think it would look easier to read if you just put the sysconf() call
here and avoid an extra variable

>  
>  /* Set up a handler for SIGHUP and unblock SIGHUP. */
>  sigaction(SIGHUP, &sa, NULL);




[PATCH v3 5/6] util/async-teardown: Use qemu_close_range() to close fds

2023-06-16 Thread Bin Meng
From: Zhangjin Wu 

Based on the old close_all_open_fd() of util/async-teardown.c, a new
generic qemu_close_range() has been added in osdep.c.

Now, let's switch over to use the generic qemu_close_range().

Signed-off-by: Zhangjin Wu 
Signed-off-by: Bin Meng 

---

Changes in v3:
- limit the last_fd of qemu_close_range() to sysconf(_SC_OPEN_MAX)

Changes in v2:
- new patch: "util/async-teardown: Use qemu_close_range() to close fds"

 util/async-teardown.c | 42 ++
 1 file changed, 2 insertions(+), 40 deletions(-)

diff --git a/util/async-teardown.c b/util/async-teardown.c
index 7e0177a8da..e102912f3f 100644
--- a/util/async-teardown.c
+++ b/util/async-teardown.c
@@ -29,44 +29,6 @@
 
 static pid_t the_ppid;
 
-/*
- * Close all open file descriptors.
- */
-static void close_all_open_fd(void)
-{
-struct dirent *de;
-int fd, dfd;
-DIR *dir;
-
-#ifdef CONFIG_CLOSE_RANGE
-int r = close_range(0, ~0U, 0);
-if (!r) {
-/* Success, no need to try other ways. */
-return;
-}
-#endif
-
-dir = opendir("/proc/self/fd");
-if (!dir) {
-/* If /proc is not mounted, close fds one by one. */
-int open_max = sysconf(_SC_OPEN_MAX), i;
-for (i = 0; i < open_max; i++) {
-close(i);
-}
-return;
-}
-/* Avoid closing the directory. */
-dfd = dirfd(dir);
-
-for (de = readdir(dir); de; de = readdir(dir)) {
-fd = atoi(de->d_name);
-if (fd != dfd) {
-close(fd);
-}
-}
-closedir(dir);
-}
-
 static void hup_handler(int signal)
 {
 /* Check every second if this process has been reparented. */
@@ -84,6 +46,7 @@ static int async_teardown_fn(void *arg)
 struct sigaction sa = { .sa_handler = hup_handler };
 sigset_t hup_signal;
 char name[16];
+int open_max = sysconf(_SC_OPEN_MAX);
 
 /* Set a meaningful name for this process. */
 snprintf(name, 16, "cleanup/%d", the_ppid);
@@ -92,9 +55,8 @@ 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.
- * Not using close_range for increased compatibility with older kernels.
  */
-close_all_open_fd();
+qemu_close_range(0, open_max - 1);
 
 /* Set up a handler for SIGHUP and unblock SIGHUP. */
 sigaction(SIGHUP, &sa, NULL);
-- 
2.34.1