1
0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 17:34:18 +03:00

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 malicious guest agent
- fix testsuite to the new semantics
This commit is contained in:
Peter Krempa 2016-06-20 14:15:50 +02:00
parent 69e0cd3397
commit b1aa91e140
4 changed files with 93 additions and 51 deletions

View File

@ -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 {

View File

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

View File

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

View File

@ -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: