When a child fails without exec'ing, we want a well-known status;
best is to match what env(1), nice(1), su(1), and other wrapper
programs do. This patch adds enum values that later patches will
use, and sets up virFork as the first client of EXIT_CANCELED
for errors detected prior to even attempting exec, as well as
virExec to distinguish between a missing executable vs. a binary
that cannot be executed.
This is a slight semantic change in the unlikely case of a child
process failing to restore its signal mask - we now kill the
child with a known status instead of relying on the caller to
notice and do an appropriate _exit(). A subsequent patch will
make further cleanups based on an audit of all callers.
* src/internal.h (EXIT_CANCELED, EXIT_CANNOT_INVOKE)
(EXIT_ENOENT): New enum.
* src/util/vircommand.c (virFork): Document specific exit value if
child aborts early.
(virExec): Distinguish between various exec failures.
* tests/commandtest.c (test1): Enhance test.
(test22): New test.
Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
src/internal.h | 7 +++++++
src/util/vircommand.c | 15 +++++++++------
tests/commandtest.c | 43 +++++++++++++++++++++++++++++++++++++++++--
3 files changed, 57 insertions(+), 8 deletions(-)
diff --git a/src/internal.h b/src/internal.h
index cef3da0..5a38448 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -438,5 +438,12 @@
#NAME ": " FMT, __VA_ARGS__);
# endif
+/* Specific error values for use in forwarding programs such as
+ * virt-login-shell; these values match what GNU env does. */
+enum {
+ EXIT_CANCELED = 125, /* Failed before attempting exec */
+ EXIT_CANNOT_INVOKE = 126, /* Exists but couldn't exec */
+ EXIT_ENOENT = 127, /* Could not find program to exec */
+};
#endif /* __VIR_INTERNAL_H__ */
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index b137436..b9e5f37 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -1,7 +1,7 @@
/*
* vircommand.c: Child command execution
*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 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
@@ -199,7 +199,7 @@ virCommandFDSet(virCommandPtr cmd,
* @pid - a pointer to a pid_t that will receive the return value from
* fork()
*
- * fork a new process while avoiding various race/deadlock conditions
+ * Wrapper around fork() that avoids various race/deadlock conditions.
*
* on return from virFork(), if *pid < 0, the fork failed and there is
* no new process. Otherwise, just like fork(), if *pid == 0, it is the
@@ -208,7 +208,7 @@ virCommandFDSet(virCommandPtr cmd,
* Even if *pid >= 0, if the return value from virFork() is < 0, it
* indicates a failure that occurred in the parent or child process
* after the fork. In this case, the child process should call
- * _exit(EXIT_FAILURE) after doing any additional error reporting.
+ * _exit(EXIT_CANCELED) after doing any additional error reporting.
*/
int
virFork(pid_t *pid)
@@ -304,7 +304,8 @@ virFork(pid_t *pid)
if (pthread_sigmask(SIG_SETMASK, &newmask, NULL) != 0) {
saved_errno = errno; /* save for caller */
virReportSystemError(errno, "%s", _("cannot unblock signals"));
- goto cleanup;
+ virDispatchError(NULL);
+ _exit(EXIT_CANCELED);
}
ret = 0;
}
@@ -518,6 +519,7 @@ virExec(virCommandPtr cmd)
/* child */
+ ret = EXIT_CANCELED;
if (forkRet < 0) {
/* The fork was successful, but after that there was an error
* in the child (which was already logged).
@@ -603,7 +605,7 @@ virExec(virCommandPtr cmd)
cmd->pidfile, pid);
goto fork_error;
}
- _exit(0);
+ _exit(EXIT_SUCCESS);
}
}
@@ -703,13 +705,14 @@ virExec(virCommandPtr cmd)
else
execv(binary, cmd->args);
+ ret = errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE;
virReportSystemError(errno,
_("cannot execute binary %s"),
cmd->args[0]);
fork_error:
virDispatchError(NULL);
- _exit(EXIT_FAILURE);
+ _exit(ret);
cleanup:
/* This is cleanup of parent process only - child
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 2ae8871..042f049 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -1,7 +1,7 @@
/*
* commandtest.c: Test the libCommand API
*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 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
@@ -140,7 +140,7 @@ static int test1(const void *unused ATTRIBUTE_UNUSED)
cmd = virCommandNew(abs_builddir "/commandhelper-doesnotexist");
if (virCommandRun(cmd, &status) < 0)
goto cleanup;
- if (status == 0)
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != EXIT_ENOENT)
goto cleanup;
ret = 0;
@@ -899,6 +899,44 @@ cleanup:
return ret;
}
+static int
+test22(const void *unused ATTRIBUTE_UNUSED)
+{
+ int ret = -1;
+ virCommandPtr cmd;
+ int status = -1;
+
+ cmd = virCommandNewArgList("/bin/sh", "-c", "exit 3", NULL);
+
+ if (virCommandRun(cmd, &status) < 0) {
+ virErrorPtr err = virGetLastError();
+ printf("Cannot run child %s\n", err->message);
+ goto cleanup;
+ }
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 3) {
+ printf("Unexpected status %d\n", status);
+ goto cleanup;
+ }
+
+ virCommandFree(cmd);
+ cmd = virCommandNewArgList("/bin/sh", "-c", "kill -9 $$", NULL);
+
+ if (virCommandRun(cmd, &status) < 0) {
+ virErrorPtr err = virGetLastError();
+ printf("Cannot run child %s\n", err->message);
+ goto cleanup;
+ }
+ if (!WIFSIGNALED(status) || WTERMSIG(status) != SIGKILL) {
+ printf("Unexpected status %d\n", status);
+ goto cleanup;
+ }
+
+ ret = 0;
+cleanup:
+ virCommandFree(cmd);
+ return ret;
+}
+
static void virCommandThreadWorker(void *opaque)
{
virCommandTestDataPtr test = opaque;
@@ -1046,6 +1084,7 @@ mymain(void)
DO_TEST(test19);
DO_TEST(test20);
DO_TEST(test21);
+ DO_TEST(test22);
virMutexLock(&test->lock);
if (test->running) {
--
1.8.5.3
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list