From b76dda6c153e1251de6aa935b85942bb53213470 Mon Sep 17 00:00:00 2001
Message-Id: <b76dda6c153e1251de6aa935b85942bb53213470.1377873639.git.jdenemar@redhat.com>
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Tue, 13 Aug 2013 15:20:43 +0100
Subject: [PATCH] Address missed feedback from review of virt-login-shell
For https://bugzilla.redhat.com/show_bug.cgi?id=988491
Address a number of code, style and docs issues identified
in review of virt-login-shell after it was merged.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit a396473494dd099c2a0e6e03af9a9c7406a86108)
---
tools/Makefile.am | 1 -
tools/virt-login-shell.c | 58 ++++++++++++++++++++++++++++++----------------
tools/virt-login-shell.pod | 30 ++++++++++++++++++------
3 files changed, 61 insertions(+), 28 deletions(-)
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 00c582a..d48883c 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -134,7 +134,6 @@ virt_host_validate_CFLAGS = \
$(NULL)
virt_login_shell_SOURCES = \
- virt-login-shell.conf \
virt-login-shell.c
virt_login_shell_LDFLAGS = $(COVERAGE_LDFLAGS)
diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index b27e44f..1157cd0 100644
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -41,11 +41,11 @@
#include "vircommand.h"
#define VIR_FROM_THIS VIR_FROM_NONE
-static ssize_t nfdlist = 0;
-static int *fdlist = NULL;
+static ssize_t nfdlist;
+static int *fdlist;
static const char *conf_file = SYSCONFDIR "/libvirt/virt-login-shell.conf";
-static void virLoginShellFini(virConnectPtr conn, virDomainPtr dom)
+static void virLoginShellFini(virConnectPtr conn, virDomainPtr dom)
{
size_t i;
@@ -105,7 +105,7 @@ static int virLoginShellAllowedUser(virConfPtr conf,
}
}
}
- virReportSystemError(EPERM, _("%s not listed as an allowed_users in %s"), name, conf_file);
+ virReportSystemError(EPERM, _("%s not matched against 'allowed_users' in %s"), name, conf_file);
cleanup:
VIR_FREE(gname);
return ret;
@@ -121,7 +121,7 @@ static char **virLoginShellGetShellArgv(virConfPtr conf)
if (!p)
return virStringSplit("/bin/sh -l", " ", 3);
- if (p && p->type == VIR_CONF_LIST) {
+ if (p->type == VIR_CONF_LIST) {
size_t len;
virConfValuePtr pp;
@@ -139,7 +139,6 @@ static char **virLoginShellGetShellArgv(virConfPtr conf)
if (VIR_STRDUP(shargv[i], pp->str) < 0)
goto error;
}
- shargv[len] = NULL;
}
return shargv;
error:
@@ -155,16 +154,27 @@ static char *progname;
static void
usage(void)
{
- fprintf(stdout, _("\n"
- "%s is a privileged program that allows non root users \n"
- "specified in %s to join a Linux container \n"
- "with a matching user name and launch a shell. \n"
- "\n%s [options]\n\n"
- " options:\n"
- " -h | --help this help:\n\n"), progname, conf_file, progname);
+ fprintf(stdout,
+ _("\n"
+ "Usage:\n"
+ " %s [options]\n\n"
+ "Options:\n"
+ " -h | --help Display program help:\n"
+ " -V | --version Display program version:\n"
+ "\n"
+ "libvirt login shell\n"),
+ progname);
return;
}
+/* Display version information. */
+static void
+show_version(void)
+{
+ printf("%s (%s) %s\n", progname, PACKAGE_NAME, PACKAGE_VERSION);
+}
+
+
int
main(int argc, char **argv)
{
@@ -190,6 +200,7 @@ main(int argc, char **argv)
struct option opt[] = {
{"help", no_argument, NULL, 'h'},
+ {"version", optional_argument, NULL, 'V'},
{NULL, 0, NULL, 0}
};
if (virInitialize() < 0) {
@@ -214,20 +225,25 @@ main(int argc, char **argv)
return ret;
}
- /* The only option we support is help
- */
- while ((arg = getopt_long(argc, argv, "h", opt, &longindex)) != -1) {
+ while ((arg = getopt_long(argc, argv, "hV", opt, &longindex)) != -1) {
switch (arg) {
case 'h':
usage();
exit(EXIT_SUCCESS);
- break;
+
+ case 'V':
+ show_version();
+ exit(EXIT_SUCCESS);
+
+ case '?':
+ default:
+ usage();
+ exit(EXIT_FAILURE);
}
}
if (argc > optind) {
virReportSystemError(EINVAL, _("%s takes no options"), progname);
- errno = EINVAL;
goto cleanup;
}
@@ -268,7 +284,9 @@ main(int argc, char **argv)
virErrorPtr last_error;
last_error = virGetLastError();
if (last_error->code != VIR_ERR_OPERATION_INVALID) {
- virReportSystemError(last_error->code,_("Can't create %s container: %s"), name, virGetLastErrorMessage());
+ virReportSystemError(last_error->code,
+ _("Can't create %s container: %s"),
+ name, last_error->message);
goto cleanup;
}
}
@@ -327,7 +345,7 @@ main(int argc, char **argv)
}
if (execv(shargv[0], (char *const*) shargv) < 0) {
virReportSystemError(errno, _("Unable exec shell %s"), shargv[0]);
- return -errno;
+ return EXIT_FAILURE;
}
}
return virProcessWait(ccpid, &status2);
diff --git a/tools/virt-login-shell.pod b/tools/virt-login-shell.pod
index 0cd35cf..e27d500 100644
--- a/tools/virt-login-shell.pod
+++ b/tools/virt-login-shell.pod
@@ -8,26 +8,42 @@ B<virt-login-shell>
=head1 DESCRIPTION
-The B<virt-login-shell> program is setuid shell that is used to join
-an LXC container that matches the users name. If the container is not
-running virt-login-shell will attempt to start the container.
+The B<virt-login-shell> program is a setuid shell that is used to join
+an LXC container that matches the user's name. If the container is not
+running, virt-login-shell will attempt to start the container.
virt-sandbox-shell is not allowed to be run by root. Normal users will get
-added to a container that matches their username, if it exists. And they are
+added to a container that matches their username, if it exists, and they are
configured in /etc/libvirt/virt-login-shell.conf.
The basic structure of most virt-login-shell usage is:
virt-login-shell
+=head1 OPTIONS
+
+=over
+
+=item B<-h, --help>
+
+Display command line help usage then exit.
+
+=item B<-V, --version>
+
+Display version information then exit.
+
+=back
+
=head1 CONFIG
By default, virt-login-shell will execute the /bin/sh program for the user.
-You can modify this behaviour by defining the shell variable in /etc/libvirt/virt-login-shell.conf.
+You can modify this behaviour by defining the shell variable in
+/etc/libvirt/virt-login-shell.conf.
eg. shell = [ "/bin/ksh", "--login"]
-By default no users are allowed to user virt-login-shell, if you want to allow
-certain users to use virt-login-shell, you need to modify the allowed_users variable in /etc/libvirt/virt-login-shell.conf.
+By default no users are allowed to use virt-login-shell, if you want to allow
+certain users to use virt-login-shell, you need to modify the allowed_users
+variable in /etc/libvirt/virt-login-shell.conf.
eg. allowed_users = [ "tom", "dick", "harry" ]
--
1.8.3.2