On 02/15/2017 11:44 AM, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> ---
Notes: Version 2: - no change
src/cpu/cpu_x86.c | 147 ++++++++++++++++++----------------------- src/cpu/cpu_x86.h | 3 - src/libvirt_private.syms | 1 - src/libxl/libxl_capabilities.c | 18 ++--- src/qemu/qemu_monitor_json.c | 33 ++++----- 5 files changed, 92 insertions(+), 110 deletions(-)
[...]
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 2bbd2d1b4..622e9f6bb 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -65,14 +65,14 @@ struct guest_arch { #define XEN_CAP_REGEX "(xen|hvm)-[[:digit:]]+\\.[[:digit:]]+-(aarch64|armv7l|x86_32|x86_64|ia64|powerpc64)(p|be)?"
static int -libxlCapsAddCPUID(virCPUx86Data *data, virCPUx86CPUID *cpuid, ssize_t ncaps) +libxlCapsAddCPUID(virCPUDataPtr data, virCPUx86CPUID *cpuid, ssize_t ncaps) { size_t i;
for (i = 0; i < ncaps; i++) { virCPUx86CPUID *c = &cpuid[i];
- if (virCPUx86DataAddCPUID(data, c) < 0) { + if (virCPUx86DataAddCPUID(&data->data.x86, c) < 0) { VIR_DEBUG("Failed to add CPUID(%x,%x)", c->eax_in, c->ecx_in); return -1; } @@ -112,7 +112,6 @@ libxlCapsNodeData(virCPUDefPtr cpu, libxl_hwcap hwcap, { ssize_t ncaps; virCPUDataPtr cpudata = NULL; - virCPUx86Data data = VIR_CPU_X86_DATA_INIT; virCPUx86CPUID cpuid[] = { { .eax_in = 0x00000001, .edx = hwcap[0] }, @@ -131,20 +130,23 @@ libxlCapsNodeData(virCPUDefPtr cpu, libxl_hwcap hwcap, { .eax_in = 0x80000007, .ecx_in = 0U, .edx = hwcap[7] }, };
+ if (!(cpudata = virCPUDataNew(cpu->arch))) + goto error; + ncaps = ARRAY_CARDINALITY(cpuid); - if (libxlCapsAddCPUID(&data, cpuid, ncaps) < 0) + if (libxlCapsAddCPUID(cpudata, cpuid, ncaps) < 0) goto error;
ncaps = ARRAY_CARDINALITY(cpuid_ver1); if (version > LIBXL_HWCAP_V0 && - libxlCapsAddCPUID(&data, cpuid_ver1, ncaps) < 0) + libxlCapsAddCPUID(cpudata, cpuid_ver1, ncaps) < 0) goto error;
- cpudata = virCPUx86MakeData(cpu->arch, &data); + return cpudata;
error: - virCPUx86DataClear(&data); - return cpudata; + cpuDataFree(cpudata);
Why is this not x86FreeCPUData ? Or should other code use the cpuDataFree? Am I missing something subtle?
+ return NULL; }
/* hw_caps is an array of 32-bit words whose meaning is listed in diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 415761525..b4da12167 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6565,37 +6565,35 @@ qemuMonitorJSONParseCPUx86FeatureWord(virJSONValuePtr data, }
-static int -qemuMonitorJSONParseCPUx86Features(virJSONValuePtr data, - virCPUDataPtr *cpudata) +static virCPUDataPtr +qemuMonitorJSONParseCPUx86Features(virJSONValuePtr data) { - virCPUx86Data x86Data = VIR_CPU_X86_DATA_INIT; + virCPUDataPtr cpudata = NULL; virCPUx86CPUID cpuid; size_t i; ssize_t n; - int ret = -1;
if (!data || (n = virJSONValueArraySize(data)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("invalid array of CPUID features")); - return -1; + goto error;
Could just return NULL, right?
}
+ if (!(cpudata = virCPUDataNew(VIR_ARCH_X86_64))) + goto error; + for (i = 0; i < n; i++) { if (qemuMonitorJSONParseCPUx86FeatureWord(virJSONValueArrayGet(data, i), &cpuid) < 0 || - virCPUx86DataAddCPUID(&x86Data, &cpuid) < 0) - goto cleanup; + virCPUx86DataAddCPUID(&cpudata->data.x86, &cpuid) < 0) + goto error; }
- if (!(*cpudata = virCPUx86MakeData(VIR_ARCH_X86_64, &x86Data))) - goto cleanup; + return cpudata;
- ret = 0; - - cleanup: - virCPUx86DataClear(&x86Data); - return ret; + error: + cpuDataFree(cpudata);
Similar query about x86FreeCPUData usage. Otherwise, things look fine... Perhaps a moot point since path 16 makes a common API... Perhaps the only reason to change would be if bisection breaks. ACK - I'll leave it to you to handle the calls properly John
+ return NULL; }
@@ -6622,7 +6620,10 @@ qemuMonitorJSONGetCPUx86Data(qemuMonitorPtr mon, goto cleanup;
data = virJSONValueObjectGetArray(reply, "return"); - ret = qemuMonitorJSONParseCPUx86Features(data, cpudata); + if (!(*cpudata = qemuMonitorJSONParseCPUx86Features(data))) + goto cleanup; + + ret = 0;
cleanup: virJSONValueFree(cmd);