If probing capabilities via QMP fails, we now have a check that prevents us falling back to -help parsing. Unfortunately the error message "Failed to probe capabilities for /usr/bin/qemu-kvm: unsupported configuration: QEMU 2.1.2 is too new for help parsing" is proving rather unhelpful to the user. We need to be telling them why QMP failed (the root cause), rather than they can't use -help (the side effect). To do this we should capture stderr during QMP probing, and if -help parsing then sees a new QEMU version, we know that QMP should have worked, and so we can show the messages from stderr. The message thus becomes "Failed to probe capabilities for /usr/bin/qemu-kvm: internal error: QEMU / QMP failed: Could not access KVM kernel module: No such file or directory failed to initialize KVM: No such file or directory" --- src/qemu/qemu_capabilities.c | 37 +++++++++++++++++++++++++++---------- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_process.c | 4 ++-- tests/qemuhelptest.c | 2 +- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d2c046d..f777484 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1331,7 +1331,8 @@ int virQEMUCapsParseHelpStr(const char *qemu, unsigned int *version, bool *is_kvm, unsigned int *kvm_version, - bool check_yajl) + bool check_yajl, + const char *qemuerr) { unsigned major, minor, micro; const char *p = help; @@ -1392,9 +1393,15 @@ int virQEMUCapsParseHelpStr(const char *qemu, * using QMP probing. */ if (*version >= 1002000) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("QEMU %u.%u.%u is too new for help parsing"), - major, minor, micro); + if (qemuerr && *qemuerr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QEMU / QMP failed: %s"), + qemuerr); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("QEMU %u.%u.%u is too new for help parsing"), + major, minor, micro); + } goto cleanup; } @@ -2967,7 +2974,7 @@ virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir) #define QEMU_SYSTEM_PREFIX "qemu-system-" static int -virQEMUCapsInitHelp(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid) +virQEMUCapsInitHelp(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid, const char *qemuerr) { virCommandPtr cmd = NULL; bool is_kvm; @@ -2998,7 +3005,8 @@ virQEMUCapsInitHelp(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid) &qemuCaps->version, &is_kvm, &qemuCaps->kvmVersion, - false) < 0) + false, + qemuerr) < 0) goto cleanup; /* x86_64 and i686 support PCI-multibus on all machine types @@ -3249,7 +3257,8 @@ static int virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, const char *libDir, uid_t runUid, - gid_t runGid) + gid_t runGid, + char **qemuerr) { int ret = -1; virCommandPtr cmd = NULL; @@ -3311,13 +3320,16 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, virCommandSetGID(cmd, runGid); virCommandSetUID(cmd, runUid); + virCommandSetErrorBuffer(cmd, qemuerr); + /* Log, but otherwise ignore, non-zero status. */ if (virCommandRun(cmd, &status) < 0) goto cleanup; if (status != 0) { ret = 0; - VIR_DEBUG("QEMU %s exited with status %d", qemuCaps->binary, status); + VIR_DEBUG("QEMU %s exited with status %d: %s", + qemuCaps->binary, status, *qemuerr); goto cleanup; } @@ -3366,6 +3378,8 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, VIR_ERROR(_("Failed to kill process %lld: %s"), (long long) pid, virStrerror(errno, ebuf, sizeof(ebuf))); + + VIR_FREE(*qemuerr); } if (pidfile) { unlink(pidfile); @@ -3406,6 +3420,7 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary, virQEMUCapsPtr qemuCaps; struct stat sb; int rv; + char *qemuerr = NULL; if (!(qemuCaps = virQEMUCapsNew())) goto error; @@ -3436,13 +3451,13 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary, goto error; if (rv == 0) { - if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid) < 0) { + if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid, &qemuerr) < 0) { virQEMUCapsLogProbeFailure(binary); goto error; } if (!qemuCaps->usedQMP && - virQEMUCapsInitHelp(qemuCaps, runUid, runGid) < 0) { + virQEMUCapsInitHelp(qemuCaps, runUid, runGid, qemuerr) < 0) { virQEMUCapsLogProbeFailure(binary); goto error; } @@ -3451,9 +3466,11 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary, goto error; } + VIR_FREE(qemuerr); return qemuCaps; error: + VIR_FREE(qemuerr); virObjectUnref(qemuCaps); qemuCaps = NULL; return NULL; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c5542d1..f53b8f4 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -308,7 +308,8 @@ int virQEMUCapsParseHelpStr(const char *qemu, unsigned int *version, bool *is_kvm, unsigned int *kvm_version, - bool check_yajl); + bool check_yajl, + const char *qemuerr); /* Only for use by test suite */ int virQEMUCapsParseDeviceStr(virQEMUCapsPtr qemuCaps, const char *str); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a14b6f7..68777e8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3554,12 +3554,12 @@ qemuProcessReconnect(void *opaque) struct qemuProcessReconnectData *data = opaque; virQEMUDriverPtr driver = data->driver; virDomainObjPtr obj = data->obj; - qemuDomainObjPrivatePtr priv; + qemuDomainObjPrivatePtr priv = NULL; virConnectPtr conn = data->conn; struct qemuDomainJobObj oldjob; int state; int reason; - virQEMUDriverConfigPtr cfg; + virQEMUDriverConfigPtr cfg = NULL; size_t i; int ret; diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 0c68367..6d9525c 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -59,7 +59,7 @@ static int testHelpStrParsing(const void *data) goto cleanup; if (virQEMUCapsParseHelpStr("QEMU", help, flags, - &version, &is_kvm, &kvm_version, false) == -1) { + &version, &is_kvm, &kvm_version, false, NULL) == -1) { if (info->error && virGetLastError()->code == info->error) ret = 0; goto cleanup; -- 2.1.0