diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index cbc0995134..eeede6b1da 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; @@ -1548,6 +1547,12 @@ qemuAgentSetVCPUs(qemuAgentPtr mon, for (i = 0; i < ninfo; i++) { qemuAgentCPUInfoPtr in = &info[i]; + /* don't set state for cpus that were not touched */ + if (!in->modified) + continue; + + (*nmodified)++; + /* create single cpu object */ if (!(cpu = virJSONValueNewObject())) 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 bogus since we wouldn't + * call the guest agent so that 0 cpus would be set successfully. Reporting + * more successfully 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, } +/** + * Set the VCPU state using guest agent. + * + * Attempts to set the guest agent state for all cpus or until a proper error is + * reported by the guest agent. This may require multiple calls. + * + * Returns -1 on error, 0 on success. + */ +int +qemuAgentSetVCPUs(qemuAgentPtr mon, + qemuAgentCPUInfoPtr info, + size_t ninfo) +{ + int rv; + int nmodified; + size_t i; + + do { + if ((rv = qemuAgentSetVCPUsCommand(mon, info, ninfo, &nmodified)) < 0) + return -1; + + /* all vcpus were set successfully */ + if (rv == nmodified) + return 0; + + /* un-mark vcpus that were already set */ + for (i = 0; i < ninfo && rv > 0; i++) { + if (!info[i].modified) + continue; + + info[i].modified = false; + rv--; + } + } while (1); + + return 0; +} + + /* modify the cpu info structure to set the correct amount of cpus */ int qemuAgentUpdateCPUInfo(unsigned int nvcpus, @@ -1647,12 +1704,14 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus, /* unplug */ if (cpuinfo[i].offlinable && cpuinfo[i].online) { cpuinfo[i].online = false; + cpuinfo[i].modified = true; nonline--; } } else if (nvcpus > nonline) { /* plug */ if (!cpuinfo[i].online) { cpuinfo[i].online = true; + cpuinfo[i].modified = true; nonline++; } } else { diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index c092504a38..6dd9c702dd 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -95,6 +95,8 @@ struct _qemuAgentCPUInfo { unsigned int id; /* logical cpu ID */ bool online; /* true if the CPU is activated */ bool offlinable; /* true if the CPU can be offlined */ + + bool modified; /* set to true if the vcpu state needs to be changed */ }; int qemuAgentGetVCPUs(qemuAgentPtr mon, qemuAgentCPUInfoPtr *info); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d065e45fe9..098840ffc0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4804,19 +4804,6 @@ qemuDomainSetVcpusAgent(virDomainObjPtr vm, ret = qemuAgentSetVCPUs(qemuDomainGetAgent(vm), cpuinfo, ncpuinfo); qemuDomainObjExitAgent(vm); - if (ret < 0) - goto cleanup; - - if (ret < ncpuinfo) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to set state of cpu %d via guest agent"), - cpuinfo[ret-1].id); - ret = -1; - goto cleanup; - } - - ret = 0; - cleanup: VIR_FREE(cpuinfo); diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 60dafd9c80..4aa2c492ab 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -517,17 +517,15 @@ static const char testQemuAgentCPUResponse[] = "}"; static const char testQemuAgentCPUArguments1[] = - "[{\"logical-id\":0,\"online\":true}," - "{\"logical-id\":1,\"online\":false}," - "{\"logical-id\":2,\"online\":true}," - "{\"logical-id\":3,\"online\":false}]"; + "[{\"logical-id\":1,\"online\":false}]"; static const char testQemuAgentCPUArguments2[] = - "[{\"logical-id\":0,\"online\":true}," - "{\"logical-id\":1,\"online\":true}," - "{\"logical-id\":2,\"online\":true}," + "[{\"logical-id\":1,\"online\":true}," "{\"logical-id\":3,\"online\":true}]"; +static const char testQemuAgentCPUArguments3[] = + "[{\"logical-id\":3,\"online\":true}]"; + static int testQemuAgentCPU(const void *data) { @@ -566,44 +564,40 @@ testQemuAgentCPU(const void *data) goto cleanup; if (qemuMonitorTestAddItemParams(test, "guest-set-vcpus", - "{ \"return\" : 4 }", + "{ \"return\" : 1 }", "vcpus", testQemuAgentCPUArguments1, NULL) < 0) goto cleanup; - if ((nvcpus = qemuAgentSetVCPUs(qemuMonitorTestGetAgent(test), - cpuinfo, nvcpus)) < 0) + if (qemuAgentSetVCPUs(qemuMonitorTestGetAgent(test), cpuinfo, nvcpus) < 0) goto cleanup; - if (nvcpus != 4) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "Expected '4' cpus updated , got '%d'", nvcpus); - goto cleanup; - } - - /* try to hotplug two */ + /* try to hotplug two, second one will fail*/ if (qemuMonitorTestAddAgentSyncResponse(test) < 0) goto cleanup; if (qemuMonitorTestAddItemParams(test, "guest-set-vcpus", - "{ \"return\" : 4 }", + "{ \"return\" : 1 }", "vcpus", testQemuAgentCPUArguments2, NULL) < 0) goto cleanup; + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) + goto cleanup; + + if (qemuMonitorTestAddItemParams(test, "guest-set-vcpus", + "{ \"error\" : \"random error\" }", + "vcpus", testQemuAgentCPUArguments3, + NULL) < 0) + goto cleanup; + if (qemuAgentUpdateCPUInfo(4, cpuinfo, nvcpus) < 0) goto cleanup; - if ((nvcpus = qemuAgentSetVCPUs(qemuMonitorTestGetAgent(test), - cpuinfo, nvcpus)) < 0) + /* this should fail */ + if (qemuAgentSetVCPUs(qemuMonitorTestGetAgent(test), cpuinfo, nvcpus) != -1) goto cleanup; - if (nvcpus != 4) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "Expected '4' cpus updated , got '%d'", nvcpus); - goto cleanup; - } - ret = 0; cleanup: