virFork: give specific status on failure prior to exec

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: NEric Blake <eblake@redhat.com>

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__ */

src/util/vircommand.c

 /*
  * 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

tests/commandtest.c

 /*
  * 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) {