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).
* 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.
* 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@xxxxxxxxxx>
---
src/internal.h | 7 +++++++
src/util/vircommand.c | 5 +++--
tools/virt-login-shell.c | 44 ++++++++++++++++++++++++++++----------------
tools/virt-login-shell.pod | 23 ++++++++++++++++++++++-
4 files changed, 60 insertions(+), 19 deletions(-)
diff --git a/src/internal.h b/src/internal.h
index 5e29694..c90c83f 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -431,5 +431,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 ad767d7..3bbbe5f 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -203,7 +203,8 @@ virCommandFDSet(virCommandPtr cmd,
* 2. This is the parent: the return is > 0. The parent can now attempt
* to interact with the child (but be aware that unlike raw fork(), the
* child may not return - some failures in the child result in this
- * function calling _exit(255) if the child cannot be set up correctly).
+ * function calling _exit(EXIT_CANCELED) if the child cannot be set up
+ * correctly).
* 3. This is the child: the return is 0. If this happens, the parent
* is also guaranteed to return.
*/
@@ -292,7 +293,7 @@ virFork(void)
if (pthread_sigmask(SIG_SETMASK, &newmask, NULL) != 0) {
virReportSystemError(errno, "%s", _("cannot unblock signals"));
virDispatchError(NULL);
- _exit(255);
+ _exit(EXIT_CANCELED);
}
}
return pid;
diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index bc8394e..2172325 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) < 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,20 @@ cleanup:
VIR_FREE(seclabel);
VIR_FREE(secmodel);
VIR_FREE(groups);
- if (ret)
+
+ if (virProcessWait(cpid, &status) == 0) {
+ if (WIFEXITED(status)) {
+ ret = WEXITSTATUS(status);
+ } else if (WIFSIGNALED(status)) {
+ /* Try to kill the parent with the same signal as the
+ * child, but if that fails, at least give a decent exit
+ * status value. */
+ raise(WTERMSIG(status));
+ ret = 128 + WTERMSIG(status);
+ }
+ }
+
+ if (virGetLastError())
virDispatchError(NULL);
return ret;
}
diff --git a/tools/virt-login-shell.pod b/tools/virt-login-shell.pod
index bcd7855..c1d0cb3 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
--
1.8.4.2
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list