Re: [libvirt] [PATCH 5/7] qemu: agent: Make setting of vcpus more robust

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

- 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;

+ + (*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

+ * call the guest agent so that 0 cpus would be set successfully. Reporting + * more successfuly set vcpus that we've asked for is 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, }