Check the returned value for NULL and take the cleanup path appropriately if the API fails. --- tools/virsh-domain.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 46f07ed..9ea5ffc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7739,7 +7739,10 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd) vshError(ctl, "%s", _("timeout, async and block options are exclusive")); goto cleanup; } + result = virDomainQemuAgentCommand(dom, guest_agent_cmd, timeout, flags); + if (!result) + goto cleanup; if (vshCommandOptBool(cmd, "pretty")) { char *tmp; -- 1.8.2.1

The code for arbitrary guest agent passthrough was horribly broken since introduciton. Fix it to correctly report errors. --- src/qemu/qemu_agent.c | 31 +++++++++++++++++++------------ src/qemu/qemu_driver.c | 10 +++------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index c7a9681..00fe13f 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1408,25 +1408,32 @@ qemuAgentArbitraryCommand(qemuAgentPtr mon, int timeout) { int ret = -1; - virJSONValuePtr cmd; + virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; *result = NULL; - if (timeout < VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN) - return ret; + if (timeout < VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN) { + virReportError(VIR_ERR_INVALID_ARG, + _("guest agent timeout '%d' is " + "less than the minimum '%d'"), + timeout, VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN); + goto cleanup; + } - cmd = virJSONValueFromString(cmd_str); - if (!cmd) - return ret; + if (!(cmd = virJSONValueFromString(cmd_str))) + goto cleanup; + + if ((ret = qemuAgentCommand(mon, cmd, &reply, timeout)) < 0) + goto cleanup; - ret = qemuAgentCommand(mon, cmd, &reply, timeout); + if ((ret = qemuAgentCheckError(cmd, reply)) < 0) + goto cleanup; - if (ret == 0) { - ret = qemuAgentCheckError(cmd, reply); - if (!(*result = virJSONValueToString(reply, false))) - ret = -1; - } + if (!(*result = virJSONValueToString(reply, false))) + ret = -1; + +cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5ca0fd4..9d3f632 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14881,16 +14881,12 @@ qemuDomainQemuAgentCommand(virDomainPtr domain, qemuDomainObjEnterAgent(vm); ret = qemuAgentArbitraryCommand(priv->agent, cmd, &result, timeout); qemuDomainObjExitAgent(vm); - if (ret < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Failed to execute agent command")); - goto endjob; - } + if (ret < 0) + VIR_FREE(result); endjob: - if (qemuDomainObjEndJob(driver, vm) == 0) { + if (qemuDomainObjEndJob(driver, vm) == 0) vm = NULL; - } cleanup: if (vm) -- 1.8.2.1

The original implementation didn't follow the established pattern and did not dispatch errors in case of failure. --- src/libvirt-qemu.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c index 747488d..e884e32 100644 --- a/src/libvirt-qemu.c +++ b/src/libvirt-qemu.c @@ -211,6 +211,7 @@ virDomainQemuAgentCommand(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; + char *ret; VIR_DEBUG("domain=%p, cmd=%s, timeout=%d, flags=%x", domain, cmd, timeout, flags); @@ -228,13 +229,17 @@ virDomainQemuAgentCommand(virDomainPtr domain, conn = domain->conn; if (conn->driver->domainQemuAgentCommand) { - return conn->driver->domainQemuAgentCommand(domain, cmd, - timeout, flags); + ret = conn->driver->domainQemuAgentCommand(domain, cmd, + timeout, flags); + if (!ret) + goto error; + return ret; } virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); /* Copy to connection error object for back compatibility */ +error: virDispatchError(conn); return NULL; } -- 1.8.2.1