QMP Capability probing will fail if QEMU cannot bind to the QMP monitor socket in the qemu_driver->libDir directory. That's because the child process is stripped of all capabilities and this directory is chown'ed to the configured QEMU user/group (normally qemu:qemu) by the QEMU driver. To prevent this from happening, the driver startup will now pass the QEMU uid and gid down to the capability probing code. All capability probing invocations of QEMU will be run with the configured QEMU uid instead of libvirtd's. Furter, the pid file handling is moved to libvirt, as QEMU cannot write to the qemu_driver->runDir (root:root). This also means that the libvirt daemonizing must be used. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com&gt; --- src/qemu/qemu_capabilities.c | 88 ++++++++++++++++++++++++++++++++++---------- src/qemu/qemu_capabilities.h | 7 +++- src/qemu/qemu_driver.c | 4 +- 3 files changed, 76 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6ce2638..a76d108 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -223,6 +223,8 @@ struct _qemuCapsCache { virHashTablePtr binaries; char *libDir; char *runDir; + uid_t runUid; + gid_t runGid; }; @@ -241,9 +243,37 @@ static int qemuCapsOnceInit(void) VIR_ONCE_GLOBAL_INIT(qemuCaps) +struct _qemuCapsHookData { + uid_t runUid; + gid_t runGid; +}; +typedef struct _qemuCapsHookData qemuCapsHookData; +typedef qemuCapsHookData *qemuCapsHookDataPtr; + +static int qemuCapsHook(void * data) +{ + int ret; + qemuCapsHookDataPtr hookData = data; + + if (!hookData) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU uid:gid not specified by caller")); + ret = -1; + goto cleanup; + } + + VIR_DEBUG("Switch QEMU uid:gid to %d:%d", + hookData->runUid, hookData->runGid); + ret = virSetUIDGID(hookData->runUid, hookData->runGid); + +cleanup: + return ret; +} + static virCommandPtr qemuCapsProbeCommand(const char *qemu, - qemuCapsPtr caps) + qemuCapsPtr caps, + qemuCapsHookDataPtr hookData) { virCommandPtr cmd = virCommandNew(qemu); @@ -256,6 +286,7 @@ qemuCapsProbeCommand(const char *qemu, virCommandAddEnvPassCommon(cmd); virCommandClearCaps(cmd); + virCommandSetPreExecHook(cmd, qemuCapsHook, hookData); return cmd; } @@ -342,7 +373,7 @@ no_memory: } static int -qemuCapsProbeMachineTypes(qemuCapsPtr caps) +qemuCapsProbeMachineTypes(qemuCapsPtr caps, qemuCapsHookDataPtr hookData) { char *output; int ret = -1; @@ -359,7 +390,7 @@ qemuCapsProbeMachineTypes(qemuCapsPtr caps) return -1; } - cmd = qemuCapsProbeCommand(caps->binary, caps); + cmd = qemuCapsProbeCommand(caps->binary, caps, hookData); virCommandAddArgList(cmd, "-M", "?", NULL); virCommandSetOutputBuffer(cmd, &output); @@ -498,7 +529,7 @@ cleanup: } static int -qemuCapsProbeCPUModels(qemuCapsPtr caps) +qemuCapsProbeCPUModels(qemuCapsPtr caps, qemuCapsHookDataPtr hookData) { char *output = NULL; int ret = -1; @@ -516,7 +547,7 @@ qemuCapsProbeCPUModels(qemuCapsPtr caps) return 0; } - cmd = qemuCapsProbeCommand(caps->binary, caps); + cmd = qemuCapsProbeCommand(caps->binary, caps, hookData); virCommandAddArgList(cmd, "-cpu", "?", NULL); virCommandSetOutputBuffer(cmd, &output); @@ -1530,7 +1561,8 @@ qemuCapsParseDeviceStr(qemuCapsPtr caps, const char *str) static int qemuCapsExtractDeviceStr(const char *qemu, - qemuCapsPtr caps) + qemuCapsPtr caps, + qemuCapsHookDataPtr hookData) { char *output = NULL; virCommandPtr cmd; @@ -1544,7 +1576,7 @@ qemuCapsExtractDeviceStr(const char *qemu, * understand '-device name,?', and always exits with status 1 for * the simpler '-device ?', so this function is really only useful * if -help includes "device driver,?". */ - cmd = qemuCapsProbeCommand(qemu, caps); + cmd = qemuCapsProbeCommand(qemu, caps, hookData); virCommandAddArgList(cmd, "-device", "?", "-device", "pci-assign,?", @@ -2094,7 +2126,7 @@ int qemuCapsProbeQMP(qemuCapsPtr caps, #define QEMU_SYSTEM_PREFIX "qemu-system-" static int -qemuCapsInitHelp(qemuCapsPtr caps) +qemuCapsInitHelp(qemuCapsPtr caps, uid_t runUid, gid_t runGid) { virCommandPtr cmd = NULL; unsigned int is_kvm; @@ -2102,6 +2134,7 @@ qemuCapsInitHelp(qemuCapsPtr caps) int ret = -1; const char *tmp; struct utsname ut; + qemuCapsHookData hookData; VIR_DEBUG("caps=%p", caps); @@ -2123,7 +2156,9 @@ qemuCapsInitHelp(qemuCapsPtr caps) goto cleanup; } - cmd = qemuCapsProbeCommand(caps->binary, NULL); + hookData.runUid = runUid; + hookData.runGid = runGid; + cmd = qemuCapsProbeCommand(caps->binary, NULL, &hookData); virCommandAddArgList(cmd, "-help", NULL); virCommandSetOutputBuffer(cmd, &help); @@ -2153,13 +2188,13 @@ qemuCapsInitHelp(qemuCapsPtr caps) * understands the 0.13.0+ notion of "-device driver,". */ if (qemuCapsGet(caps, QEMU_CAPS_DEVICE) && strstr(help, "-device driver,?") && - qemuCapsExtractDeviceStr(caps->binary, caps) < 0) + qemuCapsExtractDeviceStr(caps->binary, caps, &hookData) < 0) goto cleanup; - if (qemuCapsProbeCPUModels(caps) < 0) + if (qemuCapsProbeCPUModels(caps, &hookData) < 0) goto cleanup; - if (qemuCapsProbeMachineTypes(caps) < 0) + if (qemuCapsProbeMachineTypes(caps, &hookData) < 0) goto cleanup; ret = 0; @@ -2242,7 +2277,9 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps) static int qemuCapsInitQMP(qemuCapsPtr caps, const char *libDir, - const char *runDir) + const char *runDir, + uid_t runUid, + gid_t runGid) { int ret = -1; virCommandPtr cmd = NULL; @@ -2254,6 +2291,7 @@ qemuCapsInitQMP(qemuCapsPtr caps, char *monarg = NULL; char *monpath = NULL; char *pidfile = NULL; + qemuCapsHookData hookData; /* the ".sock" sufix is important to avoid a possible clash with a qemu * domain called "capabilities" @@ -2289,11 +2327,14 @@ qemuCapsInitQMP(qemuCapsPtr caps, "-nographic", "-M", "none", "-qmp", monarg, - "-pidfile", pidfile, - "-daemonize", NULL); virCommandAddEnvPassCommon(cmd); virCommandClearCaps(cmd); + hookData.runUid = runUid; + hookData.runGid = runGid; + virCommandSetPreExecHook(cmd, qemuCapsHook, &hookData); + virCommandSetPidFile(cmd, pidfile); + virCommandDaemonize(cmd); if (virCommandRun(cmd, &status) < 0) goto cleanup; @@ -2410,7 +2451,9 @@ cleanup: qemuCapsPtr qemuCapsNewForBinary(const char *binary, const char *libDir, - const char *runDir) + const char *runDir, + uid_t runUid, + gid_t runGid) { qemuCapsPtr caps = qemuCapsNew(); struct stat sb; @@ -2438,11 +2481,11 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary, goto error; } - if ((rv = qemuCapsInitQMP(caps, libDir, runDir)) < 0) + if ((rv = qemuCapsInitQMP(caps, libDir, runDir, runUid, runGid)) < 0) goto error; if (!caps->usedQMP && - qemuCapsInitHelp(caps) < 0) + qemuCapsInitHelp(caps, runUid, runGid) < 0) goto error; return caps; @@ -2478,7 +2521,8 @@ qemuCapsHashDataFree(void *payload, const void *key ATTRIBUTE_UNUSED) qemuCapsCachePtr -qemuCapsCacheNew(const char *libDir, const char *runDir) +qemuCapsCacheNew(const char *libDir, const char *runDir, + uid_t runUid, gid_t runGid) { qemuCapsCachePtr cache; @@ -2502,6 +2546,9 @@ qemuCapsCacheNew(const char *libDir, const char *runDir) goto error; } + cache->runUid = runUid; + cache->runGid = runGid; + return cache; error: @@ -2526,7 +2573,8 @@ qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary) if (!ret) { VIR_DEBUG("Creating capabilities for %s", binary); - ret = qemuCapsNewForBinary(binary, cache->libDir, cache->runDir); + ret = qemuCapsNewForBinary(binary, cache->libDir, cache->runDir, + cache->runUid, cache->runGid); if (ret) { VIR_DEBUG("Caching capabilities %p for %s", ret, binary); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 751b3ec..f420c43 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -169,7 +169,9 @@ qemuCapsPtr qemuCapsNew(void); qemuCapsPtr qemuCapsNewCopy(qemuCapsPtr caps); qemuCapsPtr qemuCapsNewForBinary(const char *binary, const char *libDir, - const char *runDir); + const char *runDir, + uid_t runUid, + gid_t runGid); int qemuCapsProbeQMP(qemuCapsPtr caps, qemuMonitorPtr mon); @@ -207,7 +209,8 @@ int qemuCapsGetMachineTypesCaps(qemuCapsPtr caps, bool qemuCapsIsValid(qemuCapsPtr caps); -qemuCapsCachePtr qemuCapsCacheNew(const char *libDir, const char *runDir); +qemuCapsCachePtr qemuCapsCacheNew(const char *libDir, const char *runDir, + uid_t uid, gid_t gid); qemuCapsPtr qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary); qemuCapsPtr qemuCapsCacheLookupCopy(qemuCapsCachePtr cache, const char *binary); void qemuCapsCacheFree(qemuCapsCachePtr cache); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d4cafcc..7e97110 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -787,7 +787,9 @@ qemudStartup(int privileged) { goto error; qemu_driver->capsCache = qemuCapsCacheNew(qemu_driver->libDir, - qemu_driver->stateDir); + qemu_driver->stateDir, + qemu_driver->user, + qemu_driver->group); if (!qemu_driver->capsCache) goto error; -- 1.7.12.4

With QMP capability probing, the version was not set. virsh version returns: ... Cannot extract running QEMU hypervisor version This is fixed by computing caps->version from QMP major, minor, micro values. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com&gt; --- src/qemu/qemu_capabilities.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a76d108..d92947f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2377,6 +2377,7 @@ qemuCapsInitQMP(qemuCapsPtr caps, goto cleanup; } + caps->version = major * 1000000 + minor * 1000 + micro; caps->usedQMP = true; qemuCapsInitQMPBasic(caps); -- 1.7.12.4