[Libguestfs] [PATCH] Add safe wrapper around waitpid which deals with EINTR correctly.
Richard W.M. Jones
rjones at redhat.com
Thu Apr 14 17:26:49 UTC 2016
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
More information about the Libguestfs
mailing list