Report original error when QMP probing fails with new QEMU

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"
This commit is contained in:
Daniel P. Berrange 2014-12-04 15:26:41 +00:00
parent d1e460136a
commit 25bf888a66
3 changed files with 30 additions and 12 deletions

View File

@ -1331,7 +1331,8 @@ int virQEMUCapsParseHelpStr(const char *qemu,
unsigned int *version, unsigned int *version,
bool *is_kvm, bool *is_kvm,
unsigned int *kvm_version, unsigned int *kvm_version,
bool check_yajl) bool check_yajl,
const char *qmperr)
{ {
unsigned major, minor, micro; unsigned major, minor, micro;
const char *p = help; const char *p = help;
@ -1392,9 +1393,15 @@ int virQEMUCapsParseHelpStr(const char *qemu,
* using QMP probing. * using QMP probing.
*/ */
if (*version >= 1002000) { if (*version >= 1002000) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, if (qmperr && *qmperr) {
_("QEMU %u.%u.%u is too new for help parsing"), virReportError(VIR_ERR_INTERNAL_ERROR,
major, minor, micro); _("QEMU / QMP failed: %s"),
qmperr);
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("QEMU %u.%u.%u is too new for help parsing"),
major, minor, micro);
}
goto cleanup; goto cleanup;
} }
@ -2967,7 +2974,7 @@ virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir)
#define QEMU_SYSTEM_PREFIX "qemu-system-" #define QEMU_SYSTEM_PREFIX "qemu-system-"
static int static int
virQEMUCapsInitHelp(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid) virQEMUCapsInitHelp(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid, const char *qmperr)
{ {
virCommandPtr cmd = NULL; virCommandPtr cmd = NULL;
bool is_kvm; bool is_kvm;
@ -2998,7 +3005,8 @@ virQEMUCapsInitHelp(virQEMUCapsPtr qemuCaps, uid_t runUid, gid_t runGid)
&qemuCaps->version, &qemuCaps->version,
&is_kvm, &is_kvm,
&qemuCaps->kvmVersion, &qemuCaps->kvmVersion,
false) < 0) false,
qmperr) < 0)
goto cleanup; goto cleanup;
/* x86_64 and i686 support PCI-multibus on all machine types /* x86_64 and i686 support PCI-multibus on all machine types
@ -3249,7 +3257,8 @@ static int
virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
const char *libDir, const char *libDir,
uid_t runUid, uid_t runUid,
gid_t runGid) gid_t runGid,
char **qmperr)
{ {
int ret = -1; int ret = -1;
virCommandPtr cmd = NULL; virCommandPtr cmd = NULL;
@ -3311,13 +3320,16 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
virCommandSetGID(cmd, runGid); virCommandSetGID(cmd, runGid);
virCommandSetUID(cmd, runUid); virCommandSetUID(cmd, runUid);
virCommandSetErrorBuffer(cmd, qmperr);
/* Log, but otherwise ignore, non-zero status. */ /* Log, but otherwise ignore, non-zero status. */
if (virCommandRun(cmd, &status) < 0) if (virCommandRun(cmd, &status) < 0)
goto cleanup; goto cleanup;
if (status != 0) { if (status != 0) {
ret = 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, *qmperr);
goto cleanup; goto cleanup;
} }
@ -3366,6 +3378,8 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
VIR_ERROR(_("Failed to kill process %lld: %s"), VIR_ERROR(_("Failed to kill process %lld: %s"),
(long long) pid, (long long) pid,
virStrerror(errno, ebuf, sizeof(ebuf))); virStrerror(errno, ebuf, sizeof(ebuf)));
VIR_FREE(*qmperr);
} }
if (pidfile) { if (pidfile) {
unlink(pidfile); unlink(pidfile);
@ -3406,6 +3420,7 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary,
virQEMUCapsPtr qemuCaps; virQEMUCapsPtr qemuCaps;
struct stat sb; struct stat sb;
int rv; int rv;
char *qmperr = NULL;
if (!(qemuCaps = virQEMUCapsNew())) if (!(qemuCaps = virQEMUCapsNew()))
goto error; goto error;
@ -3436,13 +3451,13 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary,
goto error; goto error;
if (rv == 0) { if (rv == 0) {
if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid) < 0) { if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid, &qmperr) < 0) {
virQEMUCapsLogProbeFailure(binary); virQEMUCapsLogProbeFailure(binary);
goto error; goto error;
} }
if (!qemuCaps->usedQMP && if (!qemuCaps->usedQMP &&
virQEMUCapsInitHelp(qemuCaps, runUid, runGid) < 0) { virQEMUCapsInitHelp(qemuCaps, runUid, runGid, qmperr) < 0) {
virQEMUCapsLogProbeFailure(binary); virQEMUCapsLogProbeFailure(binary);
goto error; goto error;
} }
@ -3451,9 +3466,11 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary,
goto error; goto error;
} }
VIR_FREE(qmperr);
return qemuCaps; return qemuCaps;
error: error:
VIR_FREE(qmperr);
virObjectUnref(qemuCaps); virObjectUnref(qemuCaps);
qemuCaps = NULL; qemuCaps = NULL;
return NULL; return NULL;

View File

@ -308,7 +308,8 @@ int virQEMUCapsParseHelpStr(const char *qemu,
unsigned int *version, unsigned int *version,
bool *is_kvm, bool *is_kvm,
unsigned int *kvm_version, unsigned int *kvm_version,
bool check_yajl); bool check_yajl,
const char *qmperr);
/* Only for use by test suite */ /* Only for use by test suite */
int virQEMUCapsParseDeviceStr(virQEMUCapsPtr qemuCaps, const char *str); int virQEMUCapsParseDeviceStr(virQEMUCapsPtr qemuCaps, const char *str);

View File

@ -59,7 +59,7 @@ static int testHelpStrParsing(const void *data)
goto cleanup; goto cleanup;
if (virQEMUCapsParseHelpStr("QEMU", help, flags, 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) if (info->error && virGetLastError()->code == info->error)
ret = 0; ret = 0;
goto cleanup; goto cleanup;