On 04/15/2013 09:11 AM, Peter Krempa wrote:
The qemu guest agent allows to online and offline CPUs from the perspective of the guest. This patch adds helpers that call 'guest-get-vcpus' and 'guest-set-vcpus' guest agent functions and convert the data for internal libvirt usage. --- src/qemu/qemu_agent.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 12 +++++ 2 files changed, 149 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
+int +qemuAgentGetVCPUs(qemuAgentPtr mon, + qemuAgentCPUInfoPtr *info) +{ + int ret = -1; + int i; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + + if (!(cmd = qemuAgentMakeCommand("guest-get-vcpus", NULL))) + return -1; + + if (qemuAgentCommand(mon, cmd, &reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 || + qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest-get-vcpus reply was missing return data")); + goto cleanup; + } + + if (data->type != VIR_JSON_TYPE_ARRAY) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest-get-vcpus return information was not an array")); + goto cleanup; + } + + if (VIR_ALLOC_N(*info, virJSONValueArraySize(data)) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < virJSONValueArraySize(data) ; i++) {
Quite a few calls to this...
+ virJSONValuePtr entry = virJSONValueArrayGet(data, i); + qemuAgentCPUInfoPtr in = *info + i; + + if (!entry) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("array element missing in guest-get-vcpus return " + "value")); + goto cleanup; + } + + if (virJSONValueObjectGetNumberUint(entry, "logical-id", &in->id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'logical-id' missing in reply of guest-get-vcpus")); + goto cleanup; + } + + if (virJSONValueObjectGetBoolean(entry, "online", &in->online) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'online' missing in reply of guest-get-vcpus "));
trailing space in the error message
+ goto cleanup; + } + + if (virJSONValueObjectGetBoolean(entry, "can-offline", + &in->offlinable) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'can-offline' missing in reply of guest-get-vcpus")); + goto cleanup; + } + } + + ret = virJSONValueArraySize(data);
...and again. Worth caching in a local variable?
+ +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + virJSONValueFree(data); + return ret; +}
I checked qemu's qga/qapi-schema.json, and this matches the documentation.
+ +int +qemuAgentSetVCPUs(qemuAgentPtr mon, + qemuAgentCPUInfoPtr info, + size_t ninfo) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + virJSONValuePtr cpus = NULL; + virJSONValuePtr cpu = NULL; + size_t i; + + /* create the key data array */ + if (!(cpus = virJSONValueNewArray())) + goto no_memory; + + for (i = 0; i < ninfo; i++) { + qemuAgentCPUInfoPtr in = &info[i]; + + /* create single cpu object */ + if (!(cpu = virJSONValueNewObject())) + goto no_memory; + + if (virJSONValueObjectAppendNumberInt(cpu, "logical-id", in->id) < 0) + goto no_memory; + + if (virJSONValueObjectAppendBoolean(cpu, "online", in->online) < 0) + goto no_memory; + + if (virJSONValueArrayAppend(cpus, cpu) < 0) + goto no_memory; + + cpu = NULL; + } + + if (!(cmd = qemuAgentMakeCommand("guest-set-vcpus", + "a:vcpus", cpus, + NULL))) + goto cleanup;
This has transfer semantics - after this point, cpus is owned by cmd. You need to do: cpus = NULL here...
+ + if (qemuAgentCommand(mon, cmd, &reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 || + qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed return value")); + }
Potential problem. In the qga documentation, this function is documented as returning < ninfo if the command partially succeeded. I don't know how you plan on using this in later functions, but it might be worth documenting that this function has the same read()-like semantics (a return of -1 is outright error, a return of 0 meant no changes were requested, a return of < ninfo means the first few values were processed before hitting an error, and a return of ninfo means all changes were successful).
+ +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + virJSONValueFree(cpu); + virJSONValueFree(cpus);
...in order to avoid a double-free here.
+ return ret; + +no_memory: + virReportOOMError(); + goto cleanup; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index ba04e61..79be283 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -82,4 +82,16 @@ int qemuAgentArbitraryCommand(qemuAgentPtr mon, int timeout); int qemuAgentFSTrim(qemuAgentPtr mon, unsigned long long minimum); + + +typedef struct _qemuAgentCPUInfo qemuAgentCPUInfo; +typedef qemuAgentCPUInfo *qemuAgentCPUInfoPtr; +struct _qemuAgentCPUInfo { + unsigned int id; /* locigal cpu ID */
s/locigal/logical/
+ bool online; /* true if the CPU is activated */ + bool offlinable; /* true if the CPU can be offlined - ignored when setting*/
space before */, and maybe wrap this long line Enough comments that it is probably worth seeing a v2 (or at least waiting for my review of the client of this new function in the tail of the series). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org