On Mon, Jun 20, 2016 at 04:34:19PM +0200, Peter Krempa wrote:
Documentation for the "guest-set-vcpus" command describes a proper algorithm how to set vcpus. This patch makes the following changes:
- state of cpus that has not changed is not updated - if the command was partially successful the command is re-tried with the rest of the arguments to get a proper error message - code is more robust against mailicious guest agent
s/mailicious/malicious/
- fix testsuite to the new semantics --- src/qemu/qemu_agent.c | 83 ++++++++++++++++++++++++++++++++++++++++++-------- src/qemu/qemu_agent.h | 2 ++ src/qemu/qemu_driver.c | 13 -------- tests/qemuagenttest.c | 44 ++++++++++++-------------- 4 files changed, 92 insertions(+), 50 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index cbc0995..5bd767a 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1523,16 +1523,13 @@ qemuAgentGetVCPUs(qemuAgentPtr mon, return ret; }
-/** - * Set the VCPU state using guest agent. - * - * Returns -1 on error, ninfo in case everything was successful and less than - * ninfo on a partial failure. - */ -int -qemuAgentSetVCPUs(qemuAgentPtr mon, - qemuAgentCPUInfoPtr info, - size_t ninfo) + +/* returns the value provided by the guest agent or -1 on internal error */ +static int +qemuAgentSetVCPUsCommand(qemuAgentPtr mon, + qemuAgentCPUInfoPtr info, + size_t ninfo, + int *nmodified) { int ret = -1; virJSONValuePtr cmd = NULL; @@ -1541,6 +1538,8 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, virJSONValuePtr cpu = NULL; size_t i;
+ *nmodified = 0; + /* create the key data array */ if (!(cpus = virJSONValueNewArray())) goto cleanup; @@ -1552,6 +1551,12 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, if (!(cpu = virJSONValueNewObject())) goto cleanup;
+ /* don't set state for cpus that were not touched */ + if (!in->modified) + continue;
This needs to go before the virJSONValueNewObject, otherwise it would leak memory.
+ + (*nmodified)++; + if (virJSONValueObjectAppendNumberInt(cpu, "logical-id", in->id) < 0) goto cleanup;
@@ -1564,6 +1569,11 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, cpu = NULL; }
+ if (*nmodified == 0) { + ret = 0; + goto cleanup; + } + if (!(cmd = qemuAgentMakeCommand("guest-set-vcpus", "a:vcpus", cpus, NULL))) @@ -1575,9 +1585,17 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) goto cleanup;
- if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { + if (qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + /* All negative values are invalid. Return of 0 is bougs since we wouldn't
s/bougs/bogus/
+ * call the guest agent so that 0 cpus would be set successfully. Reporting + * more successfuly set vcpus that we've asked for is invalid */
s/successfuly/successfully/ s/invalid/invalid./
+ if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0 || + ret <= 0 || ret > *nmodified) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("malformed return value")); + _("guest agent returned malformed or invalid return value")); + ret = -1; }
cleanup: @@ -1589,6 +1607,45 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, }
[...] ACK