On Sat, Jan 04, 2025 at 05:04:08PM +0900, Akihiko Odaki 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.od...@daynix.com>
> ---
> net/tap.c | 169
> ++++++++++++++++++++++++--------------------------------------
> 1 file changed, 66 insertions(+), 103 deletions(-)
>
> diff --git a/net/tap.c b/net/tap.c
> index ae1c7e398321..392a024f8ed9 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -385,56 +385,30 @@ static TAPState *net_tap_fd_init(NetClientState *peer,
> return s;
> }
>
> -static void close_all_fds_after_fork(int excluded_fd)
> +static void unset_cloexec(gpointer data)
> {
> - const int skip_fd[] = {STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO,
> - excluded_fd};
> - unsigned int nskip = ARRAY_SIZE(skip_fd);
> -
> - /*
> - * skip_fd must be an ordered array of distinct fds, exclude
> - * excluded_fd if already included in the [STDIN_FILENO - STDERR_FILENO]
> - * range
> - */
> - if (excluded_fd <= STDERR_FILENO) {
> - nskip--;
> - }
> -
> - qemu_close_all_open_fd(skip_fd, nskip);
> + g_assert(!fcntl(GPOINTER_TO_INT(data), F_SETFD, 0));
> }
>
> 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_CHILD_INHERITS_STDIN,
> + unset_cloexec, GINT_TO_POINTER(fd),
> + NULL, NULL, &status, &error)) {
This unset_cloexec callback is relying on knowledge of current internal
impl details of g_spawn_sync. The API docs say that all file descriptors
will be closed, except for stdin/out/err. We should not assume glib is
doing this by setting O_CLOEXEC, as opposed to directly calling close().
If we need specific FDs to remain open, we neeed to be using the other
g_spawn_async_with_pipes_and_fds API that accepts a list of FDs to remain
open.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
