From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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(a)redhat.com&gt; --- 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.1