On 04/14/2016 11:22 AM, Daniel P. Berrange wrote: > Currently the shell must be looked up from the config setting in > /etc/libvirt/virt-login-shell.conf. This is inflexible if there > are containers where different users need different shells. Add > add a new 'auto-shell' config parameter which instructs us to > query the containers' /etc/passwd for the shell to be exec'd. > > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx> > --- > tools/virt-login-shell.c | 33 +++++++++++++++++++++++++++++++++ > tools/virt-login-shell.conf | 14 ++++++++++++++ > 2 files changed, 47 insertions(+) > Should there be a virt-login-shell.pod change too in the CONFIG section? Hmmm.. it just dawned on me, should there be virt-login-shell.pod change for the -c addition? > diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c > index 6e8056d..7680266 100644 > --- a/tools/virt-login-shell.c > +++ b/tools/virt-login-shell.c > @@ -100,6 +100,24 @@ static int virLoginShellAllowedUser(virConfPtr conf, > return ret; > } > > +static int virLoginShellGetAutoShell(virConfPtr conf, bool *autoshell) I see virLoginShellAllowedUser goes w/ multiple lies for each argument... > +{ > + virConfValuePtr p; > + > + p = virConfGetValue(conf, "auto_shell"); > + if (!p) { > + autoshell = false; *autoshell > + } else if (p->type == VIR_CONF_LONG || > + p->type == VIR_CONF_ULONG) { > + *autoshell = (p->l != 0); > + } else { > + virReportSystemError(EINVAL, "%s", > + _("auto-shell must be a boolean value")); s/-/_/ > + return -1; > + } > + return 0; > +} > + > static int virLoginShellGetShellArgv(virConfPtr conf, > char ***retshargv, > size_t *retshargvlen) > @@ -221,6 +239,7 @@ main(int argc, char **argv) > char *tmp; > char *term = NULL; > virErrorPtr saved_err = NULL; > + bool autoshell = false; > > struct option opt[] = { > {"help", no_argument, NULL, 'h'}, > @@ -300,6 +319,8 @@ main(int argc, char **argv) > > if (virLoginShellGetShellArgv(conf, &shargv, &shargvlen) < 0) > goto cleanup; Extra line between to follow existing... > + if (virLoginShellGetAutoShell(conf, &autoshell) < 0) > + goto cleanup; > > conn = virConnectOpen("lxc:///"); > if (!conn) > @@ -351,6 +372,18 @@ main(int argc, char **argv) > goto cleanup; > } > > + if (autoshell) { > + tmp = virGetUserShell(uid); > + if (tmp) { > + virStringFreeList(shargv); > + shargvlen = 1; > + if (VIR_ALLOC_N(shargv[0], shargvlen + 1) < 0) tmp will be leaked. It's 'overloaded' with the subsequent strrchr ACK - with the obvious adjustments... John > + goto cleanup; > + shargv[0] = tmp; > + shargv[1] = NULL; > + } > + } > + > if (cmdstr) { > if (VIR_REALLOC_N(shargv, shargvlen + 3) < 0) > goto cleanup; > diff --git a/tools/virt-login-shell.conf b/tools/virt-login-shell.conf > index e29d222..4a504b3 100644 > --- a/tools/virt-login-shell.conf > +++ b/tools/virt-login-shell.conf > @@ -18,6 +18,20 @@ > # Note there is no need to pass a '--login' / '-l' argument since > # virt-login-shell will always request a login shell > > +# Normally virt-login-shell will always use the shell identified > +# by the 'shell' configuration setting above. If the container > +# is running a full OS, it might be desirable to allow the choice > +# of shell to be delegated to the owner of the shell, by querying > +# the /etc/passwd file inside the container > +# > +# To allow for that, uncomment the following: > +# auto_shell = 1 > +# > +# NB, this should /not/ be used if any container is sharing the > +# host filesystem /etc, as this would cause virt-login-shell to > +# look at the host's /etc/passwd finding itself as the listed > +# shell. Hilarious recursion would then ensue. > + > # allowed_users specifies the user names of all users that are allowed to > # execute virt-login-shell. You can specify the users as a comma > # separated list of usernames or user groups. > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list