[libvirt] [PATCH 2/3] qemu: Fix QMP Capabability Probing Failure
Viktor Mihajlovski
mihajlov at linux.vnet.ibm.com
Mon Nov 26 14:17:13 UTC 2012
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 at linux.vnet.ibm.com>
---
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
More information about the libvir-list
mailing list