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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list