[PATCH 0/2] tap: Use g_spawn_sync() and g_spawn_check_wait_status()

[PATCH 0/2] tap: Use g_spawn_sync() and g_spawn_check_wait_status()

[Akihiko Odaki]

g_spawn_sync() gives an informative message if it fails to execute
the script instead of reporting exiting status 1.

g_spawn_check_wait_status() also gives an message easier to understand
than the raw value returned by waitpid().

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Akihiko Odaki (2):
      glib-compat: Define g_spawn_check_wait_status()
      tap: Use g_spawn_sync() and g_spawn_check_wait_status()

 include/glib-compat.h |  2 ++
 net/tap.c             | 52 ++++++++++++++++++++++-----------------------------
 2 files changed, 24 insertions(+), 30 deletions(-)
---
base-commit: 9c74490bff6c8886a922008d0c9ce6cae70dd17e
change-id: 20231219-glib-034a34bb05d8

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>

[PATCH 1/2] glib-compat: Define g_spawn_check_wait_status()

[Akihiko Odaki]

g_spawn_check_exit_status() is renamed to g_spawn_check_wait_status()
in 2.70.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 include/glib-compat.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/glib-compat.h b/include/glib-compat.h
index 43a562974d80..5f76fbd28e6b 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -68,6 +68,8 @@
  * without generating warnings.
  */
 
+#define g_spawn_check_wait_status g_spawn_check_exit_status
+
 /*
  * g_memdup2_qemu:
  * @mem: (nullable): the memory to copy.

-- 
2.43.0

[PATCH 2/2] tap: Use g_spawn_sync() and g_spawn_check_wait_status()

[Akihiko Odaki]

g_spawn_sync() gives an informative message if it fails to execute
the script instead of reporting exiting status 1.

g_spawn_check_wait_status() also gives an message easier to understand
than the raw value returned by waitpid().

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 net/tap.c | 52 ++++++++++++++++++++++------------------------------
 1 file changed, 22 insertions(+), 30 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index c23d0323c2ae..74f718613009 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -444,44 +444,36 @@ static TAPState *net_tap_fd_init(NetClientState *peer,
     return s;
 }
 
+static void close_fds_except(gpointer data)
+{
+    int open_max = sysconf(_SC_OPEN_MAX), i;
+
+    for (i = 3; i < open_max; i++) {
+        if (i != (intptr_t)data) {
+            close(i);
+        }
+    }
+}
+
 static void launch_script(const char *setup_script, const char *ifname,
                           int fd, Error **errp)
 {
-    int pid, status;
-    char *args[3];
-    char **parg;
+    gint status;
+    gchar *argv[] = { (gchar *)setup_script, (gchar *)ifname, NULL };
+    g_autoptr(GError) error = NULL;
 
     /* try to launch network script */
-    pid = fork();
-    if (pid < 0) {
-        error_setg_errno(errp, errno, "could not launch network script %s",
-                         setup_script);
+    if (!g_spawn_sync(NULL, argv, NULL, G_SPAWN_LEAVE_DESCRIPTORS_OPEN,
+                      close_fds_except, (gpointer)(intptr_t)fd, NULL, NULL,
+                      &status, &error)) {
+        error_setg(errp, "could not launch network script %s: %s",
+                   setup_script, error->message);
         return;
     }
-    if (pid == 0) {
-        int open_max = sysconf(_SC_OPEN_MAX), i;
 
-        for (i = 3; i < open_max; i++) {
-            if (i != fd) {
-                close(i);
-            }
-        }
-        parg = args;
-        *parg++ = (char *)setup_script;
-        *parg++ = (char *)ifname;
-        *parg = NULL;
-        execv(setup_script, args);
-        _exit(1);
-    } else {
-        while (waitpid(pid, &status, 0) != pid) {
-            /* loop */
-        }
-
-        if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
-            return;
-        }
-        error_setg(errp, "network script %s failed with status %d",
-                   setup_script, status);
+    if (!g_spawn_check_wait_status(status, &error)) {
+        error_setg(errp, "network script %s failed: %s",
+                   setup_script, error->message);
     }
 }
 

-- 
2.43.0

Re: [PATCH 0/2] tap: Use g_spawn_sync() and g_spawn_check_wait_status()

[Jason Wang]

On Tue, Dec 19, 2023 at 7:59 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> g_spawn_sync() gives an informative message if it fails to execute
> the script instead of reporting exiting status 1.
>
> g_spawn_check_wait_status() also gives an message easier to understand
> than the raw value returned by waitpid().
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> Akihiko Odaki (2):
>       glib-compat: Define g_spawn_check_wait_status()
>       tap: Use g_spawn_sync() and g_spawn_check_wait_status()
>
>  include/glib-compat.h |  2 ++
>  net/tap.c             | 52 ++++++++++++++++++++++-----------------------------
>  2 files changed, 24 insertions(+), 30 deletions(-)
> ---
> base-commit: 9c74490bff6c8886a922008d0c9ce6cae70dd17e
> change-id: 20231219-glib-034a34bb05d8
>
> Best regards,
> --
> Akihiko Odaki <akihiko.odaki@daynix.com>

I've queued this for 9.1

Thanks

>