Thanks: Eric Blake. --- src/Makefile.am | 1 + src/command.c | 7 ++---- src/guestfs-internal.h | 4 +++ src/launch-direct.c | 11 +++------ src/launch-uml.c | 11 +++------ src/umask.c | 10 ++------ src/wait.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 84 insertions(+), 27 deletions(-) create mode 100644 src/wait.c diff --git a/src/Makefile.am b/src/Makefile.am index 34c4fa6..7cc80c4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -131,6 +131,7 @@ libguestfs_la_SOURCES = \ structs-free.c \ tmpdirs.c \ umask.c \ + wait.c \ whole-file.c \ libguestfs.syms diff --git a/src/command.c b/src/command.c index 866847d..e2bbf65 100644 --- a/src/command.c +++ b/src/command.c @@ -74,7 +74,6 @@ #include <assert.h> #include <sys/types.h> #include <sys/stat.h> -#include <sys/wait.h> #include <sys/select.h> #ifdef HAVE_SYS_TIME_H @@ -692,10 +691,8 @@ wait_command (struct command *cmd) { int status; - if (waitpid (cmd->pid, &status, 0) == -1) { - perrorf (cmd->g, "waitpid"); + if (guestfs_int_waitpid (cmd->g, cmd->pid, &status, "command") == -1) return -1; - } cmd->pid = 0; @@ -902,7 +899,7 @@ guestfs_int_cmd_close (struct command *cmd) free (cmd->outbuf.buffer); if (cmd->pid > 0) - waitpid (cmd->pid, NULL, 0); + guestfs_int_waitpid_noerror (cmd->pid); for (child_rlimit = cmd->child_rlimits; child_rlimit != NULL; child_rlimit = child_rlimit_next) { diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index bf107d0..7b3927b 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -918,4 +918,8 @@ extern int guestfs_int_validate_guid (const char *); /* umask.c */ extern int guestfs_int_getumask (guestfs_h *g); +/* wait.c */ +extern int guestfs_int_waitpid (guestfs_h *g, pid_t pid, int *status, const char *errmsg); +extern void guestfs_int_waitpid_noerror (pid_t pid); + #endif /* GUESTFS_INTERNAL_H_ */ diff --git a/src/launch-direct.c b/src/launch-direct.c index 322737d..ee0a855 100644 --- a/src/launch-direct.c +++ b/src/launch-direct.c @@ -26,7 +26,6 @@ #include <errno.h> #include <fcntl.h> #include <sys/types.h> -#include <sys/wait.h> #include <sys/stat.h> #include <signal.h> #include <sys/socket.h> @@ -851,8 +850,8 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) close (sv[0]); if (data->pid > 0) kill (data->pid, 9); if (data->recoverypid > 0) kill (data->recoverypid, 9); - if (data->pid > 0) waitpid (data->pid, NULL, 0); - if (data->recoverypid > 0) waitpid (data->recoverypid, NULL, 0); + if (data->pid > 0) guestfs_int_waitpid_noerror (data->pid); + if (data->recoverypid > 0) guestfs_int_waitpid_noerror (data->recoverypid); data->pid = 0; data->recoverypid = 0; memset (&g->launch_t, 0, sizeof g->launch_t); @@ -1484,16 +1483,14 @@ shutdown_direct (guestfs_h *g, void *datav, int check_for_errors) /* Wait for subprocess(es) to exit. */ if (g->recovery_proc /* RHBZ#998482 */ && data->pid > 0) { - if (waitpid (data->pid, &status, 0) == -1) { - perrorf (g, "waitpid (qemu)"); + if (guestfs_int_waitpid (g, data->pid, &status, "qemu") == -1) ret = -1; - } else if (!WIFEXITED (status) || WEXITSTATUS (status) != 0) { guestfs_int_external_command_failed (g, status, g->hv, NULL); ret = -1; } } - if (data->recoverypid > 0) waitpid (data->recoverypid, NULL, 0); + if (data->recoverypid > 0) guestfs_int_waitpid_noerror (data->recoverypid); data->pid = data->recoverypid = 0; diff --git a/src/launch-uml.c b/src/launch-uml.c index 318081c..8c48795 100644 --- a/src/launch-uml.c +++ b/src/launch-uml.c @@ -26,7 +26,6 @@ #include <unistd.h> #include <sys/types.h> #include <sys/socket.h> -#include <sys/wait.h> #include <sys/signal.h> #include <libintl.h> @@ -470,8 +469,8 @@ launch_uml (guestfs_h *g, void *datav, const char *arg) close (dsv[0]); if (data->pid > 0) kill (data->pid, SIGKILL); if (data->recoverypid > 0) kill (data->recoverypid, SIGKILL); - if (data->pid > 0) waitpid (data->pid, NULL, 0); - if (data->recoverypid > 0) waitpid (data->recoverypid, NULL, 0); + if (data->pid > 0) guestfs_int_waitpid_noerror (data->pid); + if (data->recoverypid > 0) guestfs_int_waitpid_noerror (data->recoverypid); data->pid = 0; data->recoverypid = 0; memset (&g->launch_t, 0, sizeof g->launch_t); @@ -535,10 +534,8 @@ shutdown_uml (guestfs_h *g, void *datav, int check_for_errors) /* Wait for subprocess(es) to exit. */ if (data->pid > 0) { - if (waitpid (data->pid, &status, 0) == -1) { - perrorf (g, "waitpid (vmlinux)"); + if (guestfs_int_waitpid (g, data->pid, &status, "vmlinux") == -1) ret = -1; - } /* Note it's normal for the pre-3.11 vmlinux process to exit with * status "killed by signal 15" (where 15 == SIGTERM). Post 3.11 * the exit status can normally be 1. @@ -552,7 +549,7 @@ shutdown_uml (guestfs_h *g, void *datav, int check_for_errors) ret = -1; } } - if (data->recoverypid > 0) waitpid (data->recoverypid, NULL, 0); + if (data->recoverypid > 0) guestfs_int_waitpid_noerror (data->recoverypid); data->pid = data->recoverypid = 0; diff --git a/src/umask.c b/src/umask.c index 834ef43..b8748e8 100644 --- a/src/umask.c +++ b/src/umask.c @@ -29,7 +29,6 @@ #include <errno.h> #include <sys/stat.h> #include <sys/types.h> -#include <sys/wait.h> #include "ignore-value.h" @@ -91,18 +90,13 @@ guestfs_int_getumask (guestfs_h *g) if (read (fd[0], &mask, sizeof mask) != sizeof mask) { perrorf (g, "read"); close (fd[0]); - while (waitpid (pid, NULL, 0) == -1 && errno == EINTR) - ; + guestfs_int_waitpid_noerror (pid); return -1; } close (fd[0]); - again: - if (waitpid (pid, &status, 0) == -1) { - if (errno == EINTR) goto again; - perrorf (g, "waitpid"); + if (guestfs_int_waitpid (g, pid, &status, "umask") == -1) return -1; - } else if (!WIFEXITED (status) || WEXITSTATUS (status) != 0) { guestfs_int_external_command_failed (g, status, "umask", NULL); return -1; diff --git a/src/wait.c b/src/wait.c new file mode 100644 index 0000000..bdfb0b9 --- /dev/null +++ b/src/wait.c @@ -0,0 +1,67 @@ +/* libguestfs + * Copyright (C) 2016 Red Hat Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <errno.h> +#include <sys/types.h> +#include <sys/wait.h> + +#include "guestfs.h" +#include "guestfs-internal.h" + +/** + * A safe version of L<waitpid(3)> which retries if C<EINTR> is + * returned. + * + * I<Note:> this only needs to be used in the library, or in programs + * that install a non-restartable C<SIGCHLD> handler (which is not the + * case for any current libguestfs virt tools). + * + * If the main program installs a SIGCHLD handler and sets it to be + * non-restartable, then what can happen is the library is waiting in + * a wait syscall, the child exits, C<SIGCHLD> is sent to the process, + * and the wait syscall returns C<EINTR>. Since the library cannot + * control the signal handler, we have to instead restart the wait + * syscall, which is the purpose of this wrapper. + */ +int +guestfs_int_waitpid (guestfs_h *g, pid_t pid, int *status, const char *errmsg) +{ + again: + if (waitpid (pid, status, 0) == -1) { + if (errno == EINTR) + goto again; + perrorf (g, "%s: waitpid", errmsg); + return -1; + } + return 0; +} + +/** + * Like C<guestfs_int_waitpid>, but ignore errors. + */ +void +guestfs_int_waitpid_noerror (pid_t pid) +{ + while (waitpid (pid, NULL, 0) == -1 && errno == EINTR) + ; +} -- 2.7.4