Re: [libvirt] [PATCH 03/41] qemu: Use virDomainCapsCPUModels for cpuDefinitions
On 08/12/2016 09:32 AM, Jiri Denemark wrote:
The list of supported CPU models in domain capabilities is stored in
virDomainCapsCPUModels. Let's use the same object for storing CPU models
in QEMU capabilities.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 162 +++++++++++++++++++++++++++----------------
src/qemu/qemu_capabilities.h | 10 +--
src/qemu/qemu_command.c | 8 ++-
src/qemu/qemu_monitor.c | 12 +++-
src/qemu/qemu_monitor.h | 10 ++-
src/qemu/qemu_monitor_json.c | 24 +++++--
src/qemu/qemu_monitor_json.h | 2 +-
tests/qemumonitorjsontest.c | 8 +--
tests/qemuxml2argvtest.c | 18 ++---
9 files changed, 164 insertions(+), 90 deletions(-)
[..]
-size_t virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
- char ***names)
+int
+virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
+ char ***names,
+ size_t *count)
{
+ size_t i;
+ char **models = NULL;
+
+ *count = 0;
if (names)
- *names = qemuCaps->cpuDefinitions;
- return qemuCaps->ncpuDefinitions;
+ *names = NULL;
+
+ if (!qemuCaps->cpuDefinitions)
+ return 0;
+
+ if (names && VIR_ALLOC_N(models, qemuCaps->cpuDefinitions->count) <
0)
+ return -1;
+
So if we don't have names, we don't get models and the following loop
will only add to models if we've allocated it because we have names, right?
Thus could there be an optimization to set/return ->count if !names
prior to this check? e.g.
if (!names) {
*count = qemuCaps->cpuDefinitions->count;
return 0;
}
This works, but it's tough to read.
+ for (i = 0; i < qemuCaps->cpuDefinitions->count; i++)
{
+ virDomainCapsCPUModelPtr cpu = qemuCaps->cpuDefinitions->models + i;
+ if (models && VIR_STRDUP(models[i], cpu->name) < 0)
+ goto error;
+ }
+
+ if (names)
+ *names = models;
+ *count = qemuCaps->cpuDefinitions->count;
+ return 0;
+
+ error:
+ virStringFreeListCount(models, i);
+ return -1;
}
[...]
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4855,14 +4855,15 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon,
}
-int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
- char ***cpus)
+int
+qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
+ qemuMonitorCPUDefInfoPtr **cpus)
{
int ret = -1;
virJSONValuePtr cmd;
virJSONValuePtr reply = NULL;
virJSONValuePtr data;
- char **cpulist = NULL;
+ qemuMonitorCPUDefInfoPtr *cpulist = NULL;
int n = 0;
size_t i;
@@ -4898,13 +4899,18 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
goto cleanup;
}
- /* null-terminated list */
- if (VIR_ALLOC_N(cpulist, n + 1) < 0)
+ if (VIR_ALLOC_N(cpulist, n) < 0)
goto cleanup;
for (i = 0; i < n; i++) {
virJSONValuePtr child = virJSONValueArrayGet(data, i);
const char *tmp;
+ qemuMonitorCPUDefInfoPtr cpu;
+
+ if (VIR_ALLOC(cpu) < 0)
+ goto cleanup;
+
+ cpulist[i] = cpu;
if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -4912,7 +4918,7 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
goto cleanup;
}
- if (VIR_STRDUP(cpulist[i], tmp) < 0)
+ if (VIR_STRDUP(cpu->name, tmp) < 0)
goto cleanup;
}
@@ -4921,7 +4927,11 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
cpulist = NULL;
cleanup:
- virStringFreeList(cpulist);
+ if (ret < 0 && cpulist) {
I think "ret < 0" is superfluous... Coverity whines too
+ for (i = 0; i < n; i++)
+ qemuMonitorCPUDefInfoFree(cpulist[i]);
+ VIR_FREE(cpulist);
+ }
virJSONValueFree(cmd);
virJSONValueFree(reply);
return ret;
[...]