From 730fc9622b295b1dc37f620fe8a5c7391ee9a47b Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 23 Dec 2013 21:07:01 -0700
Subject: [PATCH] virt-login-shell: saner exit value

virt-login-shell was exiting with status 0, regardless of what the
wrapped shell returned.  This is unkind to users; we should behave
more like env(1), nice(1), su(1), and other wrapper programs, by
preserving the invoked application's status (which includes the
distinction between death due to signal vs. normal death).

* tools/virt-login-shell.c (main): Pass through child exit status.
* tools/virt-login-shell.pod: Document exit status.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tools/virt-login-shell.c   | 35 +++++++++++++++++++----------------
 tools/virt-login-shell.pod | 25 +++++++++++++++++++++++--
 2 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index 75a5223d10..3ea7ade4be 100644
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -21,13 +21,14 @@
  */
 #include <config.h>
 
-#include <stdarg.h>
-#include <getopt.h>
-#include <stdio.h>
 #include <errno.h>
-#include <stdlib.h>
 #include <fnmatch.h>
+#include <getopt.h>
 #include <locale.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
 
 #include "internal.h"
 #include "virerror.h"
@@ -168,8 +169,8 @@ main(int argc, char **argv)
 {
     virConfPtr conf = NULL;
     const char *login_shell_path = conf_file;
-    pid_t cpid;
-    int ret = EXIT_FAILURE;
+    pid_t cpid = -1;
+    int ret = EXIT_CANCELED;
     int status;
     uid_t uid = getuid();
     gid_t gid = getgid();
@@ -195,8 +196,8 @@ main(int argc, char **argv)
         {NULL, 0, NULL, 0}
     };
     if (virInitialize() < 0) {
-        fprintf(stderr, _("Failed to initialize libvirt Error Handling"));
-        return EXIT_FAILURE;
+        fprintf(stderr, _("Failed to initialize libvirt error handling"));
+        return EXIT_CANCELED;
     }
 
     setenv("PATH", "/bin:/usr/bin", 1);
@@ -231,7 +232,7 @@ main(int argc, char **argv)
         case '?':
         default:
             usage();
-            exit(EXIT_FAILURE);
+            exit(EXIT_CANCELED);
         }
     }
 
@@ -330,15 +331,13 @@ main(int argc, char **argv)
         if (execv(shargv[0], (char *const*) shargv) < 0) {
             virReportSystemError(errno, _("Unable to exec shell %s"),
                                  shargv[0]);
-            return EXIT_FAILURE;
+            virDispatchError(NULL);
+            return errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE;
         }
-        return EXIT_SUCCESS;
     }
 
-    if (virProcessWait(cpid, &status, true) < 0)
-        goto cleanup;
-    ret = EXIT_SUCCESS;
-
+    /* At this point, the parent is now waiting for the child to exit,
+     * but as that may take a long time, we release resources now.  */
 cleanup:
     for (i = 0; i < nfdlist; i++)
         VIR_FORCE_CLOSE(fdlist[i]);
@@ -354,7 +353,11 @@ cleanup:
     VIR_FREE(seclabel);
     VIR_FREE(secmodel);
     VIR_FREE(groups);
-    if (ret)
+
+    if (virProcessWait(cpid, &status, true) == 0)
+        virProcessExitWithStatus(status);
+
+    if (virGetLastError())
         virDispatchError(NULL);
     return ret;
 }
diff --git a/tools/virt-login-shell.pod b/tools/virt-login-shell.pod
index bcd7855593..56861f7138 100644
--- a/tools/virt-login-shell.pod
+++ b/tools/virt-login-shell.pod
@@ -4,7 +4,7 @@ virt-login-shell - tool to execute a shell within a container matching the users
 
 =head1 SYNOPSIS
 
-B<virt-login-shell>
+B<virt-login-shell> [I<OPTION>]
 
 =head1 DESCRIPTION
 
@@ -47,6 +47,27 @@ variable in /etc/libvirt/virt-login-shell.conf.
 
 eg. allowed_users = [ "tom", "dick", "harry" ]
 
+=head1 EXIT STATUS
+
+B<virt-login-shell> normally returns the exit status of the command it
+executed. If the command was killed by a signal, but that signal is not
+fatal to virt-login-shell, then it returns the signal number plus 128.
+
+Exit status generated by B<virt-login-shell> itself:
+
+=over 4
+
+=item B<0> An option was used to learn more about this binary.
+
+=item B<125> Generic error before attempting execution of the configured
+shell; for example, if libvirtd is not running.
+
+=item B<126> The configured shell exists but could not be executed.
+
+=item B<127> The configured shell could not be found.
+
+=back
+
 =head1 BUGS
 
 Report any bugs discovered to the libvirt community via the mailing
@@ -61,7 +82,7 @@ Alternatively report bugs to your software distributor / vendor.
 
 =head1 COPYRIGHT
 
-Copyright (C) 2013 Red Hat, Inc., and the authors listed in the
+Copyright (C) 2013-2014 Red Hat, Inc., and the authors listed in the
 libvirt AUTHORS file.
 
 =head1 LICENSE
-- 
GitLab