[libvirt] [PATCH 0/3] Several cleanups for cpu driver
Jiri Denemark (3): cpu_x86: Use x86-specific CPU data structure Replace union cpuData with virCPUData cpu: Store arch in virCPUData src/cpu/cpu.c | 31 +++-- src/cpu/cpu.h | 62 ++++----- src/cpu/cpu_arm.c | 8 +- src/cpu/cpu_powerpc.c | 12 +- src/cpu/cpu_s390.c | 8 +- src/cpu/cpu_x86.c | 301 +++++++++++++++++++++++++++---------------- src/qemu/qemu_capabilities.c | 4 +- src/qemu/qemu_command.c | 9 +- src/vmware/vmware_conf.c | 5 +- tests/cputest.c | 10 +- 10 files changed, 264 insertions(+), 186 deletions(-) -- 1.8.3.2
--- src/cpu/cpu.h | 2 +- src/cpu/cpu_x86.c | 271 ++++++++++++++++++++++++++++++++++-------------------- 2 files changed, 173 insertions(+), 100 deletions(-) diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index cba7149..b2c02db 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -33,7 +33,7 @@ union cpuData { - struct cpuX86Data x86; + struct cpuX86Data *x86; /* generic driver needs no data */ /* PowerPC driver need data*/ struct cpuPPCData ppc; diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index cc7926e..c6e78c5 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -51,7 +51,7 @@ struct x86_vendor { struct x86_feature { char *name; - union cpuData *data; + struct cpuX86Data *data; struct x86_feature *next; }; @@ -59,7 +59,7 @@ struct x86_feature { struct x86_model { char *name; const struct x86_vendor *vendor; - union cpuData *data; + struct cpuX86Data *data; struct x86_model *next; }; @@ -80,7 +80,7 @@ enum compare_result { struct data_iterator { - union cpuData *data; + struct cpuX86Data *data; int pos; bool extended; }; @@ -150,13 +150,11 @@ static struct cpuX86cpuid * x86DataCpuidNext(struct data_iterator *iterator) { struct cpuX86cpuid *ret; - struct cpuX86Data *data; + struct cpuX86Data *data = iterator->data; - if (!iterator->data) + if (!data) return NULL; - data = &iterator->data->x86; - do { ret = NULL; iterator->pos++; @@ -180,7 +178,7 @@ x86DataCpuidNext(struct data_iterator *iterator) static struct cpuX86cpuid * -x86DataCpuid(const union cpuData *data, +x86DataCpuid(const struct cpuX86Data *data, uint32_t function) { struct cpuX86cpuid *cpuids; @@ -188,13 +186,13 @@ x86DataCpuid(const union cpuData *data, size_t i; if (function < CPUX86_EXTENDED) { - cpuids = data->x86.basic; - len = data->x86.basic_len; + cpuids = data->basic; + len = data->basic_len; i = function; } else { - cpuids = data->x86.extended; - len = data->x86.extended_len; + cpuids = data->extended; + len = data->extended_len; i = function - CPUX86_EXTENDED; } @@ -206,65 +204,90 @@ x86DataCpuid(const union cpuData *data, static void -x86DataFree(union cpuData *data) +x86DataFree(struct cpuX86Data *data) { if (data == NULL) return; - VIR_FREE(data->x86.basic); - VIR_FREE(data->x86.extended); + VIR_FREE(data->basic); + VIR_FREE(data->extended); VIR_FREE(data); } static union cpuData * -x86DataCopy(const union cpuData *data) +x86MakeCPUData(struct cpuX86Data **data) +{ + union cpuData *cpuData; + + if (VIR_ALLOC(cpuData) < 0) + return NULL; + + cpuData->x86 = *data; + *data = NULL; + + return cpuData; +} + +static void +x86FreeCPUData(union cpuData *data) +{ + if (!data) + return; + + x86DataFree(data->x86); + VIR_FREE(data); +} + + +static struct cpuX86Data * +x86DataCopy(const struct cpuX86Data *data) { - union cpuData *copy = NULL; + struct cpuX86Data *copy = NULL; size_t i; if (VIR_ALLOC(copy) < 0 - || VIR_ALLOC_N(copy->x86.basic, data->x86.basic_len) < 0 - || VIR_ALLOC_N(copy->x86.extended, data->x86.extended_len) < 0) { + || VIR_ALLOC_N(copy->basic, data->basic_len) < 0 + || VIR_ALLOC_N(copy->extended, data->extended_len) < 0) { x86DataFree(copy); return NULL; } - copy->x86.basic_len = data->x86.basic_len; - for (i = 0; i < data->x86.basic_len; i++) - copy->x86.basic[i] = data->x86.basic[i]; + copy->basic_len = data->basic_len; + for (i = 0; i < data->basic_len; i++) + copy->basic[i] = data->basic[i]; - copy->x86.extended_len = data->x86.extended_len; - for (i = 0; i < data->x86.extended_len; i++) - copy->x86.extended[i] = data->x86.extended[i]; + copy->extended_len = data->extended_len; + for (i = 0; i < data->extended_len; i++) + copy->extended[i] = data->extended[i]; return copy; } static int -x86DataExpand(union cpuData *data, +x86DataExpand(struct cpuX86Data *data, int basic_by, int extended_by) { size_t i; if (basic_by > 0) { - size_t len = data->x86.basic_len; - if (VIR_EXPAND_N(data->x86.basic, data->x86.basic_len, basic_by) < 0) + size_t len = data->basic_len; + if (VIR_EXPAND_N(data->basic, data->basic_len, basic_by) < 0) return -1; for (i = 0; i < basic_by; i++) - data->x86.basic[len + i].function = len + i; + data->basic[len + i].function = len + i; } if (extended_by > 0) { - size_t len = data->x86.extended_len; - if (VIR_EXPAND_N(data->x86.extended, data->x86.extended_len, extended_by) < 0) + size_t len = data->extended_len; + if (VIR_EXPAND_N(data->extended, data->extended_len, extended_by) < 0) return -1; for (i = 0; i < extended_by; i++) - data->x86.extended[len + i].function = len + i + CPUX86_EXTENDED; + data->extended[len + i].function = len + i + CPUX86_EXTENDED; } return 0; @@ -272,7 +295,7 @@ x86DataExpand(union cpuData *data, static int -x86DataAddCpuid(union cpuData *data, +x86DataAddCpuid(struct cpuX86Data *data, const struct cpuX86cpuid *cpuid) { unsigned int basic_by = 0; @@ -282,12 +305,12 @@ x86DataAddCpuid(union cpuData *data, if (cpuid->function < CPUX86_EXTENDED) { pos = cpuid->function; - basic_by = pos + 1 - data->x86.basic_len; - cpuids = &data->x86.basic; + basic_by = pos + 1 - data->basic_len; + cpuids = &data->basic; } else { pos = cpuid->function - CPUX86_EXTENDED; - extended_by = pos + 1 - data->x86.extended_len; - cpuids = &data->x86.extended; + extended_by = pos + 1 - data->extended_len; + cpuids = &data->extended; } if (x86DataExpand(data, basic_by, extended_by) < 0) @@ -300,24 +323,24 @@ x86DataAddCpuid(union cpuData *data, static int -x86DataAdd(union cpuData *data1, - const union cpuData *data2) +x86DataAdd(struct cpuX86Data *data1, + const struct cpuX86Data *data2) { size_t i; if (x86DataExpand(data1, - data2->x86.basic_len - data1->x86.basic_len, - data2->x86.extended_len - data1->x86.extended_len) < 0) + data2->basic_len - data1->basic_len, + data2->extended_len - data1->extended_len) < 0) return -1; - for (i = 0; i < data2->x86.basic_len; i++) { - x86cpuidSetBits(data1->x86.basic + i, - data2->x86.basic + i); + for (i = 0; i < data2->basic_len; i++) { + x86cpuidSetBits(data1->basic + i, + data2->basic + i); } - for (i = 0; i < data2->x86.extended_len; i++) { - x86cpuidSetBits(data1->x86.extended + i, - data2->x86.extended + i); + for (i = 0; i < data2->extended_len; i++) { + x86cpuidSetBits(data1->extended + i, + data2->extended + i); } return 0; @@ -325,29 +348,29 @@ x86DataAdd(union cpuData *data1, static void -x86DataSubtract(union cpuData *data1, - const union cpuData *data2) +x86DataSubtract(struct cpuX86Data *data1, + const struct cpuX86Data *data2) { size_t i; unsigned int len; - len = MIN(data1->x86.basic_len, data2->x86.basic_len); + len = MIN(data1->basic_len, data2->basic_len); for (i = 0; i < len; i++) { - x86cpuidClearBits(data1->x86.basic + i, - data2->x86.basic + i); + x86cpuidClearBits(data1->basic + i, + data2->basic + i); } - len = MIN(data1->x86.extended_len, data2->x86.extended_len); + len = MIN(data1->extended_len, data2->extended_len); for (i = 0; i < len; i++) { - x86cpuidClearBits(data1->x86.extended + i, - data2->x86.extended + i); + x86cpuidClearBits(data1->extended + i, + data2->extended + i); } } static void -x86DataIntersect(union cpuData *data1, - const union cpuData *data2) +x86DataIntersect(struct cpuX86Data *data1, + const struct cpuX86Data *data2) { struct data_iterator iter = DATA_ITERATOR_INIT(data1); struct cpuX86cpuid *cpuid1; @@ -364,7 +387,7 @@ x86DataIntersect(union cpuData *data1, static bool -x86DataIsEmpty(union cpuData *data) +x86DataIsEmpty(struct cpuX86Data *data) { struct data_iterator iter = DATA_ITERATOR_INIT(data); @@ -373,11 +396,11 @@ x86DataIsEmpty(union cpuData *data) static bool -x86DataIsSubset(const union cpuData *data, - const union cpuData *subset) +x86DataIsSubset(const struct cpuX86Data *data, + const struct cpuX86Data *subset) { - struct data_iterator iter = DATA_ITERATOR_INIT((union cpuData *) subset); + struct data_iterator iter = DATA_ITERATOR_INIT((struct cpuX86Data *)subset); const struct cpuX86cpuid *cpuid; const struct cpuX86cpuid *cpuidSubset; @@ -395,7 +418,7 @@ x86DataIsSubset(const union cpuData *data, static int x86DataToCPUFeatures(virCPUDefPtr cpu, int policy, - union cpuData *data, + struct cpuX86Data *data, const struct x86_map *map) { const struct x86_feature *feature = map->features; @@ -415,7 +438,7 @@ x86DataToCPUFeatures(virCPUDefPtr cpu, /* also removes bits corresponding to vendor string from data */ static const struct x86_vendor * -x86DataToVendor(union cpuData *data, +x86DataToVendor(struct cpuX86Data *data, const struct x86_map *map) { const struct x86_vendor *vendor = map->vendors; @@ -435,13 +458,13 @@ x86DataToVendor(union cpuData *data, static virCPUDefPtr -x86DataToCPU(const union cpuData *data, +x86DataToCPU(const struct cpuX86Data *data, const struct x86_model *model, const struct x86_map *map) { virCPUDefPtr cpu; - union cpuData *copy = NULL; - union cpuData *modelData = NULL; + struct cpuX86Data *copy = NULL; + struct cpuX86Data *modelData = NULL; const struct x86_vendor *vendor; if (VIR_ALLOC(cpu) < 0 || @@ -616,7 +639,7 @@ x86FeatureFind(const struct x86_map *map, static char * x86FeatureNames(const struct x86_map *map, const char *separator, - union cpuData *data) + struct cpuX86Data *data) { virBuffer ret = VIR_BUFFER_INITIALIZER; bool first = true; @@ -1103,7 +1126,7 @@ error: /* A helper macro to exit the cpu computation function without writing * redundant code: * MSG: error message - * CPU_DEF: a union cpuData pointer with flags that are conflicting + * CPU_DEF: a struct cpuX86Data pointer with flags that are conflicting * RET: return code to set * * This macro generates the error string outputs it into logs. @@ -1228,6 +1251,8 @@ x86Compute(virCPUDefPtr host, } if (guest != NULL) { + struct cpuX86Data *guestData; + if ((guest_model = x86ModelCopy(host_model)) == NULL) goto error; @@ -1240,8 +1265,11 @@ x86Compute(virCPUDefPtr host, x86DataSubtract(guest_model->data, cpu_disable->data); - if ((*guest = x86DataCopy(guest_model->data)) == NULL) + if (!(guestData = x86DataCopy(guest_model->data)) || + !(*guest = x86MakeCPUData(&guestData))) { + x86DataFree(guestData); goto error; + } } out: @@ -1284,7 +1312,7 @@ x86GuestData(virCPUDefPtr host, static int x86Decode(virCPUDefPtr cpu, - const union cpuData *data, + const struct cpuX86Data *data, const char **models, unsigned int nmodels, const char *preferred) @@ -1383,14 +1411,24 @@ out: return ret; } +static int +x86DecodeCPUData(virCPUDefPtr cpu, + const union cpuData *data, + const char **models, + unsigned int nmodels, + const char *preferred) +{ + return x86Decode(cpu, data->x86, models, nmodels, preferred); +} -static union cpuData * + +static struct cpuX86Data * x86EncodePolicy(const virCPUDefPtr cpu, const struct x86_map *map, enum virCPUFeaturePolicy policy) { struct x86_model *model; - union cpuData *data = NULL; + struct cpuX86Data *data = NULL; if (!(model = x86ModelFromCPU(cpu, map, policy))) return NULL; @@ -1413,14 +1451,27 @@ x86Encode(const virCPUDefPtr cpu, union cpuData **vendor) { struct x86_map *map = NULL; - union cpuData *data_forced = NULL; - union cpuData *data_required = NULL; - union cpuData *data_optional = NULL; - union cpuData *data_disabled = NULL; - union cpuData *data_forbidden = NULL; - union cpuData *data_vendor = NULL; + struct cpuX86Data *data_forced = NULL; + struct cpuX86Data *data_required = NULL; + struct cpuX86Data *data_optional = NULL; + struct cpuX86Data *data_disabled = NULL; + struct cpuX86Data *data_forbidden = NULL; + struct cpuX86Data *data_vendor = NULL; int ret = -1; + if (forced) + *forced = NULL; + if (required) + *required = NULL; + if (optional) + *optional = NULL; + if (disabled) + *disabled = NULL; + if (forbidden) + *forbidden = NULL; + if (vendor) + *vendor = NULL; + if ((map = x86LoadMap()) == NULL) goto error; @@ -1470,18 +1521,24 @@ x86Encode(const virCPUDefPtr cpu, } } - if (forced) - *forced = data_forced; - if (required) - *required = data_required; - if (optional) - *optional = data_optional; - if (disabled) - *disabled = data_disabled; - if (forbidden) - *forbidden = data_forbidden; - if (vendor) - *vendor = data_vendor; + if (forced && + !(*forced = x86MakeCPUData(&data_forced))) + goto error; + if (required && + !(*required = x86MakeCPUData(&data_required))) + goto error; + if (optional && + !(*optional = x86MakeCPUData(&data_optional))) + goto error; + if (disabled && + !(*disabled = x86MakeCPUData(&data_disabled))) + goto error; + if (forbidden && + !(*forbidden = x86MakeCPUData(&data_forbidden))) + goto error; + if (vendor && + !(*vendor = x86MakeCPUData(&data_vendor))) + goto error; ret = 0; @@ -1497,6 +1554,18 @@ error: x86DataFree(data_disabled); x86DataFree(data_forbidden); x86DataFree(data_vendor); + if (forced) + x86FreeCPUData(*forced); + if (required) + x86FreeCPUData(*required); + if (optional) + x86FreeCPUData(*optional); + if (disabled) + x86FreeCPUData(*disabled); + if (forbidden) + x86FreeCPUData(*forbidden); + if (vendor) + x86FreeCPUData(*vendor); goto cleanup; } @@ -1562,21 +1631,25 @@ cpuidSet(uint32_t base, struct cpuX86cpuid **set) static union cpuData * x86NodeData(void) { - union cpuData *data; + union cpuData *cpuData = NULL; + struct cpuX86Data *data; int ret; if (VIR_ALLOC(data) < 0) return NULL; - if ((ret = cpuidSet(CPUX86_BASIC, &data->x86.basic)) < 0) + if ((ret = cpuidSet(CPUX86_BASIC, &data->basic)) < 0) goto error; - data->x86.basic_len = ret; + data->basic_len = ret; - if ((ret = cpuidSet(CPUX86_EXTENDED, &data->x86.extended)) < 0) + if ((ret = cpuidSet(CPUX86_EXTENDED, &data->extended)) < 0) goto error; - data->x86.extended_len = ret; + data->extended_len = ret; - return data; + if (!(cpuData = x86MakeCPUData(&data))) + goto error; + + return cpuData; error: x86DataFree(data); @@ -1812,7 +1885,7 @@ static int x86HasFeature(const union cpuData *data, if (!(feature = x86FeatureFind(map, name))) goto cleanup; - ret = x86DataIsSubset(data, feature->data) ? 1 : 0; + ret = x86DataIsSubset(data->x86, feature->data) ? 1 : 0; cleanup: x86MapFree(map); @@ -1824,9 +1897,9 @@ struct cpuArchDriver cpuDriverX86 = { .arch = archs, .narch = ARRAY_CARDINALITY(archs), .compare = x86Compare, - .decode = x86Decode, + .decode = x86DecodeCPUData, .encode = x86Encode, - .free = x86DataFree, + .free = x86FreeCPUData, #if HAVE_CPUID .nodeData = x86NodeData, #else -- 1.8.3.2
On 07/19/13 19:16, Jiri Denemark wrote:
--- src/cpu/cpu.h | 2 +- src/cpu/cpu_x86.c | 271 ++++++++++++++++++++++++++++++++++-------------------- 2 files changed, 173 insertions(+), 100 deletions(-)
ACK, mostly mechanical. Peter
--- src/cpu/cpu.c | 22 +++++++++---------- src/cpu/cpu.h | 51 ++++++++++++++++++++++---------------------- src/cpu/cpu_arm.c | 8 +++---- src/cpu/cpu_powerpc.c | 8 +++---- src/cpu/cpu_s390.c | 8 +++---- src/cpu/cpu_x86.c | 30 +++++++++++++------------- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_command.c | 4 ++-- src/vmware/vmware_conf.c | 2 +- tests/cputest.c | 4 ++-- 10 files changed, 70 insertions(+), 69 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index f3bc76f..a2d797d 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -130,7 +130,7 @@ cpuCompare(virCPUDefPtr host, int cpuDecode(virCPUDefPtr cpu, - const union cpuData *data, + const virCPUDataPtr data, const char **models, unsigned int nmodels, const char *preferred) @@ -174,12 +174,12 @@ cpuDecode(virCPUDefPtr cpu, int cpuEncode(virArch arch, const virCPUDefPtr cpu, - union cpuData **forced, - union cpuData **required, - union cpuData **optional, - union cpuData **disabled, - union cpuData **forbidden, - union cpuData **vendor) + virCPUDataPtr *forced, + virCPUDataPtr *required, + virCPUDataPtr *optional, + virCPUDataPtr *disabled, + virCPUDataPtr *forbidden, + virCPUDataPtr *vendor) { struct cpuArchDriver *driver; @@ -205,7 +205,7 @@ cpuEncode(virArch arch, void cpuDataFree(virArch arch, - union cpuData *data) + virCPUDataPtr data) { struct cpuArchDriver *driver; @@ -228,7 +228,7 @@ cpuDataFree(virArch arch, } -union cpuData * +virCPUDataPtr cpuNodeData(virArch arch) { struct cpuArchDriver *driver; @@ -252,7 +252,7 @@ cpuNodeData(virArch arch) virCPUCompareResult cpuGuestData(virCPUDefPtr host, virCPUDefPtr guest, - union cpuData **data, + virCPUDataPtr *data, char **msg) { struct cpuArchDriver *driver; @@ -420,7 +420,7 @@ cpuUpdate(virCPUDefPtr guest, int cpuHasFeature(virArch arch, - const union cpuData *data, + const virCPUDataPtr data, const char *feature) { struct cpuArchDriver *driver; diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index b2c02db..1c67489 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -32,11 +32,12 @@ # include "cpu_ppc_data.h" -union cpuData { +typedef union _virCPUData virCPUData; +typedef virCPUData *virCPUDataPtr; +union _virCPUData { struct cpuX86Data *x86; - /* generic driver needs no data */ - /* PowerPC driver need data*/ struct cpuPPCData ppc; + /* generic driver needs no data */ }; @@ -46,30 +47,30 @@ typedef virCPUCompareResult typedef int (*cpuArchDecode) (virCPUDefPtr cpu, - const union cpuData *data, + const virCPUDataPtr data, const char **models, unsigned int nmodels, const char *preferred); typedef int (*cpuArchEncode) (const virCPUDefPtr cpu, - union cpuData **forced, - union cpuData **required, - union cpuData **optional, - union cpuData **disabled, - union cpuData **forbidden, - union cpuData **vendor); + virCPUDataPtr *forced, + virCPUDataPtr *required, + virCPUDataPtr *optional, + virCPUDataPtr *disabled, + virCPUDataPtr *forbidden, + virCPUDataPtr *vendor); typedef void -(*cpuArchDataFree) (union cpuData *data); +(*cpuArchDataFree) (virCPUDataPtr data); -typedef union cpuData * +typedef virCPUDataPtr (*cpuArchNodeData) (void); typedef virCPUCompareResult (*cpuArchGuestData) (virCPUDefPtr host, virCPUDefPtr guest, - union cpuData **data, + virCPUDataPtr *data, char **message); typedef virCPUDefPtr @@ -83,7 +84,7 @@ typedef int const virCPUDefPtr host); typedef int -(*cpuArchHasFeature) (const union cpuData *data, +(*cpuArchHasFeature) (const virCPUDataPtr data, const char *feature); @@ -113,7 +114,7 @@ cpuCompare (virCPUDefPtr host, extern int cpuDecode (virCPUDefPtr cpu, - const union cpuData *data, + const virCPUDataPtr data, const char **models, unsigned int nmodels, const char *preferred); @@ -121,24 +122,24 @@ cpuDecode (virCPUDefPtr cpu, extern int cpuEncode (virArch arch, const virCPUDefPtr cpu, - union cpuData **forced, - union cpuData **required, - union cpuData **optional, - union cpuData **disabled, - union cpuData **forbidden, - union cpuData **vendor); + virCPUDataPtr *forced, + virCPUDataPtr *required, + virCPUDataPtr *optional, + virCPUDataPtr *disabled, + virCPUDataPtr *forbidden, + virCPUDataPtr *vendor); extern void cpuDataFree (virArch arch, - union cpuData *data); + virCPUDataPtr data); -extern union cpuData * +extern virCPUDataPtr cpuNodeData (virArch arch); extern virCPUCompareResult cpuGuestData(virCPUDefPtr host, virCPUDefPtr guest, - union cpuData **data, + virCPUDataPtr *data, char **msg); extern char * @@ -159,7 +160,7 @@ cpuUpdate (virCPUDefPtr guest, extern int cpuHasFeature(virArch arch, - const union cpuData *data, + const virCPUDataPtr data, const char *feature); diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 0748267..25e25ba 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -30,10 +30,10 @@ static const virArch archs[] = { VIR_ARCH_ARMV7L }; -static union cpuData * +static virCPUDataPtr ArmNodeData(void) { - union cpuData *data; + virCPUDataPtr data; ignore_value(VIR_ALLOC(data)); return data; @@ -41,7 +41,7 @@ ArmNodeData(void) static int ArmDecode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, - const union cpuData *data ATTRIBUTE_UNUSED, + const virCPUDataPtr data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, const char *preferred ATTRIBUTE_UNUSED) @@ -50,7 +50,7 @@ ArmDecode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, } static void -ArmDataFree(union cpuData *data) +ArmDataFree(virCPUDataPtr data) { VIR_FREE(data); } diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index 8bac133..0d319f4 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -301,7 +301,7 @@ ppcCompare(virCPUDefPtr host, static int ppcDecode(virCPUDefPtr cpu, - const union cpuData *data, + const virCPUDataPtr data, const char **models, unsigned int nmodels, const char *preferred ATTRIBUTE_UNUSED) @@ -342,7 +342,7 @@ cleanup: static void -ppcDataFree(union cpuData *data) +ppcDataFree(virCPUDataPtr data) { if (data == NULL) return; @@ -351,10 +351,10 @@ ppcDataFree(union cpuData *data) } #if defined(__powerpc__) || defined(__powerpc64__) -static union cpuData * +static virCPUDataPtr ppcNodeData(void) { - union cpuData *data; + virCPUDataPtr data; if (VIR_ALLOC(data) < 0) return NULL; diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c index 0d578a4..cbfae42 100644 --- a/src/cpu/cpu_s390.c +++ b/src/cpu/cpu_s390.c @@ -31,10 +31,10 @@ static const virArch archs[] = { VIR_ARCH_S390, VIR_ARCH_S390X }; -static union cpuData * +static virCPUDataPtr s390NodeData(void) { - union cpuData *data; + virCPUDataPtr data; if (VIR_ALLOC(data) < 0) return NULL; @@ -45,7 +45,7 @@ s390NodeData(void) static int s390Decode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, - const union cpuData *data ATTRIBUTE_UNUSED, + const virCPUDataPtr data ATTRIBUTE_UNUSED, const char **models ATTRIBUTE_UNUSED, unsigned int nmodels ATTRIBUTE_UNUSED, const char *preferred ATTRIBUTE_UNUSED) @@ -54,7 +54,7 @@ s390Decode(virCPUDefPtr cpu ATTRIBUTE_UNUSED, } static void -s390DataFree(union cpuData *data) +s390DataFree(virCPUDataPtr data) { VIR_FREE(data); } diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index c6e78c5..75ad647 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -215,10 +215,10 @@ x86DataFree(struct cpuX86Data *data) } -static union cpuData * +static virCPUDataPtr x86MakeCPUData(struct cpuX86Data **data) { - union cpuData *cpuData; + virCPUDataPtr cpuData; if (VIR_ALLOC(cpuData) < 0) return NULL; @@ -230,7 +230,7 @@ x86MakeCPUData(struct cpuX86Data **data) } static void -x86FreeCPUData(union cpuData *data) +x86FreeCPUData(virCPUDataPtr data) { if (!data) return; @@ -1151,7 +1151,7 @@ error: static virCPUCompareResult x86Compute(virCPUDefPtr host, virCPUDefPtr cpu, - union cpuData **guest, + virCPUDataPtr *guest, char **message) { struct x86_map *map = NULL; @@ -1303,7 +1303,7 @@ x86Compare(virCPUDefPtr host, static virCPUCompareResult x86GuestData(virCPUDefPtr host, virCPUDefPtr guest, - union cpuData **data, + virCPUDataPtr *data, char **message) { return x86Compute(host, guest, data, message); @@ -1413,7 +1413,7 @@ out: static int x86DecodeCPUData(virCPUDefPtr cpu, - const union cpuData *data, + const virCPUDataPtr data, const char **models, unsigned int nmodels, const char *preferred) @@ -1443,12 +1443,12 @@ x86EncodePolicy(const virCPUDefPtr cpu, static int x86Encode(const virCPUDefPtr cpu, - union cpuData **forced, - union cpuData **required, - union cpuData **optional, - union cpuData **disabled, - union cpuData **forbidden, - union cpuData **vendor) + virCPUDataPtr *forced, + virCPUDataPtr *required, + virCPUDataPtr *optional, + virCPUDataPtr *disabled, + virCPUDataPtr *forbidden, + virCPUDataPtr *vendor) { struct x86_map *map = NULL; struct cpuX86Data *data_forced = NULL; @@ -1628,10 +1628,10 @@ cpuidSet(uint32_t base, struct cpuX86cpuid **set) } -static union cpuData * +static virCPUDataPtr x86NodeData(void) { - union cpuData *cpuData = NULL; + virCPUDataPtr cpuData = NULL; struct cpuX86Data *data; int ret; @@ -1872,7 +1872,7 @@ x86Update(virCPUDefPtr guest, return -1; } -static int x86HasFeature(const union cpuData *data, +static int x86HasFeature(const virCPUDataPtr data, const char *name) { struct x86_map *map; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 16a3664..8d40f25 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -832,7 +832,7 @@ virQEMUCapsInitCPU(virCapsPtr caps, virArch arch) { virCPUDefPtr cpu = NULL; - union cpuData *data = NULL; + virCPUDataPtr data = NULL; virNodeInfo nodeinfo; int ret = -1; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8c58a7c..30b7bc0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5648,7 +5648,7 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, size_t ncpus = 0; char **cpus = NULL; const char *default_model; - union cpuData *data = NULL; + virCPUDataPtr data = NULL; bool have_cpu = false; char *compare_msg = NULL; int ret = -1; @@ -9902,7 +9902,7 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if (dom->os.arch == VIR_ARCH_X86_64) { bool is_32bit = false; if (cpu) { - union cpuData *cpuData = NULL; + virCPUDataPtr cpuData = NULL; if (cpuEncode(VIR_ARCH_X86_64, cpu, NULL, &cpuData, NULL, NULL, NULL, NULL) < 0) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index ed99e59..bb88899 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -56,7 +56,7 @@ vmwareCapsInit(void) virCapsPtr caps = NULL; virCapsGuestPtr guest = NULL; virCPUDefPtr cpu = NULL; - union cpuData *data = NULL; + virCPUDataPtr data = NULL; if ((caps = virCapabilitiesNew(virArchFromHost(), 0, 0)) == NULL) diff --git a/tests/cputest.c b/tests/cputest.c index d08550d..6708bcf 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -261,7 +261,7 @@ cpuTestGuestData(const void *arg) virCPUDefPtr host = NULL; virCPUDefPtr cpu = NULL; virCPUDefPtr guest = NULL; - union cpuData *guestData = NULL; + virCPUDataPtr guestData = NULL; virCPUCompareResult cmpResult; virBuffer buf = VIR_BUFFER_INITIALIZER; char *result = NULL; @@ -417,7 +417,7 @@ cpuTestHasFeature(const void *arg) const struct data *data = arg; int ret = -1; virCPUDefPtr host = NULL; - union cpuData *hostData = NULL; + virCPUDataPtr hostData = NULL; int result; if (!(host = cpuTestLoadXML(data->arch, data->host))) -- 1.8.3.2
On 07/19/13 19:16, Jiri Denemark wrote:
--- src/cpu/cpu.c | 22 +++++++++---------- src/cpu/cpu.h | 51 ++++++++++++++++++++++---------------------- src/cpu/cpu_arm.c | 8 +++---- src/cpu/cpu_powerpc.c | 8 +++---- src/cpu/cpu_s390.c | 8 +++---- src/cpu/cpu_x86.c | 30 +++++++++++++------------- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_command.c | 4 ++-- src/vmware/vmware_conf.c | 2 +- tests/cputest.c | 4 ++-- 10 files changed, 70 insertions(+), 69 deletions(-)
ACK. Mechanical. Peter
--- src/cpu/cpu.c | 11 +++++------ src/cpu/cpu.h | 19 +++++++++++-------- src/cpu/cpu_powerpc.c | 4 ++-- src/cpu/cpu_x86.c | 34 ++++++++++++++++++++-------------- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_command.c | 5 ++--- src/vmware/vmware_conf.c | 3 +-- tests/cputest.c | 6 ++---- 8 files changed, 44 insertions(+), 40 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index a2d797d..a4e1840 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -198,29 +198,28 @@ cpuEncode(virArch arch, return -1; } - return driver->encode(cpu, forced, required, + return driver->encode(arch, cpu, forced, required, optional, disabled, forbidden, vendor); } void -cpuDataFree(virArch arch, - virCPUDataPtr data) +cpuDataFree(virCPUDataPtr data) { struct cpuArchDriver *driver; - VIR_DEBUG("arch=%s, data=%p", virArchToString(arch), data); + VIR_DEBUG("data=%p", data); if (data == NULL) return; - if ((driver = cpuGetSubDriver(arch)) == NULL) + if ((driver = cpuGetSubDriver(data->arch)) == NULL) return; if (driver->free == NULL) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot free CPU data for %s architecture"), - virArchToString(arch)); + virArchToString(data->arch)); return; } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 1c67489..1feca82 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -32,12 +32,15 @@ # include "cpu_ppc_data.h" -typedef union _virCPUData virCPUData; +typedef struct _virCPUData virCPUData; typedef virCPUData *virCPUDataPtr; -union _virCPUData { - struct cpuX86Data *x86; - struct cpuPPCData ppc; - /* generic driver needs no data */ +struct _virCPUData { + virArch arch; + union { + struct cpuX86Data *x86; + struct cpuPPCData ppc; + /* generic driver needs no data */ + } data; }; @@ -53,7 +56,8 @@ typedef int const char *preferred); typedef int -(*cpuArchEncode) (const virCPUDefPtr cpu, +(*cpuArchEncode) (virArch arch, + const virCPUDefPtr cpu, virCPUDataPtr *forced, virCPUDataPtr *required, virCPUDataPtr *optional, @@ -130,8 +134,7 @@ cpuEncode (virArch arch, virCPUDataPtr *vendor); extern void -cpuDataFree (virArch arch, - virCPUDataPtr data); +cpuDataFree (virCPUDataPtr data); extern virCPUDataPtr cpuNodeData (virArch arch); diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c index 0d319f4..62437d3 100644 --- a/src/cpu/cpu_powerpc.c +++ b/src/cpu/cpu_powerpc.c @@ -313,10 +313,10 @@ ppcDecode(virCPUDefPtr cpu, if (data == NULL || (map = ppcLoadMap()) == NULL) return -1; - if (!(model = ppcModelFindPVR(map, data->ppc.pvr))) { + if (!(model = ppcModelFindPVR(map, data->data.ppc.pvr))) { virReportError(VIR_ERR_OPERATION_FAILED, _("Cannot find CPU model with PVR 0x%08x"), - data->ppc.pvr); + data->data.ppc.pvr); goto cleanup; } diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 75ad647..4564bd4 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -216,14 +216,15 @@ x86DataFree(struct cpuX86Data *data) static virCPUDataPtr -x86MakeCPUData(struct cpuX86Data **data) +x86MakeCPUData(virArch arch, struct cpuX86Data **data) { virCPUDataPtr cpuData; if (VIR_ALLOC(cpuData) < 0) return NULL; - cpuData->x86 = *data; + cpuData->arch = arch; + cpuData->data.x86 = *data; *data = NULL; return cpuData; @@ -235,7 +236,7 @@ x86FreeCPUData(virCPUDataPtr data) if (!data) return; - x86DataFree(data->x86); + x86DataFree(data->data.x86); VIR_FREE(data); } @@ -1165,6 +1166,7 @@ x86Compute(virCPUDefPtr host, struct x86_model *guest_model = NULL; virCPUCompareResult ret; enum compare_result result; + virArch arch; size_t i; if (cpu->arch != VIR_ARCH_NONE) { @@ -1187,6 +1189,9 @@ x86Compute(virCPUDefPtr host, goto error; return VIR_CPU_COMPARE_INCOMPATIBLE; } + arch = cpu->arch; + } else { + arch = host->arch; } if (cpu->vendor && @@ -1266,7 +1271,7 @@ x86Compute(virCPUDefPtr host, x86DataSubtract(guest_model->data, cpu_disable->data); if (!(guestData = x86DataCopy(guest_model->data)) || - !(*guest = x86MakeCPUData(&guestData))) { + !(*guest = x86MakeCPUData(arch, &guestData))) { x86DataFree(guestData); goto error; } @@ -1418,7 +1423,7 @@ x86DecodeCPUData(virCPUDefPtr cpu, unsigned int nmodels, const char *preferred) { - return x86Decode(cpu, data->x86, models, nmodels, preferred); + return x86Decode(cpu, data->data.x86, models, nmodels, preferred); } @@ -1442,7 +1447,8 @@ x86EncodePolicy(const virCPUDefPtr cpu, static int -x86Encode(const virCPUDefPtr cpu, +x86Encode(virArch arch, + const virCPUDefPtr cpu, virCPUDataPtr *forced, virCPUDataPtr *required, virCPUDataPtr *optional, @@ -1522,22 +1528,22 @@ x86Encode(const virCPUDefPtr cpu, } if (forced && - !(*forced = x86MakeCPUData(&data_forced))) + !(*forced = x86MakeCPUData(arch, &data_forced))) goto error; if (required && - !(*required = x86MakeCPUData(&data_required))) + !(*required = x86MakeCPUData(arch, &data_required))) goto error; if (optional && - !(*optional = x86MakeCPUData(&data_optional))) + !(*optional = x86MakeCPUData(arch, &data_optional))) goto error; if (disabled && - !(*disabled = x86MakeCPUData(&data_disabled))) + !(*disabled = x86MakeCPUData(arch, &data_disabled))) goto error; if (forbidden && - !(*forbidden = x86MakeCPUData(&data_forbidden))) + !(*forbidden = x86MakeCPUData(arch, &data_forbidden))) goto error; if (vendor && - !(*vendor = x86MakeCPUData(&data_vendor))) + !(*vendor = x86MakeCPUData(arch, &data_vendor))) goto error; ret = 0; @@ -1646,7 +1652,7 @@ x86NodeData(void) goto error; data->extended_len = ret; - if (!(cpuData = x86MakeCPUData(&data))) + if (!(cpuData = x86MakeCPUData(virArchFromHost(), &data))) goto error; return cpuData; @@ -1885,7 +1891,7 @@ static int x86HasFeature(const virCPUDataPtr data, if (!(feature = x86FeatureFind(map, name))) goto cleanup; - ret = x86DataIsSubset(data->x86, feature->data) ? 1 : 0; + ret = x86DataIsSubset(data->data.x86, feature->data) ? 1 : 0; cleanup: x86MapFree(map); diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 8d40f25..5dc3c9e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -857,7 +857,7 @@ virQEMUCapsInitCPU(virCapsPtr caps, ret = 0; cleanup: - cpuDataFree(arch, data); + cpuDataFree(data); return ret; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 30b7bc0..a7c978f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5853,8 +5853,7 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, cleanup: VIR_FREE(compare_msg); - if (host) - cpuDataFree(host->arch, data); + cpuDataFree(data); virCPUDefFree(guest); virCPUDefFree(cpu); virObjectUnref(caps); @@ -9909,7 +9908,7 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, goto cleanup; is_32bit = (cpuHasFeature(VIR_ARCH_X86_64, cpuData, "lm") != 1); - cpuDataFree(VIR_ARCH_X86_64, cpuData); + cpuDataFree(cpuData); } else if (model) { is_32bit = STREQ(model, "qemu32"); } diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index bb88899..6ef02a9 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -115,8 +115,7 @@ vmwareCapsInit(void) cleanup: virCPUDefFree(cpu); - if (caps) - cpuDataFree(caps->host.arch, data); + cpuDataFree(data); return caps; diff --git a/tests/cputest.c b/tests/cputest.c index 6708bcf..d3865c4 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -308,8 +308,7 @@ cpuTestGuestData(const void *arg) cleanup: VIR_FREE(result); - if (host) - cpuDataFree(host->arch, guestData); + cpuDataFree(guestData); virCPUDefFree(host); virCPUDefFree(cpu); virCPUDefFree(guest); @@ -445,8 +444,7 @@ cpuTestHasFeature(const void *arg) ret = 0; cleanup: - if (host) - cpuDataFree(host->arch, hostData); + cpuDataFree(hostData); virCPUDefFree(host); return ret; } -- 1.8.3.2
On 07/19/13 19:16, Jiri Denemark wrote:
--- src/cpu/cpu.c | 11 +++++------ src/cpu/cpu.h | 19 +++++++++++-------- src/cpu/cpu_powerpc.c | 4 ++-- src/cpu/cpu_x86.c | 34 ++++++++++++++++++++-------------- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_command.c | 5 ++--- src/vmware/vmware_conf.c | 3 +-- tests/cputest.c | 6 ++---- 8 files changed, 44 insertions(+), 40 deletions(-)
ACK. Peter
--- This patch should be squashed in 3/3. src/cpu/cpu.c | 10 ++++------ src/cpu/cpu.h | 3 +-- src/qemu/qemu_command.c | 4 ++-- src/vmware/vmware_conf.c | 6 +++--- tests/cputest.c | 2 +- 5 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index a4e1840..4124354 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -418,22 +418,20 @@ cpuUpdate(virCPUDefPtr guest, } int -cpuHasFeature(virArch arch, - const virCPUDataPtr data, +cpuHasFeature(const virCPUDataPtr data, const char *feature) { struct cpuArchDriver *driver; - VIR_DEBUG("arch=%s, data=%p, feature=%s", - virArchToString(arch), data, feature); + VIR_DEBUG("data=%p, feature=%s", data, feature); - if ((driver = cpuGetSubDriver(arch)) == NULL) + if ((driver = cpuGetSubDriver(data->arch)) == NULL) return -1; if (driver->hasFeature == NULL) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot check guest CPU data for %s architecture"), - virArchToString(arch)); + virArchToString(data->arch)); return -1; } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 1feca82..4003435 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -162,8 +162,7 @@ cpuUpdate (virCPUDefPtr guest, const virCPUDefPtr host); extern int -cpuHasFeature(virArch arch, - const virCPUDataPtr data, +cpuHasFeature(const virCPUDataPtr data, const char *feature); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a7c978f..602bdec 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5712,7 +5712,7 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, /* Only 'svm' requires --enable-nesting. The nested * 'vmx' patches now simply hook off the CPU features */ - hasSVM = cpuHasFeature(host->arch, data, "svm"); + hasSVM = cpuHasFeature(data, "svm"); if (hasSVM < 0) goto cleanup; *hasHwVirt = hasSVM > 0 ? true : false; @@ -9907,7 +9907,7 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, NULL, NULL, NULL, NULL) < 0) goto cleanup; - is_32bit = (cpuHasFeature(VIR_ARCH_X86_64, cpuData, "lm") != 1); + is_32bit = (cpuHasFeature(cpuData, "lm") != 1); cpuDataFree(cpuData); } else if (model) { is_32bit = STREQ(model, "qemu32"); diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 6ef02a9..7734872 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -97,9 +97,9 @@ vmwareCapsInit(void) * - Host CPU is x86_64 with virtualization extensions */ if (caps->host.arch == VIR_ARCH_X86_64 || - (cpuHasFeature(caps->host.arch, data, "lm") && - (cpuHasFeature(caps->host.arch, data, "vmx") || - cpuHasFeature(caps->host.arch, data, "svm")))) { + (cpuHasFeature(data, "lm") && + (cpuHasFeature(data, "vmx") || + cpuHasFeature(data, "svm")))) { if ((guest = virCapabilitiesAddGuest(caps, "hvm", diff --git a/tests/cputest.c b/tests/cputest.c index 803e395..2e5f0cd 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -426,7 +426,7 @@ cpuTestHasFeature(const void *arg) NULL, NULL, NULL, NULL) < 0) goto cleanup; - result = cpuHasFeature(host->arch, hostData, data->name); + result = cpuHasFeature(hostData, data->name); if (data->result == -1) virResetLastError(); -- 1.8.3.2
--- src/vmware/vmware_conf.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 7734872..23da92d 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -80,10 +80,7 @@ vmwareCapsInit(void) if (VIR_ALLOC(cpu) < 0) goto error; - if (!(cpu->arch = caps->host.arch)) { - virReportOOMError(); - goto error; - } + cpu->arch = caps->host.arch; cpu->type = VIR_CPU_TYPE_HOST; if (!(data = cpuNodeData(cpu->arch)) -- 1.8.3.2
On Mon, Jul 22, 2013 at 11:30:33 +0200, Peter Krempa wrote:
On 07/22/13 00:18, Jiri Denemark wrote:
--- src/vmware/vmware_conf.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
ACK.
Thanks, I pushed this series. Jirka
participants (2)
-
Jiri Denemark -
Peter Krempa