In the past capabilities could be determined in other ways if QMP
messaging didn't succeed so a non-fatal error case was included in the
capabilities and QMP Process code.
For a while now, QMP capabilities failure has been a fatal case.
This patch makes QMP process failures return as a fatal error in all
cases consistent with 1) all failures actually being fatal in
QMP capabilities code and 2) the QMP process code being made generic.
The process changes impact the capabilities code because non-fatal
return codes are no longer returned.
The rest of the QMP associated capabilities code is updated to make all
errors fatal for consistency.
The process changes also force a change in QMP process stderr reporting
because the previous triggers for stderr reporting are no longer passed
through the function interfaces.
As best I can tell the only case where QMP process stderr should be
explicitly reported is when the QMP process exits with a non-zero
status.
Error reporting is moved to virQEMUCapsInitQMP and the QMP process
stderr is reported only when the QMP process status code is non-zero
(and is only reported for the primary QMP query not the secondary TCG
specific QMP query.)
Signed-off-by: Chris Venteicher <cventeic@xxxxxxxxxx>
---
src/qemu/qemu_capabilities.c | 83 ++++++++++++++++--------------------
src/qemu/qemu_process.c | 24 ++++-------
src/qemu/qemu_process.h | 1 +
3 files changed, 45 insertions(+), 63 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index bed9ca26a1..1efec85b57 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4038,7 +4038,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
if (qemuMonitorSetCapabilities(mon) < 0) {
VIR_DEBUG("Failed to set monitor capabilities %s",
virGetLastErrorMessage());
- ret = 0;
goto cleanup;
}
@@ -4047,7 +4046,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
&package) < 0) {
VIR_DEBUG("Failed to query monitor version %s",
virGetLastErrorMessage());
- ret = 0;
goto cleanup;
}
@@ -4218,7 +4216,6 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED,
if (qemuMonitorSetCapabilities(mon) < 0) {
VIR_DEBUG("Failed to set monitor capabilities %s",
virGetLastErrorMessage());
- ret = 0;
goto cleanup;
}
@@ -4234,25 +4231,47 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED,
}
+#define MESSAGE_ID_CAPS_PROBE_FAILURE "8ae2f3fb-2dbe-498e-8fbd-012d40afa361"
+
+static void
+virQEMUCapsLogProbeFailure(const char *binary)
+{
+ virLogMetadata meta[] = {
+ { .key = "MESSAGE_ID", .s = MESSAGE_ID_CAPS_PROBE_FAILURE, .iv = 0 },
+ { .key = "LIBVIRT_QEMU_BINARY", .s = binary, .iv = 0 },
+ { .key = NULL },
+ };
+
+ virLogMessage(&virLogSelf,
+ VIR_LOG_WARN,
+ __FILE__, __LINE__, __func__,
+ meta,
+ _("Failed to probe capabilities for %s: %s"),
+ binary, virGetLastErrorMessage());
+}
+
+
static int
virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
const char *libDir,
uid_t runUid,
- gid_t runGid,
- char **qmperr)
+ gid_t runGid)
{
qemuProcessQmpPtr proc = NULL;
qemuProcessQmpPtr procTCG = NULL;
+ char *qmperr = NULL;
int ret = -1;
- int rc;
if (!(proc = qemuProcessQmpNew(qemuCaps->binary, libDir,
- runUid, runGid, qmperr, false)))
+ runUid, runGid, &qmperr, false)))
goto cleanup;
- if ((rc = qemuProcessQmpRun(proc)) != 0) {
- if (rc == 1)
- ret = 0;
+ if (qemuProcessQmpRun(proc) < 0) {
+ if (proc->status != 0)
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to probe QEMU binary with QMP: %s"),
+ qmperr ? qmperr : _("uknown error"));
+
goto cleanup;
}
@@ -4270,11 +4289,8 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
procTCG = qemuProcessQmpNew(qemuCaps->binary, libDir,
runUid, runGid, NULL, true);
- if ((rc = qemuProcessQmpRun(procTCG)) != 0) {
- if (rc == 1)
- ret = 0;
+ if (qemuProcessQmpRun(procTCG) < 0)
goto cleanup;
- }
if (virQEMUCapsInitQMPMonitorTCG(qemuCaps, procTCG->mon) < 0)
goto cleanup;
@@ -4283,34 +4299,19 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
ret = 0;
cleanup:
+ if (ret < 0)
+ virQEMUCapsLogProbeFailure(qemuCaps->binary);
+
qemuProcessQmpStop(proc);
qemuProcessQmpStop(procTCG);
qemuProcessQmpFree(proc);
qemuProcessQmpFree(procTCG);
+ VIR_FREE(qmperr);
+
return ret;
}
-#define MESSAGE_ID_CAPS_PROBE_FAILURE "8ae2f3fb-2dbe-498e-8fbd-012d40afa361"
-
-static void
-virQEMUCapsLogProbeFailure(const char *binary)
-{
- virLogMetadata meta[] = {
- { .key = "MESSAGE_ID", .s = MESSAGE_ID_CAPS_PROBE_FAILURE, .iv = 0 },
- { .key = "LIBVIRT_QEMU_BINARY", .s = binary, .iv = 0 },
- { .key = NULL },
- };
-
- virLogMessage(&virLogSelf,
- VIR_LOG_WARN,
- __FILE__, __LINE__, __func__,
- meta,
- _("Failed to probe capabilities for %s: %s"),
- binary, virGetLastErrorMessage());
-}
-
-
virQEMUCapsPtr
virQEMUCapsNewForBinaryInternal(virArch hostArch,
const char *binary,
@@ -4322,7 +4323,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
{
virQEMUCapsPtr qemuCaps;
struct stat sb;
- char *qmperr = NULL;
if (!(qemuCaps = virQEMUCapsNew()))
goto error;
@@ -4349,18 +4349,8 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
goto error;
}
- if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid, &qmperr) < 0) {
- virQEMUCapsLogProbeFailure(binary);
+ if (virQEMUCapsInitQMP(qemuCaps, libDir, runUid, runGid) < 0)
goto error;
- }
-
- if (!qemuCaps->usedQMP) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to probe QEMU binary with QMP: %s"),
- qmperr ? qmperr : _("unknown error"));
- virQEMUCapsLogProbeFailure(binary);
- goto error;
- }
qemuCaps->libvirtCtime = virGetSelfLastChanged();
qemuCaps->libvirtVersion = LIBVIR_VERSION_NUMBER;
@@ -4376,7 +4366,6 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch,
}
cleanup:
- VIR_FREE(qmperr);
return qemuCaps;
error:
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 324151a7c3..2ec0d5340d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8176,16 +8176,11 @@ qemuProcessQmpNew(const char *binary,
}
-/* Returns -1 on fatal error,
- * 0 on success,
- * 1 when probing QEMU failed
- */
int
qemuProcessQmpRun(qemuProcessQmpPtr proc)
{
virDomainXMLOptionPtr xmlopt = NULL;
const char *machine;
- int status = 0;
int ret = -1;
if (proc->forceTCG)
@@ -8220,19 +8215,20 @@ qemuProcessQmpRun(qemuProcessQmpPtr proc)
virCommandSetErrorBuffer(proc->cmd, proc->qmperr);
- /* Log, but otherwise ignore, non-zero status. */
- if (virCommandRun(proc->cmd, &status) < 0)
+ proc->status = 0;
+
+ if (virCommandRun(proc->cmd, &proc->status) < 0)
goto cleanup;
- if (status != 0) {
+ if (proc->status != 0) {
VIR_DEBUG("QEMU %s exited with status %d: %s",
- proc->binary, status, *proc->qmperr);
- goto ignore;
+ proc->binary, proc->status, *proc->qmperr);
+ goto cleanup;
}
if (virPidFileReadPath(proc->pidfile, &proc->pid) < 0) {
VIR_DEBUG("Failed to read pidfile %s", proc->pidfile);
- goto ignore;
+ goto cleanup;
}
if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
@@ -8243,7 +8239,7 @@ qemuProcessQmpRun(qemuProcessQmpPtr proc)
if (!(proc->mon = qemuMonitorOpen(proc->vm, &proc->config, true, true,
0, &callbacks, NULL)))
- goto ignore;
+ goto cleanup;
virObjectLock(proc->mon);
@@ -8255,10 +8251,6 @@ qemuProcessQmpRun(qemuProcessQmpPtr proc)
virObjectUnref(xmlopt);
return ret;
-
- ignore:
- ret = 1;
- goto cleanup;
}
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 51598b402e..a56e9608cd 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -220,6 +220,7 @@ struct _qemuProcessQmp {
char *binary;
uid_t runUid;
gid_t runGid;
+ int status;
char **qmperr;
char *monarg;
char *monpath;
--
2.17.1
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
