qemu: remove redundant needReply argument of qemuAgentCommand

needReply added in [1] looks redundant. Indeed it is set to false only
when mon->await_event is set too (the only exception qemuAgentFSTrim
which is mistaken).

However it fixes the issue when qemuAgentCommand exits on error path and
mon->await_event is not reset. Let's instead reset mon->await_event properly.

Also remove "Woken up by event" debug message as it can be misleading.
We can get it also if monitor is closed due to serial changed event
currently. Anyway both qemuAgentClose and qemuAgentNotifyEvent log
itself.

[1] qemu: make sure agent returns error when required data are missing

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
Nikolay Shirokovskiy 2020-03-05 17:47:02 +03:00 committed by Michal Privoznik
parent b47e3b9b5c
commit 52532073d8
1 changed files with 37 additions and 37 deletions

View File

@ -1096,7 +1096,6 @@ static int
qemuAgentCommand(qemuAgentPtr agent, qemuAgentCommand(qemuAgentPtr agent,
virJSONValuePtr cmd, virJSONValuePtr cmd,
virJSONValuePtr *reply, virJSONValuePtr *reply,
bool needReply,
int seconds) int seconds)
{ {
int ret = -1; int ret = -1;
@ -1105,18 +1104,17 @@ qemuAgentCommand(qemuAgentPtr agent,
int await_event = agent->await_event; int await_event = agent->await_event;
*reply = NULL; *reply = NULL;
memset(&msg, 0, sizeof(msg));
if (!agent->running) { if (!agent->running) {
virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
_("Guest agent disappeared while executing command")); _("Guest agent disappeared while executing command"));
return -1; goto cleanup;
} }
if (qemuAgentGuestSync(agent) < 0) if (qemuAgentGuestSync(agent) < 0)
return -1; return -1;
memset(&msg, 0, sizeof(msg));
if (!(cmdstr = virJSONValueToString(cmd, false))) if (!(cmdstr = virJSONValueToString(cmd, false)))
goto cleanup; goto cleanup;
msg.txBuffer = g_strdup_printf("%s" LINE_ENDING, cmdstr); msg.txBuffer = g_strdup_printf("%s" LINE_ENDING, cmdstr);
@ -1129,30 +1127,32 @@ qemuAgentCommand(qemuAgentPtr agent,
VIR_DEBUG("Receive command reply ret=%d rxObject=%p", VIR_DEBUG("Receive command reply ret=%d rxObject=%p",
ret, msg.rxObject); ret, msg.rxObject);
if (ret == 0) { if (ret < 0)
/* If we haven't obtained any reply but we wait for an goto cleanup;
* event, then don't report this as error */
if (!msg.rxObject) { /* If we haven't obtained any reply but we wait for an
if (await_event && !needReply) { * event, then don't report this as error */
VIR_DEBUG("Woken up by event %d", await_event); if (!msg.rxObject) {
if (!await_event) {
if (agent->running) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing agent reply object"));
} else { } else {
if (agent->running) virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Guest agent disappeared while executing command"));
_("Missing agent reply object"));
else
virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
_("Guest agent disappeared while executing command"));
ret = -1;
} }
} else { ret = -1;
*reply = msg.rxObject;
ret = qemuAgentCheckError(cmd, *reply);
} }
goto cleanup;
} }
*reply = msg.rxObject;
ret = qemuAgentCheckError(cmd, *reply);
cleanup: cleanup:
VIR_FREE(cmdstr); VIR_FREE(cmdstr);
VIR_FREE(msg.txBuffer); VIR_FREE(msg.txBuffer);
agent->await_event = QEMU_AGENT_EVENT_NONE;
return ret; return ret;
} }
@ -1253,7 +1253,7 @@ int qemuAgentShutdown(qemuAgentPtr agent,
agent->await_event = QEMU_AGENT_EVENT_RESET; agent->await_event = QEMU_AGENT_EVENT_RESET;
else else
agent->await_event = QEMU_AGENT_EVENT_SHUTDOWN; agent->await_event = QEMU_AGENT_EVENT_SHUTDOWN;
ret = qemuAgentCommand(agent, cmd, &reply, false, ret = qemuAgentCommand(agent, cmd, &reply,
VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN); VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN);
virJSONValueFree(cmd); virJSONValueFree(cmd);
@ -1296,7 +1296,7 @@ int qemuAgentFSFreeze(qemuAgentPtr agent, const char **mountpoints,
if (!cmd) if (!cmd)
goto cleanup; goto cleanup;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0)
goto cleanup; goto cleanup;
if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
@ -1333,7 +1333,7 @@ int qemuAgentFSThaw(qemuAgentPtr agent)
if (!cmd) if (!cmd)
return -1; return -1;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0)
goto cleanup; goto cleanup;
if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) {
@ -1370,7 +1370,7 @@ qemuAgentSuspend(qemuAgentPtr agent,
return -1; return -1;
agent->await_event = QEMU_AGENT_EVENT_SUSPEND; agent->await_event = QEMU_AGENT_EVENT_SUSPEND;
ret = qemuAgentCommand(agent, cmd, &reply, false, agent->timeout); ret = qemuAgentCommand(agent, cmd, &reply, agent->timeout);
virJSONValueFree(cmd); virJSONValueFree(cmd);
virJSONValueFree(reply); virJSONValueFree(reply);
@ -1399,7 +1399,7 @@ qemuAgentArbitraryCommand(qemuAgentPtr agent,
if (!(cmd = virJSONValueFromString(cmd_str))) if (!(cmd = virJSONValueFromString(cmd_str)))
goto cleanup; goto cleanup;
if ((ret = qemuAgentCommand(agent, cmd, &reply, true, timeout)) < 0) if ((ret = qemuAgentCommand(agent, cmd, &reply, timeout)) < 0)
goto cleanup; goto cleanup;
if (!(*result = virJSONValueToString(reply, false))) if (!(*result = virJSONValueToString(reply, false)))
@ -1426,7 +1426,7 @@ qemuAgentFSTrim(qemuAgentPtr agent,
if (!cmd) if (!cmd)
return ret; return ret;
ret = qemuAgentCommand(agent, cmd, &reply, false, agent->timeout); ret = qemuAgentCommand(agent, cmd, &reply, agent->timeout);
virJSONValueFree(cmd); virJSONValueFree(cmd);
virJSONValueFree(reply); virJSONValueFree(reply);
@ -1447,7 +1447,7 @@ qemuAgentGetVCPUs(qemuAgentPtr agent,
if (!(cmd = qemuAgentMakeCommand("guest-get-vcpus", NULL))) if (!(cmd = qemuAgentMakeCommand("guest-get-vcpus", NULL)))
return -1; return -1;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0)
goto cleanup; goto cleanup;
if (!(data = virJSONValueObjectGetArray(reply, "return"))) { if (!(data = virJSONValueObjectGetArray(reply, "return"))) {
@ -1560,7 +1560,7 @@ qemuAgentSetVCPUsCommand(qemuAgentPtr agent,
NULL))) NULL)))
goto cleanup; goto cleanup;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0)
goto cleanup; goto cleanup;
/* All negative values are invalid. Return of 0 is bogus since we wouldn't /* All negative values are invalid. Return of 0 is bogus since we wouldn't
@ -1715,7 +1715,7 @@ qemuAgentGetHostname(qemuAgentPtr agent,
if (!cmd) if (!cmd)
return ret; return ret;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) { if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) {
if (qemuAgentErrorCommandUnsupported(reply)) if (qemuAgentErrorCommandUnsupported(reply))
ret = -2; ret = -2;
goto cleanup; goto cleanup;
@ -1759,7 +1759,7 @@ qemuAgentGetTime(qemuAgentPtr agent,
if (!cmd) if (!cmd)
return ret; return ret;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0)
goto cleanup; goto cleanup;
if (virJSONValueObjectGetNumberUlong(reply, "return", &json_time) < 0) { if (virJSONValueObjectGetNumberUlong(reply, "return", &json_time) < 0) {
@ -1824,7 +1824,7 @@ qemuAgentSetTime(qemuAgentPtr agent,
if (!cmd) if (!cmd)
return ret; return ret;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0)
goto cleanup; goto cleanup;
ret = 0; ret = 0;
@ -1961,7 +1961,7 @@ qemuAgentGetFSInfo(qemuAgentPtr agent,
if (!cmd) if (!cmd)
return ret; return ret;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) { if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) {
if (qemuAgentErrorCommandUnsupported(reply)) if (qemuAgentErrorCommandUnsupported(reply))
ret = -2; ret = -2;
goto cleanup; goto cleanup;
@ -2112,7 +2112,7 @@ qemuAgentGetInterfaces(qemuAgentPtr agent,
if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL))) if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL)))
goto cleanup; goto cleanup;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0)
goto cleanup; goto cleanup;
if (!(ret_array = virJSONValueObjectGet(reply, "return"))) { if (!(ret_array = virJSONValueObjectGet(reply, "return"))) {
@ -2289,7 +2289,7 @@ qemuAgentSetUserPassword(qemuAgentPtr agent,
NULL))) NULL)))
goto cleanup; goto cleanup;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0)
goto cleanup; goto cleanup;
ret = 0; ret = 0;
@ -2320,7 +2320,7 @@ qemuAgentGetUsers(qemuAgentPtr agent,
if (!(cmd = qemuAgentMakeCommand("guest-get-users", NULL))) if (!(cmd = qemuAgentMakeCommand("guest-get-users", NULL)))
return -1; return -1;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) { if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) {
if (qemuAgentErrorCommandUnsupported(reply)) if (qemuAgentErrorCommandUnsupported(reply))
return -2; return -2;
return -1; return -1;
@ -2409,7 +2409,7 @@ qemuAgentGetOSInfo(qemuAgentPtr agent,
if (!(cmd = qemuAgentMakeCommand("guest-get-osinfo", NULL))) if (!(cmd = qemuAgentMakeCommand("guest-get-osinfo", NULL)))
return -1; return -1;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) { if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) {
if (qemuAgentErrorCommandUnsupported(reply)) if (qemuAgentErrorCommandUnsupported(reply))
return -2; return -2;
return -1; return -1;
@ -2464,7 +2464,7 @@ qemuAgentGetTimezone(qemuAgentPtr agent,
if (!(cmd = qemuAgentMakeCommand("guest-get-timezone", NULL))) if (!(cmd = qemuAgentMakeCommand("guest-get-timezone", NULL)))
return -1; return -1;
if (qemuAgentCommand(agent, cmd, &reply, true, agent->timeout) < 0) { if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) {
if (qemuAgentErrorCommandUnsupported(reply)) if (qemuAgentErrorCommandUnsupported(reply))
return -2; return -2;
return -1; return -1;