virCommand is a version of virExec that doesn't fork, however it is just calling execve and doesn't honors setting uid/gid and pwd. This commit moves those pieces from virExec to virCommandExec and makes virExec use virCommandExec to avoid code duplication. --- src/util/vircommand.c | 92 ++++++++++++++++++++++++--------------------------- 1 file changed, 43 insertions(+), 49 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index e1bbc0526..aa97a5a10 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -481,21 +481,18 @@ virExec(virCommandPtr cmd) int childerr = -1; int tmpfd; char *binarystr = NULL; - const char *binary = NULL; int ret; struct sigaction waxon, waxoff; - gid_t *groups = NULL; - int ngroups; if (cmd->args[0][0] != '/') { - if (!(binary = binarystr = virFindFileInPath(cmd->args[0]))) { + if (!(binarystr = virFindFileInPath(cmd->args[0]))) { virReportSystemError(ENOENT, _("Cannot find '%s' in path"), cmd->args[0]); return -1; } - } else { - binary = cmd->args[0]; + VIR_FREE(cmd->args[0]); + cmd->args[0] = binarystr; } if (childin < 0) { @@ -556,9 +553,6 @@ virExec(virCommandPtr cmd) childerr = null; } - if ((ngroups = virGetGroupList(cmd->uid, cmd->gid, &groups)) < 0) - goto cleanup; - pid = virFork(); if (pid < 0) @@ -577,9 +571,6 @@ virExec(virCommandPtr cmd) cmd->pid = pid; - VIR_FREE(binarystr); - VIR_FREE(groups); - return 0; } @@ -727,29 +718,6 @@ virExec(virCommandPtr cmd) } # endif - /* The steps above may need to do something privileged, so we delay - * setuid and clearing capabilities until the last minute. - */ - if (cmd->uid != (uid_t)-1 || cmd->gid != (gid_t)-1 || - cmd->capabilities || (cmd->flags & VIR_EXEC_CLEAR_CAPS)) { - VIR_DEBUG("Setting child uid:gid to %d:%d with caps %llx", - (int)cmd->uid, (int)cmd->gid, cmd->capabilities); - if (virSetUIDGIDWithCaps(cmd->uid, cmd->gid, groups, ngroups, - cmd->capabilities, - !!(cmd->flags & VIR_EXEC_CLEAR_CAPS)) < 0) { - goto fork_error; - } - } - - if (cmd->pwd) { - VIR_DEBUG("Running child in %s", cmd->pwd); - if (chdir(cmd->pwd) < 0) { - virReportSystemError(errno, - _("Unable to change to %s"), cmd->pwd); - goto fork_error; - } - } - if (virCommandHandshakeChild(cmd) < 0) goto fork_error; @@ -771,15 +739,10 @@ virExec(virCommandPtr cmd) /* Close logging again to ensure no FDs leak to child */ virLogReset(); - if (cmd->env) - execve(binary, cmd->args, cmd->env); - else - execv(binary, cmd->args); + if (virCommandExec(cmd) == -2) + goto fork_error; ret = errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE; - virReportSystemError(errno, - _("cannot execute binary %s"), - cmd->args[0]); fork_error: virDispatchError(NULL); @@ -789,9 +752,6 @@ virExec(virCommandPtr cmd) /* This is cleanup of parent process only - child should never jump here on error */ - VIR_FREE(groups); - VIR_FREE(binarystr); - /* NB we don't virReportError() on any failures here because the code which jumped here already raised an error condition which we must not overwrite */ @@ -2150,23 +2110,57 @@ virCommandProcessIO(virCommandPtr cmd) * in the hook after already forking / cloning, so does not attempt to * daemonize or preserve any FDs. * - * Returns -1 on any error executing the command. + * Returns -1 on any error executing the command, -2 if the error happen + * before running the command. + * * Will not return on success. */ #ifndef WIN32 int virCommandExec(virCommandPtr cmd) { + gid_t *groups = NULL; + int ngroups; + if (!cmd ||cmd->has_error == ENOMEM) { virReportOOMError(); - return -1; + return -2; } if (cmd->has_error) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("invalid use of command API")); - return -1; + return -2; } - execve(cmd->args[0], cmd->args, cmd->env); + if ((ngroups = virGetGroupList(cmd->uid, cmd->gid, &groups)) < 0) { + VIR_FREE(groups); + return -2; + } + + if (cmd->uid != (uid_t)-1 || cmd->gid != (gid_t)-1 || + cmd->capabilities || (cmd->flags & VIR_EXEC_CLEAR_CAPS)) { + VIR_DEBUG("Setting child uid:gid to %d:%d with caps %llx", + (int)cmd->uid, (int)cmd->gid, cmd->capabilities); + if (virSetUIDGIDWithCaps(cmd->uid, cmd->gid, groups, ngroups, + cmd->capabilities, + !!(cmd->flags & VIR_EXEC_CLEAR_CAPS)) < 0) { + return -2; + } + } + VIR_FREE(groups); + + if (cmd->pwd) { + VIR_DEBUG("Running child in %s", cmd->pwd); + if (chdir(cmd->pwd) < 0) { + virReportSystemError(errno, + _("Unable to change to %s"), cmd->pwd); + return -2; + } + } + + if (cmd->env) + execve(cmd->args[0], cmd->args, cmd->env); + else + execv(cmd->args[0], cmd->args); virReportSystemError(errno, _("cannot execute binary %s"), -- 2.12.2

Some containers may want the application to run in a special directory. Add <initdir> element in the domain configuration to handle this case and use it in the lxc driver. --- docs/formatdomain.html.in | 5 +++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 5 +++++ src/conf/domain_conf.h | 1 + src/lxc/lxc_container.c | 4 +++- tests/lxcxml2xmldata/lxc-initdir.xml | 30 ++++++++++++++++++++++++++++++ tests/lxcxml2xmltest.c | 1 + 7 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 tests/lxcxml2xmldata/lxc-initdir.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8da50875b..7627fd0d0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -330,6 +330,10 @@ To set environment variables, use the <code>initenv</code> element, one for each variable. </p> + <p> + To set a custom work directory for the init, use the <code>initdir</code> + element. + </p> <pre> &lt;os&gt; @@ -338,6 +342,7 @@ &lt;initarg&gt;--unit&lt;/initarg&gt; &lt;initarg&gt;emergency.service&lt;/initarg&gt; &lt;initenv name='MYENV'&gt;some value&lt;/initenv&gt; + &lt;initdir&gt;/my/custom/cwd&lt;/initdir&gt; &lt;/os&gt; </pre> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 695214816..5a4c4ecf1 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -395,6 +395,11 @@ <text/> </element> </zeroOrMore> + <optional> + <element name="initdir"> + <ref name="absFilePath"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 89c803047..bf530fc52 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2801,6 +2801,7 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->os.initargv); for (i = 0; def->os.initenv && def->os.initenv[i]; i++) VIR_FREE(def->os.initenv[i]); + VIR_FREE(def->os.initdir); VIR_FREE(def->os.initenv); VIR_FREE(def->os.kernel); VIR_FREE(def->os.initrd); @@ -16796,6 +16797,7 @@ virDomainDefParseBootOptions(virDomainDefPtr def, if (def->os.type == VIR_DOMAIN_OSTYPE_EXE) { def->os.init = virXPathString("string(./os/init[1])", ctxt); def->os.cmdline = virXPathString("string(./os/cmdline[1])", ctxt); + def->os.initdir = virXPathString("string(./os/initdir[1])", ctxt); if ((n = virXPathNodeSet("./os/initarg", ctxt, &nodes)) < 0) goto error; @@ -24562,6 +24564,9 @@ virDomainDefFormatInternal(virDomainDefPtr def, for (i = 0; def->os.initenv && def->os.initenv[i]; i++) virBufferAsprintf(buf, "<initenv name='%s'>%s</initenv>\n", def->os.initenv[i]->name, def->os.initenv[i]->value); + if (def->os.initdir) + virBufferEscapeString(buf, "<initdir>%s</initdir>\n", + def->os.initdir); if (def->os.loader) virDomainLoaderDefFormat(buf, def->os.loader); virBufferEscapeString(buf, "<kernel>%s</kernel>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 03153b972..105f0b7a6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1841,6 +1841,7 @@ struct _virDomainOSDef { char *init; char **initargv; virDomainOSEnvPtr *initenv; + char *initdir; char *kernel; char *initrd; char *cmdline; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index ffafc39d7..c122a588e 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -237,7 +237,7 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef, virCommandAddEnvString(cmd, "PATH=/bin:/sbin"); virCommandAddEnvString(cmd, "TERM=linux"); virCommandAddEnvString(cmd, "container=lxc-libvirt"); - virCommandAddEnvString(cmd, "HOME=/"); +/* virCommandAddEnvString(cmd, "HOME=/"); */ virCommandAddEnvPair(cmd, "container_uuid", uuidstr); if (nttyPaths > 1) virCommandAddEnvPair(cmd, "container_ttys", virBufferCurrentContent(&buf)); @@ -245,6 +245,8 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef, virCommandAddEnvPair(cmd, "LIBVIRT_LXC_NAME", vmDef->name); if (vmDef->os.cmdline) virCommandAddEnvPair(cmd, "LIBVIRT_LXC_CMDLINE", vmDef->os.cmdline); + if (vmDef->os.initdir) + virCommandSetWorkingDirectory(cmd, vmDef->os.initdir); for (i = 0; vmDef->os.initenv[i]; i++) { virCommandAddEnvPair(cmd, vmDef->os.initenv[i]->name, diff --git a/tests/lxcxml2xmldata/lxc-initdir.xml b/tests/lxcxml2xmldata/lxc-initdir.xml new file mode 100644 index 000000000..2940bda91 --- /dev/null +++ b/tests/lxcxml2xmldata/lxc-initdir.xml @@ -0,0 +1,30 @@ +<domain type='lxc'> + <name>jessie</name> + <uuid>e21987a5-e98e-9c99-0e35-803e4d9ad1fe</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <resource> + <partition>/machine</partition> + </resource> + <os> + <type arch='x86_64'>exe</type> + <init>/sbin/sh</init> + <initdir>/path/to/pwd</initdir> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/libexec/libvirt_lxc</emulator> + <filesystem type='mount' accessmode='passthrough'> + <source dir='/mach/jessie'/> + <target dir='/'/> + </filesystem> + <console type='pty'> + <target type='lxc' port='0'/> + </console> + </devices> + <seclabel type='none'/> +</domain> diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c index 2a24b60b3..c81b0eace 100644 --- a/tests/lxcxml2xmltest.c +++ b/tests/lxcxml2xmltest.c @@ -99,6 +99,7 @@ mymain(void) DO_TEST_FULL("filesystem-root", 0, false, VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS); DO_TEST("initenv"); + DO_TEST("initdir"); virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.12.2

On Mon, 2017-06-05 at 15:27 +0100, Daniel P. Berrange wrote: Will be needed once we'll have docker json support in native-o-xml. lxc config file doesn't have it AFAICT. Oops, looks like I forgot some cleanup Hum... weird. I'll recheck my source tree here and resubmit properly -- Cedric