6:09 p.m.
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