On 02/15/2017 11:44 AM, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
>
> 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);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
