[libvirt] [PATCH 00/17] Cleanup CPU driver code a bit
Jiri Denemark (17): cpu_x86: Rename struct x86_vendor cpu_x86: Rename struct x86_feature cpu_x86: Rename struct x86_kvm_feature cpu_x86: Rename struct x86_model cpu_x86: Rename struct x86_map cpu_x86: Rename enum compare_result cpu_x86: Rename struct virCPUx86DataIterator cpu_x86: Compare CPU candidates in a separate function cpu_x86: Rename cleanup labels cpu_x86: Use for loop in x86Decode cpu_x86: Remove comparisons to NULL cpu_x86: Simplify insertions into a linked list cpu_x86: Don't ignore parsing errors in x86VendorLoad cpu_x86: Don't ignore parsing errors in x86FeatureLoad cpu_x86: Don't ignore parsing errors in x86ModelLoad cpu_x86: Check vendor early cpu: Properly report errors when parsing CPU map XML src/cpu/cpu_map.c | 11 +- src/cpu/cpu_x86.c | 564 +++++++++++++++++++++++++++--------------------------- 2 files changed, 287 insertions(+), 288 deletions(-) -- 2.8.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 6ee7ff9..f6fa9e7 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -44,11 +44,13 @@ static const virCPUx86CPUID cpuidNull = { 0, 0, 0, 0, 0 }; static const virArch archs[] = { VIR_ARCH_I686, VIR_ARCH_X86_64 }; -struct x86_vendor { +typedef struct _virCPUx86Vendor virCPUx86Vendor; +typedef virCPUx86Vendor *virCPUx86VendorPtr; +struct _virCPUx86Vendor { char *name; virCPUx86CPUID cpuid; - struct x86_vendor *next; + virCPUx86VendorPtr next; }; struct x86_feature { @@ -87,14 +89,14 @@ static const struct x86_kvm_feature x86_kvm_features[] = struct x86_model { char *name; - const struct x86_vendor *vendor; + virCPUx86VendorPtr vendor; virCPUx86Data *data; struct x86_model *next; }; struct x86_map { - struct x86_vendor *vendors; + virCPUx86VendorPtr vendors; struct x86_feature *features; struct x86_model *models; struct x86_feature *migrate_blockers; @@ -418,11 +420,11 @@ x86DataToCPUFeatures(virCPUDefPtr cpu, /* also removes bits corresponding to vendor string from data */ -static const struct x86_vendor * +static virCPUx86VendorPtr x86DataToVendor(virCPUx86Data *data, const struct x86_map *map) { - const struct x86_vendor *vendor = map->vendors; + virCPUx86VendorPtr vendor = map->vendors; virCPUx86CPUID *cpuid; while (vendor) { @@ -446,7 +448,7 @@ x86DataToCPU(const virCPUx86Data *data, virCPUDefPtr cpu; virCPUx86Data *copy = NULL; virCPUx86Data *modelData = NULL; - const struct x86_vendor *vendor; + virCPUx86VendorPtr vendor; if (VIR_ALLOC(cpu) < 0 || VIR_STRDUP(cpu->model, model->name) < 0 || @@ -481,7 +483,7 @@ x86DataToCPU(const virCPUx86Data *data, static void -x86VendorFree(struct x86_vendor *vendor) +x86VendorFree(virCPUx86VendorPtr vendor) { if (!vendor) return; @@ -491,11 +493,11 @@ x86VendorFree(struct x86_vendor *vendor) } -static struct x86_vendor * +static virCPUx86VendorPtr x86VendorFind(const struct x86_map *map, const char *name) { - struct x86_vendor *vendor; + virCPUx86VendorPtr vendor; vendor = map->vendors; while (vendor) { @@ -513,7 +515,7 @@ static int x86VendorLoad(xmlXPathContextPtr ctxt, struct x86_map *map) { - struct x86_vendor *vendor = NULL; + virCPUx86VendorPtr vendor = NULL; char *string = NULL; int ret = 0; @@ -1136,7 +1138,7 @@ x86MapFree(struct x86_map *map) } while (map->vendors != NULL) { - struct x86_vendor *vendor = map->vendors; + virCPUx86VendorPtr vendor = map->vendors; map->vendors = vendor->next; x86VendorFree(vendor); } @@ -1785,7 +1787,7 @@ x86Encode(virArch arch, } if (vendor) { - const struct x86_vendor *v = NULL; + virCPUx86VendorPtr v = NULL; if (cpu->vendor && !(v = x86VendorFind(map, cpu->vendor))) { virReportError(VIR_ERR_OPERATION_FAILED, @@ -1939,7 +1941,7 @@ x86Baseline(virCPUDefPtr *cpus, struct x86_model *base_model = NULL; virCPUDefPtr cpu = NULL; size_t i; - const struct x86_vendor *vendor = NULL; + virCPUx86VendorPtr vendor = NULL; struct x86_model *model = NULL; bool outputVendor = true; const char *modelName; -- 2.8.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 56 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index f6fa9e7..29680b4 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -53,11 +53,13 @@ struct _virCPUx86Vendor { virCPUx86VendorPtr next; }; -struct x86_feature { +typedef struct _virCPUx86Feature virCPUx86Feature; +typedef virCPUx86Feature *virCPUx86FeaturePtr; +struct _virCPUx86Feature { char *name; virCPUx86Data *data; - struct x86_feature *next; + virCPUx86FeaturePtr next; }; struct x86_kvm_feature { @@ -97,9 +99,9 @@ struct x86_model { struct x86_map { virCPUx86VendorPtr vendors; - struct x86_feature *features; + virCPUx86FeaturePtr features; struct x86_model *models; - struct x86_feature *migrate_blockers; + virCPUx86FeaturePtr migrate_blockers; }; static struct x86_map* virCPUx86Map; @@ -404,7 +406,7 @@ x86DataToCPUFeatures(virCPUDefPtr cpu, virCPUx86Data *data, const struct x86_map *map) { - const struct x86_feature *feature = map->features; + virCPUx86FeaturePtr feature = map->features; while (feature != NULL) { if (x86DataIsSubset(data, feature->data)) { @@ -573,10 +575,10 @@ x86VendorLoad(xmlXPathContextPtr ctxt, } -static struct x86_feature * +static virCPUx86FeaturePtr x86FeatureNew(void) { - struct x86_feature *feature; + virCPUx86FeaturePtr feature; if (VIR_ALLOC(feature) < 0) return NULL; @@ -591,7 +593,7 @@ x86FeatureNew(void) static void -x86FeatureFree(struct x86_feature *feature) +x86FeatureFree(virCPUx86FeaturePtr feature) { if (feature == NULL) return; @@ -602,10 +604,10 @@ x86FeatureFree(struct x86_feature *feature) } -static struct x86_feature * -x86FeatureCopy(const struct x86_feature *src) +static virCPUx86FeaturePtr +x86FeatureCopy(virCPUx86FeaturePtr src) { - struct x86_feature *feature; + virCPUx86FeaturePtr feature; if (VIR_ALLOC(feature) < 0) return NULL; @@ -624,11 +626,11 @@ x86FeatureCopy(const struct x86_feature *src) } -static struct x86_feature * +static virCPUx86FeaturePtr x86FeatureFind(const struct x86_map *map, const char *name) { - struct x86_feature *feature; + virCPUx86FeaturePtr feature; feature = map->features; while (feature != NULL) { @@ -650,7 +652,7 @@ x86FeatureNames(const struct x86_map *map, virBuffer ret = VIR_BUFFER_INITIALIZER; bool first = true; - struct x86_feature *next_feature = map->features; + virCPUx86FeaturePtr next_feature = map->features; virBufferAdd(&ret, "", 0); @@ -705,14 +707,14 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, { xmlNodePtr *nodes = NULL; xmlNodePtr ctxt_node = ctxt->node; - struct x86_feature *feature; + virCPUx86FeaturePtr feature; virCPUx86CPUID cpuid; int ret = 0; size_t i; int n; char *str = NULL; bool migratable = true; - struct x86_feature *migrate_blocker = NULL; + virCPUx86FeaturePtr migrate_blocker = NULL; if (!(feature = x86FeatureNew())) goto error; @@ -793,7 +795,7 @@ x86DataFromCPUFeatures(virCPUDefPtr cpu, return NULL; for (i = 0; i < cpu->nfeatures; i++) { - const struct x86_feature *feature; + virCPUx86FeaturePtr feature; if (!(feature = x86FeatureFind(map, cpu->features[i].name))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown CPU feature %s"), cpu->features[i].name); @@ -901,7 +903,7 @@ x86ModelFromCPU(const virCPUDef *cpu, } for (i = 0; i < cpu->nfeatures; i++) { - const struct x86_feature *feature; + virCPUx86FeaturePtr feature; if (cpu->type == VIR_CPU_TYPE_GUEST && cpu->features[i].policy != policy) @@ -943,7 +945,7 @@ x86ModelSubtractCPU(struct x86_model *model, x86DataSubtract(model->data, cpu_model->data); for (i = 0; i < cpu->nfeatures; i++) { - const struct x86_feature *feature; + virCPUx86FeaturePtr feature; if (!(feature = x86FeatureFind(map, cpu->features[i].name))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1076,7 +1078,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, goto ignore; for (i = 0; i < n; i++) { - const struct x86_feature *feature; + virCPUx86FeaturePtr feature; char *name; if ((name = virXMLPropString(nodes[i], "name")) == NULL) { @@ -1126,7 +1128,7 @@ x86MapFree(struct x86_map *map) return; while (map->features != NULL) { - struct x86_feature *feature = map->features; + virCPUx86FeaturePtr feature = map->features; map->features = feature->next; x86FeatureFree(feature); } @@ -1144,7 +1146,7 @@ x86MapFree(struct x86_map *map) } while (map->migrate_blockers != NULL) { - struct x86_feature *migrate_blocker = map->migrate_blockers; + virCPUx86FeaturePtr migrate_blocker = map->migrate_blockers; map->migrate_blockers = migrate_blocker->next; x86FeatureFree(migrate_blocker); } @@ -1179,7 +1181,7 @@ static int x86MapLoadInternalFeatures(struct x86_map *map) { size_t i; - struct x86_feature *feature = NULL; + virCPUx86FeaturePtr feature = NULL; for (i = 0; i < ARRAY_CARDINALITY(x86_kvm_features); i++) { const char *name = x86_kvm_features[i].name; @@ -1655,7 +1657,7 @@ x86Decode(virCPUDefPtr cpu, * features directly */ if (flags & VIR_CONNECT_BASELINE_CPU_MIGRATABLE) { for (i = 0; i < cpuModel->nfeatures; i++) { - const struct x86_feature *feat; + virCPUx86FeaturePtr feat; for (feat = map->migrate_blockers; feat; feat = feat->next) { if (STREQ(feat->name, cpuModel->features[i].name)) { VIR_FREE(cpuModel->features[i].name); @@ -2070,7 +2072,7 @@ x86UpdateCustom(virCPUDefPtr guest, for (i = 0; i < guest->nfeatures; i++) { if (guest->features[i].policy == VIR_CPU_FEATURE_OPTIONAL) { - const struct x86_feature *feature; + virCPUx86FeaturePtr feature; if (!(feature = x86FeatureFind(map, guest->features[i].name))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown CPU feature %s"), @@ -2108,7 +2110,7 @@ x86UpdateHostModel(virCPUDefPtr guest, { virCPUDefPtr oldguest = NULL; const struct x86_map *map; - const struct x86_feature *feat; + virCPUx86FeaturePtr feat; size_t i; int ret = -1; @@ -2186,7 +2188,7 @@ x86HasFeature(const virCPUData *data, const char *name) { const struct x86_map *map; - struct x86_feature *feature; + virCPUx86FeaturePtr feature; int ret = -1; if (!(map = virCPUx86GetMap())) -- 2.8.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 29680b4..be9eb8e 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -62,12 +62,14 @@ struct _virCPUx86Feature { virCPUx86FeaturePtr next; }; -struct x86_kvm_feature { +typedef struct _virCPUx86KVMFeature virCPUx86KVMFeature; +typedef virCPUx86KVMFeature *virCPUx86KVMFeaturePtr; +struct _virCPUx86KVMFeature { const char *name; const virCPUx86CPUID cpuid; }; -static const struct x86_kvm_feature x86_kvm_features[] = +static const virCPUx86KVMFeature x86_kvm_features[] = { {VIR_CPU_x86_KVM_CLOCKSOURCE, { .function = 0x40000001, .eax = 0x00000001 }}, {VIR_CPU_x86_KVM_NOP_IO_DELAY, { .function = 0x40000001, .eax = 0x00000002 }}, -- 2.8.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 72 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index be9eb8e..130119a 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -91,18 +91,20 @@ static const virCPUx86KVMFeature x86_kvm_features[] = {VIR_CPU_x86_KVM_HV_RESET, { .function = 0x40000003, .eax = 0x00000080 }}, }; -struct x86_model { +typedef struct _virCPUx86Model virCPUx86Model; +typedef virCPUx86Model *virCPUx86ModelPtr; +struct _virCPUx86Model { char *name; virCPUx86VendorPtr vendor; virCPUx86Data *data; - struct x86_model *next; + virCPUx86ModelPtr next; }; struct x86_map { virCPUx86VendorPtr vendors; virCPUx86FeaturePtr features; - struct x86_model *models; + virCPUx86ModelPtr models; virCPUx86FeaturePtr migrate_blockers; }; @@ -446,7 +448,7 @@ x86DataToVendor(virCPUx86Data *data, static virCPUDefPtr x86DataToCPU(const virCPUx86Data *data, - const struct x86_model *model, + virCPUx86ModelPtr model, const struct x86_map *map) { virCPUDefPtr cpu; @@ -816,10 +818,10 @@ x86DataFromCPUFeatures(virCPUDefPtr cpu, } -static struct x86_model * +static virCPUx86ModelPtr x86ModelNew(void) { - struct x86_model *model; + virCPUx86ModelPtr model; if (VIR_ALLOC(model) < 0) return NULL; @@ -834,7 +836,7 @@ x86ModelNew(void) static void -x86ModelFree(struct x86_model *model) +x86ModelFree(virCPUx86ModelPtr model) { if (model == NULL) return; @@ -845,10 +847,10 @@ x86ModelFree(struct x86_model *model) } -static struct x86_model * -x86ModelCopy(const struct x86_model *model) +static virCPUx86ModelPtr +x86ModelCopy(virCPUx86ModelPtr model) { - struct x86_model *copy; + virCPUx86ModelPtr copy; if (VIR_ALLOC(copy) < 0 || VIR_STRDUP(copy->name, model->name) < 0 || @@ -863,11 +865,11 @@ x86ModelCopy(const struct x86_model *model) } -static struct x86_model * +static virCPUx86ModelPtr x86ModelFind(const struct x86_map *map, const char *name) { - struct x86_model *model; + virCPUx86ModelPtr model; model = map->models; while (model != NULL) { @@ -881,12 +883,12 @@ x86ModelFind(const struct x86_map *map, } -static struct x86_model * +static virCPUx86ModelPtr x86ModelFromCPU(const virCPUDef *cpu, const struct x86_map *map, int policy) { - struct x86_model *model = NULL; + virCPUx86ModelPtr model = NULL; size_t i; if (policy == VIR_CPU_FEATURE_REQUIRE) { @@ -930,11 +932,11 @@ x86ModelFromCPU(const virCPUDef *cpu, static int -x86ModelSubtractCPU(struct x86_model *model, +x86ModelSubtractCPU(virCPUx86ModelPtr model, const virCPUDef *cpu, const struct x86_map *map) { - const struct x86_model *cpu_model; + virCPUx86ModelPtr cpu_model; size_t i; if (!(cpu_model = x86ModelFind(map, cpu->model))) { @@ -964,8 +966,8 @@ x86ModelSubtractCPU(struct x86_model *model, static enum compare_result -x86ModelCompare(const struct x86_model *model1, - const struct x86_model *model2) +x86ModelCompare(virCPUx86ModelPtr model1, + virCPUx86ModelPtr model2) { enum compare_result result = EQUAL; struct virCPUx86DataIterator iter1 = virCPUx86DataIteratorInit(model1->data); @@ -1014,7 +1016,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, struct x86_map *map) { xmlNodePtr *nodes = NULL; - struct x86_model *model; + virCPUx86ModelPtr model; char *vendor = NULL; int ret = 0; size_t i; @@ -1031,7 +1033,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, } if (virXPathNode("./model", ctxt) != NULL) { - const struct x86_model *ancestor; + virCPUx86ModelPtr ancestor; char *name; name = virXPathString("string(./model/@name)", ctxt); @@ -1136,7 +1138,7 @@ x86MapFree(struct x86_map *map) } while (map->models != NULL) { - struct x86_model *model = map->models; + virCPUx86ModelPtr model = map->models; map->models = model->next; x86ModelFree(model); } @@ -1372,14 +1374,14 @@ x86Compute(virCPUDefPtr host, char **message) { const struct x86_map *map = NULL; - struct x86_model *host_model = NULL; - struct x86_model *cpu_force = NULL; - struct x86_model *cpu_require = NULL; - struct x86_model *cpu_optional = NULL; - struct x86_model *cpu_disable = NULL; - struct x86_model *cpu_forbid = NULL; - struct x86_model *diff = NULL; - struct x86_model *guest_model = NULL; + virCPUx86ModelPtr host_model = NULL; + virCPUx86ModelPtr cpu_force = NULL; + virCPUx86ModelPtr cpu_require = NULL; + virCPUx86ModelPtr cpu_optional = NULL; + virCPUx86ModelPtr cpu_disable = NULL; + virCPUx86ModelPtr cpu_forbid = NULL; + virCPUx86ModelPtr diff = NULL; + virCPUx86ModelPtr guest_model = NULL; virCPUCompareResult ret; enum compare_result result; virArch arch; @@ -1568,7 +1570,7 @@ x86Decode(virCPUDefPtr cpu, { int ret = -1; const struct x86_map *map; - const struct x86_model *candidate; + virCPUx86ModelPtr candidate; virCPUDefPtr cpuCandidate; virCPUDefPtr cpuModel = NULL; virCPUx86Data *copy = NULL; @@ -1712,7 +1714,7 @@ x86EncodePolicy(const virCPUDef *cpu, const struct x86_map *map, virCPUFeaturePolicy policy) { - struct x86_model *model; + virCPUx86ModelPtr model; virCPUx86Data *data = NULL; if (!(model = x86ModelFromCPU(cpu, map, policy))) @@ -1942,11 +1944,11 @@ x86Baseline(virCPUDefPtr *cpus, unsigned int flags) { const struct x86_map *map = NULL; - struct x86_model *base_model = NULL; + virCPUx86ModelPtr base_model = NULL; virCPUDefPtr cpu = NULL; size_t i; virCPUx86VendorPtr vendor = NULL; - struct x86_model *model = NULL; + virCPUx86ModelPtr model = NULL; bool outputVendor = true; const char *modelName; bool matchingNames = true; @@ -2066,7 +2068,7 @@ x86UpdateCustom(virCPUDefPtr guest, int ret = -1; size_t i; const struct x86_map *map; - struct x86_model *host_model = NULL; + virCPUx86ModelPtr host_model = NULL; if (!(map = virCPUx86GetMap()) || !(host_model = x86ModelFromCPU(host, map, VIR_CPU_FEATURE_REQUIRE))) @@ -2209,7 +2211,7 @@ static int x86GetModels(char ***models) { const struct x86_map *map; - struct x86_model *model; + virCPUx86ModelPtr model; char *name; size_t nmodels = 0; -- 2.8.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 66 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 130119a..ca49b45 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -101,14 +101,16 @@ struct _virCPUx86Model { virCPUx86ModelPtr next; }; -struct x86_map { +typedef struct _virCPUx86Map virCPUx86Map; +typedef virCPUx86Map *virCPUx86MapPtr; +struct _virCPUx86Map { virCPUx86VendorPtr vendors; virCPUx86FeaturePtr features; virCPUx86ModelPtr models; virCPUx86FeaturePtr migrate_blockers; }; -static struct x86_map* virCPUx86Map; +static virCPUx86MapPtr cpuMap; int virCPUx86MapOnceInit(void); VIR_ONCE_GLOBAL_INIT(virCPUx86Map); @@ -408,7 +410,7 @@ static int x86DataToCPUFeatures(virCPUDefPtr cpu, int policy, virCPUx86Data *data, - const struct x86_map *map) + virCPUx86MapPtr map) { virCPUx86FeaturePtr feature = map->features; @@ -428,7 +430,7 @@ x86DataToCPUFeatures(virCPUDefPtr cpu, /* also removes bits corresponding to vendor string from data */ static virCPUx86VendorPtr x86DataToVendor(virCPUx86Data *data, - const struct x86_map *map) + virCPUx86MapPtr map) { virCPUx86VendorPtr vendor = map->vendors; virCPUx86CPUID *cpuid; @@ -449,7 +451,7 @@ x86DataToVendor(virCPUx86Data *data, static virCPUDefPtr x86DataToCPU(const virCPUx86Data *data, virCPUx86ModelPtr model, - const struct x86_map *map) + virCPUx86MapPtr map) { virCPUDefPtr cpu; virCPUx86Data *copy = NULL; @@ -500,7 +502,7 @@ x86VendorFree(virCPUx86VendorPtr vendor) static virCPUx86VendorPtr -x86VendorFind(const struct x86_map *map, +x86VendorFind(virCPUx86MapPtr map, const char *name) { virCPUx86VendorPtr vendor; @@ -519,7 +521,7 @@ x86VendorFind(const struct x86_map *map, static int x86VendorLoad(xmlXPathContextPtr ctxt, - struct x86_map *map) + virCPUx86MapPtr map) { virCPUx86VendorPtr vendor = NULL; char *string = NULL; @@ -631,7 +633,7 @@ x86FeatureCopy(virCPUx86FeaturePtr src) static virCPUx86FeaturePtr -x86FeatureFind(const struct x86_map *map, +x86FeatureFind(virCPUx86MapPtr map, const char *name) { virCPUx86FeaturePtr feature; @@ -649,7 +651,7 @@ x86FeatureFind(const struct x86_map *map, static char * -x86FeatureNames(const struct x86_map *map, +x86FeatureNames(virCPUx86MapPtr map, const char *separator, virCPUx86Data *data) { @@ -707,7 +709,7 @@ x86ParseCPUID(xmlXPathContextPtr ctxt, static int x86FeatureLoad(xmlXPathContextPtr ctxt, - struct x86_map *map) + virCPUx86MapPtr map) { xmlNodePtr *nodes = NULL; xmlNodePtr ctxt_node = ctxt->node; @@ -790,7 +792,7 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, static virCPUx86Data * x86DataFromCPUFeatures(virCPUDefPtr cpu, - const struct x86_map *map) + virCPUx86MapPtr map) { virCPUx86Data *data; size_t i; @@ -866,7 +868,7 @@ x86ModelCopy(virCPUx86ModelPtr model) static virCPUx86ModelPtr -x86ModelFind(const struct x86_map *map, +x86ModelFind(virCPUx86MapPtr map, const char *name) { virCPUx86ModelPtr model; @@ -885,7 +887,7 @@ x86ModelFind(const struct x86_map *map, static virCPUx86ModelPtr x86ModelFromCPU(const virCPUDef *cpu, - const struct x86_map *map, + virCPUx86MapPtr map, int policy) { virCPUx86ModelPtr model = NULL; @@ -934,7 +936,7 @@ x86ModelFromCPU(const virCPUDef *cpu, static int x86ModelSubtractCPU(virCPUx86ModelPtr model, const virCPUDef *cpu, - const struct x86_map *map) + virCPUx86MapPtr map) { virCPUx86ModelPtr cpu_model; size_t i; @@ -1013,7 +1015,7 @@ x86ModelCompare(virCPUx86ModelPtr model1, static int x86ModelLoad(xmlXPathContextPtr ctxt, - struct x86_map *map) + virCPUx86MapPtr map) { xmlNodePtr *nodes = NULL; virCPUx86ModelPtr model; @@ -1126,7 +1128,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, static void -x86MapFree(struct x86_map *map) +x86MapFree(virCPUx86MapPtr map) { if (map == NULL) return; @@ -1164,7 +1166,7 @@ x86MapLoadCallback(cpuMapElement element, xmlXPathContextPtr ctxt, void *data) { - struct x86_map *map = data; + virCPUx86MapPtr map = data; switch (element) { case CPU_MAP_ELEMENT_VENDOR: @@ -1182,7 +1184,7 @@ x86MapLoadCallback(cpuMapElement element, static int -x86MapLoadInternalFeatures(struct x86_map *map) +x86MapLoadInternalFeatures(virCPUx86MapPtr map) { size_t i; virCPUx86FeaturePtr feature = NULL; @@ -1223,10 +1225,10 @@ x86MapLoadInternalFeatures(struct x86_map *map) } -static struct x86_map * +static virCPUx86MapPtr virCPUx86LoadMap(void) { - struct x86_map *map; + virCPUx86MapPtr map; if (VIR_ALLOC(map) < 0) return NULL; @@ -1248,20 +1250,20 @@ virCPUx86LoadMap(void) int virCPUx86MapOnceInit(void) { - if (!(virCPUx86Map = virCPUx86LoadMap())) + if (!(cpuMap = virCPUx86LoadMap())) return -1; return 0; } -static const struct x86_map * +static virCPUx86MapPtr virCPUx86GetMap(void) { if (virCPUx86MapInitialize() < 0) return NULL; - return virCPUx86Map; + return cpuMap; } @@ -1373,7 +1375,7 @@ x86Compute(virCPUDefPtr host, virCPUDataPtr *guest, char **message) { - const struct x86_map *map = NULL; + virCPUx86MapPtr map = NULL; virCPUx86ModelPtr host_model = NULL; virCPUx86ModelPtr cpu_force = NULL; virCPUx86ModelPtr cpu_require = NULL; @@ -1569,7 +1571,7 @@ x86Decode(virCPUDefPtr cpu, unsigned int flags) { int ret = -1; - const struct x86_map *map; + virCPUx86MapPtr map; virCPUx86ModelPtr candidate; virCPUDefPtr cpuCandidate; virCPUDefPtr cpuModel = NULL; @@ -1711,7 +1713,7 @@ x86DecodeCPUData(virCPUDefPtr cpu, static virCPUx86Data * x86EncodePolicy(const virCPUDef *cpu, - const struct x86_map *map, + virCPUx86MapPtr map, virCPUFeaturePolicy policy) { virCPUx86ModelPtr model; @@ -1738,7 +1740,7 @@ x86Encode(virArch arch, virCPUDataPtr *forbidden, virCPUDataPtr *vendor) { - const struct x86_map *map = NULL; + virCPUx86MapPtr map = NULL; virCPUx86Data *data_forced = NULL; virCPUx86Data *data_required = NULL; virCPUx86Data *data_optional = NULL; @@ -1943,7 +1945,7 @@ x86Baseline(virCPUDefPtr *cpus, unsigned int nmodels, unsigned int flags) { - const struct x86_map *map = NULL; + virCPUx86MapPtr map = NULL; virCPUx86ModelPtr base_model = NULL; virCPUDefPtr cpu = NULL; size_t i; @@ -2067,7 +2069,7 @@ x86UpdateCustom(virCPUDefPtr guest, { int ret = -1; size_t i; - const struct x86_map *map; + virCPUx86MapPtr map; virCPUx86ModelPtr host_model = NULL; if (!(map = virCPUx86GetMap()) || @@ -2113,7 +2115,7 @@ x86UpdateHostModel(virCPUDefPtr guest, bool passthrough) { virCPUDefPtr oldguest = NULL; - const struct x86_map *map; + virCPUx86MapPtr map; virCPUx86FeaturePtr feat; size_t i; int ret = -1; @@ -2191,7 +2193,7 @@ static int x86HasFeature(const virCPUData *data, const char *name) { - const struct x86_map *map; + virCPUx86MapPtr map; virCPUx86FeaturePtr feature; int ret = -1; @@ -2210,7 +2212,7 @@ x86HasFeature(const virCPUData *data, static int x86GetModels(char ***models) { - const struct x86_map *map; + virCPUx86MapPtr map; virCPUx86ModelPtr model; char *name; size_t nmodels = 0; -- 2.8.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index ca49b45..3627398 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -115,12 +115,12 @@ int virCPUx86MapOnceInit(void); VIR_ONCE_GLOBAL_INIT(virCPUx86Map); -enum compare_result { +typedef enum { SUBSET, EQUAL, SUPERSET, UNRELATED -}; +} virCPUx86CompareResult; struct virCPUx86DataIterator { @@ -967,18 +967,18 @@ x86ModelSubtractCPU(virCPUx86ModelPtr model, } -static enum compare_result +static virCPUx86CompareResult x86ModelCompare(virCPUx86ModelPtr model1, virCPUx86ModelPtr model2) { - enum compare_result result = EQUAL; + virCPUx86CompareResult result = EQUAL; struct virCPUx86DataIterator iter1 = virCPUx86DataIteratorInit(model1->data); struct virCPUx86DataIterator iter2 = virCPUx86DataIteratorInit(model2->data); virCPUx86CPUID *cpuid1; virCPUx86CPUID *cpuid2; while ((cpuid1 = x86DataCpuidNext(&iter1))) { - enum compare_result match = SUPERSET; + virCPUx86CompareResult match = SUPERSET; if ((cpuid2 = x86DataCpuid(model2->data, cpuid1->function))) { if (x86cpuidMatch(cpuid1, cpuid2)) @@ -994,7 +994,7 @@ x86ModelCompare(virCPUx86ModelPtr model1, } while ((cpuid2 = x86DataCpuidNext(&iter2))) { - enum compare_result match = SUBSET; + virCPUx86CompareResult match = SUBSET; if ((cpuid1 = x86DataCpuid(model1->data, cpuid2->function))) { if (x86cpuidMatch(cpuid2, cpuid1)) @@ -1385,7 +1385,7 @@ x86Compute(virCPUDefPtr host, virCPUx86ModelPtr diff = NULL; virCPUx86ModelPtr guest_model = NULL; virCPUCompareResult ret; - enum compare_result result; + virCPUx86CompareResult result; virArch arch; size_t i; -- 2.8.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 3627398..f3d2194 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -123,7 +123,9 @@ typedef enum { } virCPUx86CompareResult; -struct virCPUx86DataIterator { +typedef struct _virCPUx86DataIterator virCPUx86DataIterator; +typedef virCPUx86DataIterator *virCPUx86DataIteratorPtr; +struct _virCPUx86DataIterator { const virCPUx86Data *data; int pos; }; @@ -213,7 +215,7 @@ virCPUx86CPUIDSorter(const void *a, const void *b) /* skips all zero CPUID leafs */ static virCPUx86CPUID * -x86DataCpuidNext(struct virCPUx86DataIterator *iterator) +x86DataCpuidNext(virCPUx86DataIteratorPtr iterator) { const virCPUx86Data *data = iterator->data; @@ -325,7 +327,7 @@ static int x86DataAdd(virCPUx86Data *data1, const virCPUx86Data *data2) { - struct virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data2); + virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data2); virCPUx86CPUID *cpuid1; virCPUx86CPUID *cpuid2; @@ -348,7 +350,7 @@ static void x86DataSubtract(virCPUx86Data *data1, const virCPUx86Data *data2) { - struct virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data1); + virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data1); virCPUx86CPUID *cpuid1; virCPUx86CPUID *cpuid2; @@ -363,7 +365,7 @@ static void x86DataIntersect(virCPUx86Data *data1, const virCPUx86Data *data2) { - struct virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data1); + virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data1); virCPUx86CPUID *cpuid1; virCPUx86CPUID *cpuid2; @@ -380,7 +382,7 @@ x86DataIntersect(virCPUx86Data *data1, static bool x86DataIsEmpty(virCPUx86Data *data) { - struct virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data); + virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data); return x86DataCpuidNext(&iter) == NULL; } @@ -391,7 +393,7 @@ x86DataIsSubset(const virCPUx86Data *data, const virCPUx86Data *subset) { - struct virCPUx86DataIterator iter = virCPUx86DataIteratorInit((virCPUx86Data *)subset); + virCPUx86DataIterator iter = virCPUx86DataIteratorInit((virCPUx86Data *)subset); const virCPUx86CPUID *cpuid; const virCPUx86CPUID *cpuidSubset; @@ -972,8 +974,8 @@ x86ModelCompare(virCPUx86ModelPtr model1, virCPUx86ModelPtr model2) { virCPUx86CompareResult result = EQUAL; - struct virCPUx86DataIterator iter1 = virCPUx86DataIteratorInit(model1->data); - struct virCPUx86DataIterator iter2 = virCPUx86DataIteratorInit(model2->data); + virCPUx86DataIterator iter1 = virCPUx86DataIteratorInit(model1->data); + virCPUx86DataIterator iter2 = virCPUx86DataIteratorInit(model2->data); virCPUx86CPUID *cpuid1; virCPUx86CPUID *cpuid2; @@ -1270,7 +1272,7 @@ virCPUx86GetMap(void) static char * x86CPUDataFormat(const virCPUData *data) { - struct virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data->data.x86); + virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data->data.x86); virCPUx86CPUID *cpuid; virBuffer buf = VIR_BUFFER_INITIALIZER; -- 2.8.2
Splitting the comparison into a separate function makes the code cleaner and easier to update in the future. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 65 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index f3d2194..64b9805 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1564,6 +1564,43 @@ x86GuestData(virCPUDefPtr host, } +/* + * Checks whether cpuCandidate is a better fit for the CPU data than the + * currently selected one from cpuCurrent. + * + * Returns 0 if cpuCurrent is better, + * 1 if cpuCandidate is better, + * 2 if cpuCandidate is the best one (search should stop now). + */ +static int +x86DecodeUseCandidate(virCPUDefPtr cpuCurrent, + virCPUDefPtr cpuCandidate, + const char *preferred, + bool checkPolicy) +{ + if (checkPolicy) { + size_t i; + for (i = 0; i < cpuCandidate->nfeatures; i++) { + if (cpuCandidate->features[i].policy == VIR_CPU_FEATURE_DISABLE) + return 0; + cpuCandidate->features[i].policy = -1; + } + } + + if (preferred && + STREQ(cpuCandidate->model, preferred)) + return 2; + + if (!cpuCurrent) + return 1; + + if (cpuCurrent->nfeatures > cpuCandidate->nfeatures) + return 1; + + return 0; +} + + static int x86Decode(virCPUDefPtr cpu, const virCPUx86Data *data, @@ -1581,6 +1618,7 @@ x86Decode(virCPUDefPtr cpu, virCPUx86Data *features = NULL; const virCPUx86Data *cpuData = NULL; size_t i; + int rc; virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | VIR_CONNECT_BASELINE_CPU_MIGRATABLE, -1); @@ -1611,6 +1649,7 @@ x86Decode(virCPUDefPtr cpu, if (!(cpuCandidate = x86DataToCPU(data, candidate, map))) goto out; + cpuCandidate->type = cpu->type; if (candidate->vendor && cpuCandidate->vendor && STRNEQ(candidate->vendor->name, cpuCandidate->vendor)) { @@ -1621,31 +1660,13 @@ x86Decode(virCPUDefPtr cpu, goto next; } - if (cpu->type == VIR_CPU_TYPE_HOST) { - cpuCandidate->type = VIR_CPU_TYPE_HOST; - for (i = 0; i < cpuCandidate->nfeatures; i++) { - switch (cpuCandidate->features[i].policy) { - case VIR_CPU_FEATURE_DISABLE: - virCPUDefFree(cpuCandidate); - goto next; - default: - cpuCandidate->features[i].policy = -1; - } - } - } - - if (preferred && STREQ(cpuCandidate->model, preferred)) { - virCPUDefFree(cpuModel); - cpuModel = cpuCandidate; - cpuData = candidate->data; - break; - } - - if (cpuModel == NULL - || cpuModel->nfeatures > cpuCandidate->nfeatures) { + if ((rc = x86DecodeUseCandidate(cpuModel, cpuCandidate, preferred, + cpu->type == VIR_CPU_TYPE_HOST))) { virCPUDefFree(cpuModel); cpuModel = cpuCandidate; cpuData = candidate->data; + if (rc == 2) + break; } else { virCPUDefFree(cpuCandidate); } -- 2.8.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 64b9805..ae5fdff 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -570,7 +570,7 @@ x86VendorLoad(xmlXPathContextPtr ctxt, map->vendors = vendor; } - out: + cleanup: VIR_FREE(string); return ret; @@ -579,7 +579,7 @@ x86VendorLoad(xmlXPathContextPtr ctxt, ret = -1; ignore: x86VendorFree(vendor); - goto out; + goto cleanup; } @@ -775,7 +775,7 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, map->features = feature; } - out: + cleanup: ctxt->node = ctxt_node; VIR_FREE(nodes); VIR_FREE(str); @@ -788,7 +788,7 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, ignore: x86FeatureFree(feature); x86FeatureFree(migrate_blocker); - goto out; + goto cleanup; } @@ -1115,7 +1115,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, map->models = model; } - out: + cleanup: VIR_FREE(vendor); VIR_FREE(nodes); return ret; @@ -1125,7 +1125,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, ignore: x86ModelFree(model); - goto out; + goto cleanup; } @@ -1634,7 +1634,7 @@ x86Decode(virCPUDefPtr cpu, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("CPU model %s is not supported by hypervisor"), preferred); - goto out; + goto cleanup; } else { VIR_WARN("Preferred CPU model %s not allowed by" " hypervisor; closest supported model will be" @@ -1648,7 +1648,7 @@ x86Decode(virCPUDefPtr cpu, } if (!(cpuCandidate = x86DataToCPU(data, candidate, map))) - goto out; + goto cleanup; cpuCandidate->type = cpu->type; if (candidate->vendor && cpuCandidate->vendor && @@ -1678,7 +1678,7 @@ x86Decode(virCPUDefPtr cpu, if (cpuModel == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot find suitable CPU model for given data")); - goto out; + goto cleanup; } /* Remove non-migratable features if requested @@ -1699,12 +1699,12 @@ x86Decode(virCPUDefPtr cpu, if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES) { if (!(copy = x86DataCopy(cpuData)) || !(features = x86DataFromCPUFeatures(cpuModel, map))) - goto out; + goto cleanup; x86DataSubtract(copy, features); if (x86DataToCPUFeatures(cpuModel, VIR_CPU_FEATURE_REQUIRE, copy, map) < 0) - goto out; + goto cleanup; } cpu->model = cpuModel->model; @@ -1715,7 +1715,7 @@ x86Decode(virCPUDefPtr cpu, ret = 0; - out: + cleanup: virCPUDefFree(cpuModel); virCPUx86DataFree(copy); virCPUx86DataFree(features); -- 2.8.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index ae5fdff..58bcacb 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1626,8 +1626,7 @@ x86Decode(virCPUDefPtr cpu, if (!data || !(map = virCPUx86GetMap())) return -1; - candidate = map->models; - while (candidate != NULL) { + for (candidate = map->models; candidate; candidate = candidate->next) { if (!cpuModelIsAllowed(candidate->name, models, nmodels)) { if (preferred && STREQ(candidate->name, preferred)) { if (cpu->fallback != VIR_CPU_FALLBACK_ALLOW) { @@ -1644,7 +1643,7 @@ x86Decode(virCPUDefPtr cpu, VIR_DEBUG("CPU model %s not allowed by hypervisor; ignoring", candidate->name); } - goto next; + continue; } if (!(cpuCandidate = x86DataToCPU(data, candidate, map))) @@ -1657,7 +1656,7 @@ x86Decode(virCPUDefPtr cpu, candidate->vendor->name, candidate->name, cpuCandidate->vendor); virCPUDefFree(cpuCandidate); - goto next; + continue; } if ((rc = x86DecodeUseCandidate(cpuModel, cpuCandidate, preferred, @@ -1670,9 +1669,6 @@ x86Decode(virCPUDefPtr cpu, } else { virCPUDefFree(cpuCandidate); } - - next: - candidate = candidate->next; } if (cpuModel == NULL) { -- 2.8.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 66 +++++++++++++++++++++++++++---------------------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 58bcacb..f8121d1 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -248,7 +248,7 @@ x86DataCpuid(const virCPUx86Data *data, void virCPUx86DataFree(virCPUx86Data *data) { - if (data == NULL) + if (!data) return; VIR_FREE(data->data); @@ -384,7 +384,7 @@ x86DataIsEmpty(virCPUx86Data *data) { virCPUx86DataIterator iter = virCPUx86DataIteratorInit(data); - return x86DataCpuidNext(&iter) == NULL; + return !x86DataCpuidNext(&iter); } @@ -416,7 +416,7 @@ x86DataToCPUFeatures(virCPUDefPtr cpu, { virCPUx86FeaturePtr feature = map->features; - while (feature != NULL) { + while (feature) { if (x86DataIsSubset(data, feature->data)) { x86DataSubtract(data, feature->data); if (virCPUDefAddFeature(cpu, feature->name, policy) < 0) @@ -603,7 +603,7 @@ x86FeatureNew(void) static void x86FeatureFree(virCPUx86FeaturePtr feature) { - if (feature == NULL) + if (!feature) return; VIR_FREE(feature->name); @@ -623,7 +623,7 @@ x86FeatureCopy(virCPUx86FeaturePtr src) if (VIR_STRDUP(feature->name, src->name) < 0) goto error; - if ((feature->data = x86DataCopy(src->data)) == NULL) + if (!(feature->data = x86DataCopy(src->data))) goto error; return feature; @@ -641,7 +641,7 @@ x86FeatureFind(virCPUx86MapPtr map, virCPUx86FeaturePtr feature; feature = map->features; - while (feature != NULL) { + while (feature) { if (STREQ(feature->name, name)) return feature; @@ -728,7 +728,7 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, goto error; feature->name = virXPathString("string(@name)", ctxt); - if (feature->name == NULL) { + if (!feature->name) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing CPU feature name")); goto ignore; @@ -761,14 +761,14 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, } if (!migratable) { - if ((migrate_blocker = x86FeatureCopy(feature)) == NULL) + if (!(migrate_blocker = x86FeatureCopy(feature))) goto error; migrate_blocker->next = map->migrate_blockers; map->migrate_blockers = migrate_blocker; } - if (map->features == NULL) { + if (!map->features) { map->features = feature; } else { feature->next = map->features; @@ -842,7 +842,7 @@ x86ModelNew(void) static void x86ModelFree(virCPUx86ModelPtr model) { - if (model == NULL) + if (!model) return; VIR_FREE(model->name); @@ -876,7 +876,7 @@ x86ModelFind(virCPUx86MapPtr map, virCPUx86ModelPtr model; model = map->models; - while (model != NULL) { + while (model) { if (STREQ(model->name, name)) return model; @@ -896,13 +896,13 @@ x86ModelFromCPU(const virCPUDef *cpu, size_t i; if (policy == VIR_CPU_FEATURE_REQUIRE) { - if ((model = x86ModelFind(map, cpu->model)) == NULL) { + if (!(model = x86ModelFind(map, cpu->model))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown CPU model %s"), cpu->model); goto error; } - if ((model = x86ModelCopy(model)) == NULL) + if (!(model = x86ModelCopy(model))) goto error; } else if (!(model = x86ModelNew())) { goto error; @@ -917,7 +917,7 @@ x86ModelFromCPU(const virCPUDef *cpu, && cpu->features[i].policy != policy) continue; - if ((feature = x86FeatureFind(map, cpu->features[i].name)) == NULL) { + if (!(feature = x86FeatureFind(map, cpu->features[i].name))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown CPU feature %s"), cpu->features[i].name); goto error; @@ -1030,25 +1030,25 @@ x86ModelLoad(xmlXPathContextPtr ctxt, goto error; model->name = virXPathString("string(@name)", ctxt); - if (model->name == NULL) { + if (!model->name) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing CPU model name")); goto ignore; } - if (virXPathNode("./model", ctxt) != NULL) { + if (virXPathNode("./model", ctxt)) { virCPUx86ModelPtr ancestor; char *name; name = virXPathString("string(./model/@name)", ctxt); - if (name == NULL) { + if (!name) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Missing ancestor's name in CPU model %s"), model->name); goto ignore; } - if ((ancestor = x86ModelFind(map, name)) == NULL) { + if (!(ancestor = x86ModelFind(map, name))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Ancestor model %s not found for CPU model %s"), name, model->name); @@ -1089,13 +1089,13 @@ x86ModelLoad(xmlXPathContextPtr ctxt, virCPUx86FeaturePtr feature; char *name; - if ((name = virXMLPropString(nodes[i], "name")) == NULL) { + if (!(name = virXMLPropString(nodes[i], "name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Missing feature name for CPU model %s"), model->name); goto ignore; } - if ((feature = x86FeatureFind(map, name)) == NULL) { + if (!(feature = x86FeatureFind(map, name))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Feature %s required by CPU model %s not found"), name, model->name); @@ -1108,7 +1108,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, goto error; } - if (map->models == NULL) { + if (!map->models) { map->models = model; } else { model->next = map->models; @@ -1132,28 +1132,28 @@ x86ModelLoad(xmlXPathContextPtr ctxt, static void x86MapFree(virCPUx86MapPtr map) { - if (map == NULL) + if (!map) return; - while (map->features != NULL) { + while (map->features) { virCPUx86FeaturePtr feature = map->features; map->features = feature->next; x86FeatureFree(feature); } - while (map->models != NULL) { + while (map->models) { virCPUx86ModelPtr model = map->models; map->models = model->next; x86ModelFree(model); } - while (map->vendors != NULL) { + while (map->vendors) { virCPUx86VendorPtr vendor = map->vendors; map->vendors = vendor->next; x86VendorFree(vendor); } - while (map->migrate_blockers != NULL) { + while (map->migrate_blockers) { virCPUx86FeaturePtr migrate_blocker = map->migrate_blockers; map->migrate_blockers = migrate_blocker->next; x86FeatureFree(migrate_blocker); @@ -1209,7 +1209,7 @@ x86MapLoadInternalFeatures(virCPUx86MapPtr map) if (virCPUx86DataAddCPUID(feature->data, &x86_kvm_features[i].cpuid)) goto error; - if (map->features == NULL) { + if (!map->features) { map->features = feature; } else { feature->next = map->features; @@ -1469,7 +1469,7 @@ x86Compute(virCPUDefPtr host, ret = VIR_CPU_COMPARE_IDENTICAL; - if ((diff = x86ModelCopy(host_model)) == NULL) + if (!(diff = x86ModelCopy(host_model))) goto error; x86DataSubtract(diff->data, cpu_optional->data); @@ -1489,10 +1489,10 @@ x86Compute(virCPUDefPtr host, goto cleanup; } - if (guest != NULL) { + if (guest) { virCPUx86Data *guestData; - if ((guest_model = x86ModelCopy(host_model)) == NULL) + if (!(guest_model = x86ModelCopy(host_model))) goto error; if (cpu->type == VIR_CPU_TYPE_GUEST @@ -1671,7 +1671,7 @@ x86Decode(virCPUDefPtr cpu, } } - if (cpuModel == NULL) { + if (!cpuModel) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot find suitable CPU model for given data")); goto cleanup; @@ -1780,7 +1780,7 @@ x86Encode(virArch arch, if (vendor) *vendor = NULL; - if ((map = virCPUx86GetMap()) == NULL) + if (!(map = virCPUx86GetMap())) goto error; if (forced) { @@ -2243,7 +2243,7 @@ x86GetModels(char ***models) goto error; model = map->models; - while (model != NULL) { + while (model) { if (models) { if (VIR_STRDUP(name, model->name) < 0) goto error; -- 2.8.2
The next pointer is initialized to NULL, overwriting to with another NULL doesn't hurt. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index f8121d1..94260e9 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -563,12 +563,8 @@ x86VendorLoad(xmlXPathContextPtr ctxt, vendor->cpuid.edx = virReadBufInt32LE(string + 4); vendor->cpuid.ecx = virReadBufInt32LE(string + 8); - if (!map->vendors) { - map->vendors = vendor; - } else { - vendor->next = map->vendors; - map->vendors = vendor; - } + vendor->next = map->vendors; + map->vendors = vendor; cleanup: VIR_FREE(string); @@ -768,12 +764,8 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, map->migrate_blockers = migrate_blocker; } - if (!map->features) { - map->features = feature; - } else { - feature->next = map->features; - map->features = feature; - } + feature->next = map->features; + map->features = feature; cleanup: ctxt->node = ctxt_node; @@ -1108,12 +1100,8 @@ x86ModelLoad(xmlXPathContextPtr ctxt, goto error; } - if (!map->models) { - map->models = model; - } else { - model->next = map->models; - map->models = model; - } + model->next = map->models; + map->models = model; cleanup: VIR_FREE(vendor); @@ -1209,13 +1197,8 @@ x86MapLoadInternalFeatures(virCPUx86MapPtr map) if (virCPUx86DataAddCPUID(feature->data, &x86_kvm_features[i].cpuid)) goto error; - if (!map->features) { - map->features = feature; - } else { - feature->next = map->features; - map->features = feature; - } - + feature->next = map->features; + map->features = feature; feature = NULL; } -- 2.8.2
CPU map XML is our internal data file, it makes no sense to tolerate any errors in it. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 94260e9..8cedb3f 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -527,22 +527,22 @@ x86VendorLoad(xmlXPathContextPtr ctxt, { virCPUx86VendorPtr vendor = NULL; char *string = NULL; - int ret = 0; + int ret = -1; if (VIR_ALLOC(vendor) < 0) - goto error; + goto cleanup; vendor->name = virXPathString("string(@name)", ctxt); if (!vendor->name) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing CPU vendor name")); - goto ignore; + goto cleanup; } if (x86VendorFind(map, vendor->name)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("CPU vendor %s already defined"), vendor->name); - goto ignore; + goto cleanup; } string = virXPathString("string(@string)", ctxt); @@ -550,12 +550,12 @@ x86VendorLoad(xmlXPathContextPtr ctxt, virReportError(VIR_ERR_INTERNAL_ERROR, _("Missing vendor string for CPU vendor %s"), vendor->name); - goto ignore; + goto cleanup; } if (strlen(string) != VENDOR_STRING_LENGTH) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid CPU vendor string '%s'"), string); - goto ignore; + goto cleanup; } vendor->cpuid.function = 0; @@ -565,17 +565,14 @@ x86VendorLoad(xmlXPathContextPtr ctxt, vendor->next = map->vendors; map->vendors = vendor; + vendor = NULL; + + ret = 0; cleanup: VIR_FREE(string); - - return ret; - - error: - ret = -1; - ignore: x86VendorFree(vendor); - goto cleanup; + return ret; } -- 2.8.2
CPU map XML is our internal data file, it makes no sense to tolerate any errors in it. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 37 +++++++++++++++++-------------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 8cedb3f..03f8ccf 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -710,27 +710,26 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, xmlNodePtr ctxt_node = ctxt->node; virCPUx86FeaturePtr feature; virCPUx86CPUID cpuid; - int ret = 0; + int ret = -1; size_t i; int n; char *str = NULL; bool migratable = true; - virCPUx86FeaturePtr migrate_blocker = NULL; if (!(feature = x86FeatureNew())) - goto error; + goto cleanup; feature->name = virXPathString("string(@name)", ctxt); if (!feature->name) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing CPU feature name")); - goto ignore; + goto cleanup; } if (x86FeatureFind(map, feature->name)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("CPU feature %s already defined"), feature->name); - goto ignore; + goto cleanup; } str = virXPathString("string(@migratable)", ctxt); @@ -739,7 +738,7 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, n = virXPathNodeSet("./cpuid", ctxt, &nodes); if (n < 0) - goto ignore; + goto cleanup; for (i = 0; i < n; i++) { ctxt->node = nodes[i]; @@ -747,37 +746,35 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid cpuid[%zu] in %s feature"), i, feature->name); - goto ignore; + goto cleanup; } if (virCPUx86DataAddCPUID(feature->data, &cpuid)) - goto error; + goto cleanup; } if (!migratable) { - if (!(migrate_blocker = x86FeatureCopy(feature))) - goto error; + virCPUx86FeaturePtr blocker; - migrate_blocker->next = map->migrate_blockers; - map->migrate_blockers = migrate_blocker; + if (!(blocker = x86FeatureCopy(feature))) + goto cleanup; + + blocker->next = map->migrate_blockers; + map->migrate_blockers = blocker; } feature->next = map->features; map->features = feature; + feature = NULL; + + ret = 0; cleanup: + x86FeatureFree(feature); ctxt->node = ctxt_node; VIR_FREE(nodes); VIR_FREE(str); return ret; - - error: - ret = -1; - - ignore: - x86FeatureFree(feature); - x86FeatureFree(migrate_blocker); - goto cleanup; } -- 2.8.2
CPU map XML is our internal data file, it makes no sense to tolerate any errors in it. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 03f8ccf..e7bd521 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1008,18 +1008,18 @@ x86ModelLoad(xmlXPathContextPtr ctxt, xmlNodePtr *nodes = NULL; virCPUx86ModelPtr model; char *vendor = NULL; - int ret = 0; + int ret = -1; size_t i; int n; if (!(model = x86ModelNew())) - goto error; + goto cleanup; model->name = virXPathString("string(@name)", ctxt); if (!model->name) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing CPU model name")); - goto ignore; + goto cleanup; } if (virXPathNode("./model", ctxt)) { @@ -1031,7 +1031,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, virReportError(VIR_ERR_INTERNAL_ERROR, _("Missing ancestor's name in CPU model %s"), model->name); - goto ignore; + goto cleanup; } if (!(ancestor = x86ModelFind(map, name))) { @@ -1039,7 +1039,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, _("Ancestor model %s not found for CPU model %s"), name, model->name); VIR_FREE(name); - goto ignore; + goto cleanup; } VIR_FREE(name); @@ -1047,7 +1047,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, model->vendor = ancestor->vendor; virCPUx86DataFree(model->data); if (!(model->data = x86DataCopy(ancestor->data))) - goto error; + goto cleanup; } if (virXPathBoolean("boolean(./vendor)", ctxt)) { @@ -1056,20 +1056,20 @@ x86ModelLoad(xmlXPathContextPtr ctxt, virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid vendor element in CPU model %s"), model->name); - goto ignore; + goto cleanup; } if (!(model->vendor = x86VendorFind(map, vendor))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown vendor %s referenced by CPU model %s"), vendor, model->name); - goto ignore; + goto cleanup; } } n = virXPathNodeSet("./feature", ctxt, &nodes); if (n < 0) - goto ignore; + goto cleanup; for (i = 0; i < n; i++) { virCPUx86FeaturePtr feature; @@ -1078,7 +1078,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, if (!(name = virXMLPropString(nodes[i], "name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Missing feature name for CPU model %s"), model->name); - goto ignore; + goto cleanup; } if (!(feature = x86FeatureFind(map, name))) { @@ -1086,28 +1086,25 @@ x86ModelLoad(xmlXPathContextPtr ctxt, _("Feature %s required by CPU model %s not found"), name, model->name); VIR_FREE(name); - goto ignore; + goto cleanup; } VIR_FREE(name); if (x86DataAdd(model->data, feature->data)) - goto error; + goto cleanup; } model->next = map->models; map->models = model; + model = NULL; + + ret = 0; cleanup: + x86ModelFree(model); VIR_FREE(vendor); VIR_FREE(nodes); return ret; - - error: - ret = -1; - - ignore: - x86ModelFree(model); - goto cleanup; } -- 2.8.2
When searching for the best CPU model for CPUID data we can easily ignore models with non-matching vendor before spending time on CPUID data to virCPUDef conversion. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_x86.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index e7bd521..701486c 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -431,7 +431,7 @@ x86DataToCPUFeatures(virCPUDefPtr cpu, /* also removes bits corresponding to vendor string from data */ static virCPUx86VendorPtr -x86DataToVendor(virCPUx86Data *data, +x86DataToVendor(const virCPUx86Data *data, virCPUx86MapPtr map) { virCPUx86VendorPtr vendor = map->vendors; @@ -1591,6 +1591,7 @@ x86Decode(virCPUDefPtr cpu, virCPUx86Data *copy = NULL; virCPUx86Data *features = NULL; const virCPUx86Data *cpuData = NULL; + virCPUx86VendorPtr vendor; size_t i; int rc; @@ -1600,6 +1601,8 @@ x86Decode(virCPUDefPtr cpu, if (!data || !(map = virCPUx86GetMap())) return -1; + vendor = x86DataToVendor(data, map); + for (candidate = map->models; candidate; candidate = candidate->next) { if (!cpuModelIsAllowed(candidate->name, models, nmodels)) { if (preferred && STREQ(candidate->name, preferred)) { @@ -1620,19 +1623,19 @@ x86Decode(virCPUDefPtr cpu, continue; } + /* Both vendor and candidate->vendor are pointers to a single list of + * known vendors stored in the map. + */ + if (vendor && candidate->vendor && vendor != candidate->vendor) { + VIR_DEBUG("CPU vendor %s of model %s differs from %s; ignoring", + candidate->vendor->name, candidate->name, vendor->name); + continue; + } + if (!(cpuCandidate = x86DataToCPU(data, candidate, map))) goto cleanup; cpuCandidate->type = cpu->type; - if (candidate->vendor && cpuCandidate->vendor && - STRNEQ(candidate->vendor->name, cpuCandidate->vendor)) { - VIR_DEBUG("CPU vendor %s of model %s differs from %s; ignoring", - candidate->vendor->name, candidate->name, - cpuCandidate->vendor); - virCPUDefFree(cpuCandidate); - continue; - } - if ((rc = x86DecodeUseCandidate(cpuModel, cpuCandidate, preferred, cpu->type == VIR_CPU_TYPE_HOST))) { virCPUDefFree(cpuModel); @@ -1677,8 +1680,10 @@ x86Decode(virCPUDefPtr cpu, goto cleanup; } + if (vendor && VIR_STRDUP(cpu->vendor, vendor->name) < 0) + goto cleanup; + cpu->model = cpuModel->model; - cpu->vendor = cpuModel->vendor; cpu->nfeatures = cpuModel->nfeatures; cpu->features = cpuModel->features; VIR_FREE(cpuModel); -- 2.8.2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_map.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c index 6130f8a..066be97 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -105,17 +105,8 @@ int cpuMapLoad(const char *arch, goto cleanup; } - if ((xml = xmlParseFile(mapfile)) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse CPU map file: %s"), - mapfile); + if (!(xml = virXMLParseFileCtxt(mapfile, &ctxt))) goto cleanup; - } - - if ((ctxt = xmlXPathNewContext(xml)) == NULL) { - virReportOOMError(); - goto cleanup; - } virBufferAsprintf(&buf, "./arch[@name='%s']", arch); if (virBufferCheckError(&buf) < 0) -- 2.8.2
On Sat, May 14, 2016 at 10:30:19AM +0200, Jiri Denemark wrote:
Jiri Denemark (17): cpu_x86: Rename struct x86_vendor cpu_x86: Rename struct x86_feature cpu_x86: Rename struct x86_kvm_feature cpu_x86: Rename struct x86_model cpu_x86: Rename struct x86_map cpu_x86: Rename enum compare_result cpu_x86: Rename struct virCPUx86DataIterator cpu_x86: Compare CPU candidates in a separate function cpu_x86: Rename cleanup labels cpu_x86: Use for loop in x86Decode cpu_x86: Remove comparisons to NULL cpu_x86: Simplify insertions into a linked list cpu_x86: Don't ignore parsing errors in x86VendorLoad cpu_x86: Don't ignore parsing errors in x86FeatureLoad cpu_x86: Don't ignore parsing errors in x86ModelLoad cpu_x86: Check vendor early cpu: Properly report errors when parsing CPU map XML
ACK series Pavel
On Mon, May 16, 2016 at 15:12:38 +0200, Pavel Hrdina wrote:
On Sat, May 14, 2016 at 10:30:19AM +0200, Jiri Denemark wrote:
Jiri Denemark (17): cpu_x86: Rename struct x86_vendor cpu_x86: Rename struct x86_feature cpu_x86: Rename struct x86_kvm_feature cpu_x86: Rename struct x86_model cpu_x86: Rename struct x86_map cpu_x86: Rename enum compare_result cpu_x86: Rename struct virCPUx86DataIterator cpu_x86: Compare CPU candidates in a separate function cpu_x86: Rename cleanup labels cpu_x86: Use for loop in x86Decode cpu_x86: Remove comparisons to NULL cpu_x86: Simplify insertions into a linked list cpu_x86: Don't ignore parsing errors in x86VendorLoad cpu_x86: Don't ignore parsing errors in x86FeatureLoad cpu_x86: Don't ignore parsing errors in x86ModelLoad cpu_x86: Check vendor early cpu: Properly report errors when parsing CPU map XML
ACK series
Thanks, pushed. Jirka
participants (2)
-
Jiri Denemark -
Pavel Hrdina