[libvirt] [PATCH] qemu: cleanup error checking on agent replies
On all the places where qemuAgentComand() was called, we did a check
for errors in the reply. Unfortunately, some of the places called
qemuAgentCheckError() without checking for non-null reply which might
have resulted in a crash.
So this patch makes the error-checking part of qemuAgentCommand()
itself, which:
a) makes it look better,
b) makes the check mandatory and, most importantly,
c) checks for the errors if and only if it is appropriate.
This actually fixes a potential crashers when qemuAgentComand()
returned 0, but reply was NULL. Having said that, it *should* fix the
following bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1058149
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/qemu/qemu_agent.c | 27 +++++++--------------------
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 27af1be..4c83d7d 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -117,6 +117,8 @@ struct _qemuAgent {
qemuAgentEvent await_event;
};
+static int qemuAgentCheckError(virJSONValuePtr cmd, virJSONValuePtr reply);
+
static virClassPtr qemuAgentClass;
static void qemuAgentDispose(void *obj);
@@ -1009,6 +1011,7 @@ qemuAgentCommand(qemuAgentPtr mon,
}
} else {
*reply = msg.rxObject;
+ ret = qemuAgentCheckError(cmd, *reply);
}
}
@@ -1277,9 +1280,6 @@ int qemuAgentShutdown(qemuAgentPtr mon,
ret = qemuAgentCommand(mon, cmd, &reply,
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK);
- if (reply && ret == 0)
- ret = qemuAgentCheckError(cmd, reply);
-
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -1308,8 +1308,7 @@ int qemuAgentFSFreeze(qemuAgentPtr mon)
return -1;
if (qemuAgentCommand(mon, cmd, &reply,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 ||
- qemuAgentCheckError(cmd, reply) < 0)
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
goto cleanup;
if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
@@ -1346,8 +1345,7 @@ int qemuAgentFSThaw(qemuAgentPtr mon)
return -1;
if (qemuAgentCommand(mon, cmd, &reply,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 ||
- qemuAgentCheckError(cmd, reply) < 0)
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
goto cleanup;
if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
@@ -1386,9 +1384,6 @@ qemuAgentSuspend(qemuAgentPtr mon,
ret = qemuAgentCommand(mon, cmd, &reply,
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK);
- if (reply && ret == 0)
- ret = qemuAgentCheckError(cmd, reply);
-
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -1419,9 +1414,6 @@ qemuAgentArbitraryCommand(qemuAgentPtr mon,
if ((ret = qemuAgentCommand(mon, cmd, &reply, timeout)) < 0)
goto cleanup;
- if ((ret = qemuAgentCheckError(cmd, reply)) < 0)
- goto cleanup;
-
if (!(*result = virJSONValueToString(reply, false)))
ret = -1;
@@ -1449,9 +1441,6 @@ qemuAgentFSTrim(qemuAgentPtr mon,
ret = qemuAgentCommand(mon, cmd, &reply,
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK);
- if (reply && ret == 0)
- ret = qemuAgentCheckError(cmd, reply);
-
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
@@ -1472,8 +1461,7 @@ qemuAgentGetVCPUs(qemuAgentPtr mon,
return -1;
if (qemuAgentCommand(mon, cmd, &reply,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 ||
- qemuAgentCheckError(cmd, reply) < 0)
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
goto cleanup;
if (!(data = virJSONValueObjectGet(reply, "return"))) {
@@ -1581,8 +1569,7 @@ qemuAgentSetVCPUs(qemuAgentPtr mon,
cpus = NULL;
if (qemuAgentCommand(mon, cmd, &reply,
- VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 ||
- qemuAgentCheckError(cmd, reply) < 0)
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
goto cleanup;
if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
--
1.9.1