Use the ppc64Driver prefix for all functions that are used to fill in the cpuDriverPPC64 structure, ie. those that are going to be called by the generic CPU code. This makes it clear which functions are exported and which are implementation details; it also gets rid of the ambiguity that affected the ppc64DataFree() function which, despite what the name suggested, was not related to ppc64DataCopy() and could not be used to release the memory allocated for a virCPUppc64Data* instance. No functional changes. --- src/cpu/cpu_ppc64.c | 62 ++++++++++++++++++++++++++--------------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index c3a51fb..5140297 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -448,9 +448,9 @@ ppc64Compute(virCPUDefPtr host, } static virCPUCompareResult -ppc64Compare(virCPUDefPtr host, - virCPUDefPtr cpu, - bool failIncompatible) +ppc64DriverCompare(virCPUDefPtr host, + virCPUDefPtr cpu, + bool failIncompatible) { if ((cpu->arch == VIR_ARCH_NONE || host->arch == cpu->arch) && STREQ(host->model, cpu->model)) @@ -465,12 +465,12 @@ ppc64Compare(virCPUDefPtr host, } static int -ppc64Decode(virCPUDefPtr cpu, - const virCPUData *data, - const char **models, - unsigned int nmodels, - const char *preferred ATTRIBUTE_UNUSED, - unsigned int flags) +ppc64DriverDecode(virCPUDefPtr cpu, + const virCPUData *data, + const char **models, + unsigned int nmodels, + const char *preferred ATTRIBUTE_UNUSED, + unsigned int flags) { int ret = -1; struct ppc64_map *map; @@ -510,7 +510,7 @@ ppc64Decode(virCPUDefPtr cpu, static void -ppc64DataFree(virCPUDataPtr data) +ppc64DriverFree(virCPUDataPtr data) { if (data == NULL) return; @@ -519,7 +519,7 @@ ppc64DataFree(virCPUDataPtr data) } static virCPUDataPtr -ppc64NodeData(virArch arch) +ppc64DriverNodeData(virArch arch) { virCPUDataPtr cpuData; @@ -537,17 +537,17 @@ ppc64NodeData(virArch arch) } static virCPUCompareResult -ppc64GuestData(virCPUDefPtr host, - virCPUDefPtr guest, - virCPUDataPtr *data, - char **message) +ppc64DriverGuestData(virCPUDefPtr host, + virCPUDefPtr guest, + virCPUDataPtr *data, + char **message) { return ppc64Compute(host, guest, data, message); } static int -ppc64Update(virCPUDefPtr guest, - const virCPUDef *host) +ppc64DriverUpdate(virCPUDefPtr guest, + const virCPUDef *host) { switch ((virCPUMode) guest->mode) { case VIR_CPU_MODE_HOST_MODEL: @@ -569,11 +569,11 @@ ppc64Update(virCPUDefPtr guest, } static virCPUDefPtr -ppc64Baseline(virCPUDefPtr *cpus, - unsigned int ncpus, - const char **models ATTRIBUTE_UNUSED, - unsigned int nmodels ATTRIBUTE_UNUSED, - unsigned int flags) +ppc64DriverBaseline(virCPUDefPtr *cpus, + unsigned int ncpus, + const char **models ATTRIBUTE_UNUSED, + unsigned int nmodels ATTRIBUTE_UNUSED, + unsigned int flags) { struct ppc64_map *map = NULL; const struct ppc64_model *model; @@ -653,7 +653,7 @@ ppc64Baseline(virCPUDefPtr *cpus, } static int -ppc64GetModels(char ***models) +ppc64DriverGetModels(char ***models) { struct ppc64_map *map; struct ppc64_model *model; @@ -699,14 +699,14 @@ struct cpuArchDriver cpuDriverPPC64 = { .name = "ppc64", .arch = archs, .narch = ARRAY_CARDINALITY(archs), - .compare = ppc64Compare, - .decode = ppc64Decode, + .compare = ppc64DriverCompare, + .decode = ppc64DriverDecode, .encode = NULL, - .free = ppc64DataFree, - .nodeData = ppc64NodeData, - .guestData = ppc64GuestData, - .baseline = ppc64Baseline, - .update = ppc64Update, + .free = ppc64DriverFree, + .nodeData = ppc64DriverNodeData, + .guestData = ppc64DriverGuestData, + .baseline = ppc64DriverBaseline, + .update = ppc64DriverUpdate, .hasFeature = NULL, - .getModels = ppc64GetModels, + .getModels = ppc64DriverGetModels, }; -- 2.4.3

Use briefer checks, eg. (!model) instead of (model == NULL), and avoid initializing to NULL a pointer that would be assigned in the first line of the function anyway. Also remove a pointless NULL assignment. No functional changes. --- src/cpu/cpu_ppc64.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 5140297..05ff8f2 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -61,7 +61,7 @@ struct ppc64_map { static void ppc64ModelFree(struct ppc64_model *model) { - if (model == NULL) + if (!model) return; VIR_FREE(model->name); @@ -75,7 +75,7 @@ ppc64ModelFind(const struct ppc64_map *map, struct ppc64_model *model; model = map->models; - while (model != NULL) { + while (model) { if (STREQ(model->name, name)) return model; @@ -92,7 +92,7 @@ ppc64ModelFindPVR(const struct ppc64_map *map, struct ppc64_model *model; model = map->models; - while (model != NULL) { + while (model) { if (model->data.pvr == pvr) return model; @@ -158,15 +158,15 @@ static struct ppc64_model * ppc64ModelFromCPU(const virCPUDef *cpu, const struct ppc64_map *map) { - struct ppc64_model *model = NULL; + struct ppc64_model *model; - if ((model = ppc64ModelFind(map, cpu->model)) == NULL) { + if (!(model = ppc64ModelFind(map, cpu->model))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown CPU model %s"), cpu->model); goto error; } - if ((model = ppc64ModelCopy(model)) == NULL) + if (!(model = ppc64ModelCopy(model))) goto error; return model; @@ -181,7 +181,7 @@ static int ppc64VendorLoad(xmlXPathContextPtr ctxt, struct ppc64_map *map) { - struct ppc64_vendor *vendor = NULL; + struct ppc64_vendor *vendor; if (VIR_ALLOC(vendor) < 0) return -1; @@ -264,7 +264,7 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt, } model->data.pvr = pvr; - if (map->models == NULL) { + if (!map->models) { map->models = model; } else { model->next = map->models; @@ -303,16 +303,16 @@ ppc64MapLoadCallback(cpuMapElement element, static void ppc64MapFree(struct ppc64_map *map) { - if (map == NULL) + if (!map) return; - while (map->models != NULL) { + while (map->models) { struct ppc64_model *model = map->models; map->models = model->next; ppc64ModelFree(model); } - while (map->vendors != NULL) { + while (map->vendors) { struct ppc64_vendor *vendor = map->vendors; map->vendors = vendor->next; ppc64VendorFree(vendor); @@ -350,7 +350,6 @@ ppc64MakeCPUData(virArch arch, cpuData->arch = arch; cpuData->data.ppc64 = *data; - data = NULL; return cpuData; } @@ -417,7 +416,7 @@ ppc64Compute(virCPUDefPtr host, !(guest_model = ppc64ModelFromCPU(cpu, map))) goto cleanup; - if (guestData != NULL) { + if (guestData) { if (cpu->type == VIR_CPU_TYPE_GUEST && cpu->match == VIR_CPU_MATCH_STRICT && STRNEQ(guest_model->name, host_model->name)) { @@ -478,7 +477,7 @@ ppc64DriverDecode(virCPUDefPtr cpu, virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); - if (data == NULL || (map = ppc64LoadMap()) == NULL) + if (!data || !(map = ppc64LoadMap())) return -1; if (!(model = ppc64ModelFindPVR(map, data->data.ppc64.pvr))) { @@ -512,7 +511,7 @@ ppc64DriverDecode(virCPUDefPtr cpu, static void ppc64DriverFree(virCPUDataPtr data) { - if (data == NULL) + if (!data) return; VIR_FREE(data); @@ -575,7 +574,7 @@ ppc64DriverBaseline(virCPUDefPtr *cpus, unsigned int nmodels ATTRIBUTE_UNUSED, unsigned int flags) { - struct ppc64_map *map = NULL; + struct ppc64_map *map; const struct ppc64_model *model; const struct ppc64_vendor *vendor = NULL; virCPUDefPtr cpu = NULL; @@ -667,7 +666,7 @@ ppc64DriverGetModels(char ***models) goto error; model = map->models; - while (model != NULL) { + while (model) { if (models) { if (VIR_STRDUP(name, model->name) < 0) goto error; -- 2.4.3

--- src/cpu/cpu_ppc64.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 05ff8f2..18dbf86 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -163,17 +163,10 @@ ppc64ModelFromCPU(const virCPUDef *cpu, if (!(model = ppc64ModelFind(map, cpu->model))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown CPU model %s"), cpu->model); - goto error; + return NULL; } - if (!(model = ppc64ModelCopy(model))) - goto error; - - return model; - - error: - ppc64ModelFree(model); - return NULL; + return ppc64ModelCopy(model); } -- 2.4.3

Having the functions grouped together this way will avoid further shuffling around down the line. No functional changes. --- src/cpu/cpu_ppc64.c | 127 +++++++++++++++++++++++++--------------------------- 1 file changed, 62 insertions(+), 65 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 18dbf86..5921263 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -57,6 +57,32 @@ struct ppc64_map { struct ppc64_model *models; }; +static void +ppc64VendorFree(struct ppc64_vendor *vendor) +{ + if (!vendor) + return; + + VIR_FREE(vendor->name); + VIR_FREE(vendor); +} + +static struct ppc64_vendor * +ppc64VendorFind(const struct ppc64_map *map, + const char *name) +{ + struct ppc64_vendor *vendor; + + vendor = map->vendors; + while (vendor) { + if (STREQ(vendor->name, name)) + return vendor; + + vendor = vendor->next; + } + + return NULL; +} static void ppc64ModelFree(struct ppc64_model *model) @@ -69,6 +95,23 @@ ppc64ModelFree(struct ppc64_model *model) } static struct ppc64_model * +ppc64ModelCopy(const struct ppc64_model *model) +{ + struct ppc64_model *copy; + + if (VIR_ALLOC(copy) < 0 || + VIR_STRDUP(copy->name, model->name) < 0) { + ppc64ModelFree(copy); + return NULL; + } + + copy->data.pvr = model->data.pvr; + copy->vendor = model->vendor; + + return copy; +} + +static struct ppc64_model * ppc64ModelFind(const struct ppc64_map *map, const char *name) { @@ -111,65 +154,41 @@ ppc64ModelFindPVR(const struct ppc64_map *map, } static struct ppc64_model * -ppc64ModelCopy(const struct ppc64_model *model) +ppc64ModelFromCPU(const virCPUDef *cpu, + const struct ppc64_map *map) { - struct ppc64_model *copy; + struct ppc64_model *model; - if (VIR_ALLOC(copy) < 0 || - VIR_STRDUP(copy->name, model->name) < 0) { - ppc64ModelFree(copy); + if (!(model = ppc64ModelFind(map, cpu->model))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown CPU model %s"), cpu->model); return NULL; } - copy->data.pvr = model->data.pvr; - copy->vendor = model->vendor; - - return copy; -} - -static struct ppc64_vendor * -ppc64VendorFind(const struct ppc64_map *map, - const char *name) -{ - struct ppc64_vendor *vendor; - - vendor = map->vendors; - while (vendor) { - if (STREQ(vendor->name, name)) - return vendor; - - vendor = vendor->next; - } - - return NULL; + return ppc64ModelCopy(model); } static void -ppc64VendorFree(struct ppc64_vendor *vendor) +ppc64MapFree(struct ppc64_map *map) { - if (!vendor) + if (!map) return; - VIR_FREE(vendor->name); - VIR_FREE(vendor); -} - -static struct ppc64_model * -ppc64ModelFromCPU(const virCPUDef *cpu, - const struct ppc64_map *map) -{ - struct ppc64_model *model; + while (map->models) { + struct ppc64_model *model = map->models; + map->models = model->next; + ppc64ModelFree(model); + } - if (!(model = ppc64ModelFind(map, cpu->model))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown CPU model %s"), cpu->model); - return NULL; + while (map->vendors) { + struct ppc64_vendor *vendor = map->vendors; + map->vendors = vendor->next; + ppc64VendorFree(vendor); } - return ppc64ModelCopy(model); + VIR_FREE(map); } - static int ppc64VendorLoad(xmlXPathContextPtr ctxt, struct ppc64_map *map) @@ -293,27 +312,6 @@ ppc64MapLoadCallback(cpuMapElement element, return 0; } -static void -ppc64MapFree(struct ppc64_map *map) -{ - if (!map) - return; - - while (map->models) { - struct ppc64_model *model = map->models; - map->models = model->next; - ppc64ModelFree(model); - } - - while (map->vendors) { - struct ppc64_vendor *vendor = map->vendors; - map->vendors = vendor->next; - ppc64VendorFree(vendor); - } - - VIR_FREE(map); -} - static struct ppc64_map * ppc64LoadMap(void) { @@ -500,7 +498,6 @@ ppc64DriverDecode(virCPUDefPtr cpu, return ret; } - static void ppc64DriverFree(virCPUDataPtr data) { -- 2.4.3

The information is not used anywhere in libvirt. No functional changes. --- src/cpu/cpu_map.xml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index b924bd3..6387ce4 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -1385,31 +1385,26 @@ <model name='power6'> <vendor name='IBM'/> - <compat isa='2.05'/> <pvr value='0x003e0000'/> </model> <model name='power7'> <vendor name='IBM'/> - <compat isa='2.06'/> <pvr value='0x003f0000'/> </model> <model name='power7+'> <vendor name='IBM'/> - <compat isa='2.06B'/> <pvr value='0x004a0000'/> </model> <model name='power8e'> <vendor name='IBM'/> - <compat isa='2.07'/> <pvr value='0x004b0000'/> </model> <model name='power8'> <vendor name='IBM'/> - <compat isa='2.07'/> <pvr value='0x004d0000'/> </model> -- 2.4.3

No functional changes. --- tests/cputestdata/ppc64-baseline-1-result.xml | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 tests/cputestdata/ppc64-baseline-1-result.xml diff --git a/tests/cputestdata/ppc64-baseline-1-result.xml b/tests/cputestdata/ppc64-baseline-1-result.xml deleted file mode 100644 index cbdd9bc..0000000 --- a/tests/cputestdata/ppc64-baseline-1-result.xml +++ /dev/null @@ -1,3 +0,0 @@ -<cpu mode='custom' match='exact'> - <model fallback='allow'>POWER7+_v2.1</model> -</cpu> -- 2.4.3

A test is considered successful if the obtained result matches the expected result: if that's not the case, whether because a test that was expected to succeed failed or because a test that was supposed to fail succeeded, then something's not right and we want the user to know about this. On the other hand, if a failure that's unrelated to the bits we're testing occurs, then the user should be notified even if the test was expected to fail. Use different values to tell these two situations apart. Fix a test case that was wrongly expected to fail as well. --- tests/cputest.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/tests/cputest.c b/tests/cputest.c index 06b3f12..93f9d2e 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -247,7 +247,7 @@ static int cpuTestGuestData(const void *arg) { const struct data *data = arg; - int ret = -1; + int ret = -2; virCPUDefPtr host = NULL; virCPUDefPtr cpu = NULL; virCPUDefPtr guest = NULL; @@ -262,8 +262,10 @@ cpuTestGuestData(const void *arg) cmpResult = cpuGuestData(host, cpu, &guestData, NULL); if (cmpResult == VIR_CPU_COMPARE_ERROR || - cmpResult == VIR_CPU_COMPARE_INCOMPATIBLE) + cmpResult == VIR_CPU_COMPARE_INCOMPATIBLE) { + ret = -1; goto cleanup; + } if (VIR_ALLOC(guest) < 0) goto cleanup; @@ -274,10 +276,7 @@ cpuTestGuestData(const void *arg) guest->fallback = cpu->fallback; if (cpuDecode(guest, guestData, data->models, data->nmodels, data->preferred) < 0) { - if (data->result < 0) { - virResetLastError(); - ret = 0; - } + ret = -1; goto cleanup; } @@ -294,7 +293,10 @@ cpuTestGuestData(const void *arg) } result = virBufferContentAndReset(&buf); - ret = cpuTestCompareXML(data->arch, guest, result, false); + if (cpuTestCompareXML(data->arch, guest, result, false) < 0) + goto cleanup; + + ret = 0; cleanup: VIR_FREE(result); @@ -302,6 +304,20 @@ cpuTestGuestData(const void *arg) virCPUDefFree(host); virCPUDefFree(cpu); virCPUDefFree(guest); + + if (ret == data->result) { + /* We got the result we expected, whether it was + * a success or a failure */ + virResetLastError(); + ret = 0; + } else { + VIR_TEST_VERBOSE("\nExpected result %d, got %d\n", + data->result, ret); + /* Pad to line up with test name ... in virTestRun */ + VIR_TEST_VERBOSE("%74s", "... "); + ret = -1; + } + return ret; } @@ -646,7 +662,7 @@ mymain(void) NULL, "Haswell-noTSX", 0); DO_TEST_GUESTDATA("ppc64", "host", "guest", ppc_models, NULL, 0); - DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, "POWER7_v2.1", -1); + DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, "POWER7_v2.1", 0); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.4.3

ppc64Compute(), called by cpuNodeData(), is used not only to retrieve the driver-specific data associated to a guest CPU definition, but also to check whether said guest CPU is compatible with the host CPU. If the user is not interested in the CPU data, it's perfectly fine to pass a NULL pointer instead of a return location, and the compatibility data returned should not be affected by this. One of the checks, specifically the one on CPU model name, was however only performed if the return location was non-NULL. --- src/cpu/cpu_ppc64.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 5921263..53d8fb0 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -407,26 +407,25 @@ ppc64Compute(virCPUDefPtr host, !(guest_model = ppc64ModelFromCPU(cpu, map))) goto cleanup; - if (guestData) { - if (cpu->type == VIR_CPU_TYPE_GUEST && - cpu->match == VIR_CPU_MATCH_STRICT && - STRNEQ(guest_model->name, host_model->name)) { - VIR_DEBUG("host CPU model does not match required CPU model %s", - guest_model->name); - if (message && - virAsprintf(message, - _("host CPU model does not match required " - "CPU model %s"), - guest_model->name) < 0) - goto cleanup; - - ret = VIR_CPU_COMPARE_INCOMPATIBLE; + if (cpu->type == VIR_CPU_TYPE_GUEST && + cpu->match == VIR_CPU_MATCH_STRICT && + STRNEQ(guest_model->name, host_model->name)) { + VIR_DEBUG("host CPU model does not match required CPU model %s", + guest_model->name); + if (message && + virAsprintf(message, + _("host CPU model does not match required " + "CPU model %s"), + guest_model->name) < 0) goto cleanup; - } + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + goto cleanup; + } + + if (guestData) if (!(*guestData = ppc64MakeCPUData(arch, &guest_model->data))) goto cleanup; - } ret = VIR_CPU_COMPARE_IDENTICAL; -- 2.4.3

Limitations of the POWER architecture mean that you can't run eg. a POWER7 guest on a POWER8 host when using KVM. This applies to all guests, not just those using VIR_CPU_MATCH_STRICT in the CPU definition; in fact, exact and strict CPU matching are basically the same on ppc64. This means, of course, that hosts using different CPUs have to be considered incompatible as well. Change ppc64Compute(), called by cpuGuestData(), to reflect this fact and update test cases accordingly. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1250977 --- src/cpu/cpu_ppc64.c | 5 +---- tests/cputest.c | 4 ++-- tests/cputestdata/ppc64-guest.xml | 2 +- tests/cputestdata/ppc64-host+guest,ppc_models-result.xml | 2 +- .../ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml | 5 ----- 5 files changed, 5 insertions(+), 13 deletions(-) delete mode 100644 tests/cputestdata/ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 53d8fb0..0769956 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -350,7 +350,6 @@ ppc64Compute(virCPUDefPtr host, const virCPUDef *cpu, virCPUDataPtr *guestData, char **message) - { struct ppc64_map *map = NULL; struct ppc64_model *host_model = NULL; @@ -407,9 +406,7 @@ ppc64Compute(virCPUDefPtr host, !(guest_model = ppc64ModelFromCPU(cpu, map))) goto cleanup; - if (cpu->type == VIR_CPU_TYPE_GUEST && - cpu->match == VIR_CPU_MATCH_STRICT && - STRNEQ(guest_model->name, host_model->name)) { + if (STRNEQ(guest_model->name, host_model->name)) { VIR_DEBUG("host CPU model does not match required CPU model %s", guest_model->name); if (message && diff --git a/tests/cputest.c b/tests/cputest.c index 93f9d2e..1e84fd3 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -501,7 +501,7 @@ static const char *model486[] = { "486" }; static const char *nomodel[] = { "nomodel" }; static const char *models[] = { "qemu64", "core2duo", "Nehalem" }; static const char *haswell[] = { "SandyBridge", "Haswell" }; -static const char *ppc_models[] = { "POWER7", "POWER7_v2.1", "POWER8_v1.0"}; +static const char *ppc_models[] = { "POWER7", "POWER7_v2.1", "POWER7_v2.3", "POWER8_v1.0"}; static int mymain(void) @@ -662,7 +662,7 @@ mymain(void) NULL, "Haswell-noTSX", 0); DO_TEST_GUESTDATA("ppc64", "host", "guest", ppc_models, NULL, 0); - DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, "POWER7_v2.1", 0); + DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, "POWER7_v2.1", -1); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/cputestdata/ppc64-guest.xml b/tests/cputestdata/ppc64-guest.xml index ac81ec0..9e91501 100644 --- a/tests/cputestdata/ppc64-guest.xml +++ b/tests/cputestdata/ppc64-guest.xml @@ -1,4 +1,4 @@ <cpu match='exact'> - <model>POWER8_v1.0</model> + <model>POWER7_v2.3</model> <topology sockets='2' cores='4' threads='1'/> </cpu> diff --git a/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml b/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml index 0cb0830..3e55f68 100644 --- a/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml +++ b/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml @@ -1,5 +1,5 @@ <cpu mode='custom' match='exact'> <arch>ppc64</arch> - <model fallback='allow'>POWER8_v1.0</model> + <model fallback='allow'>POWER7_v2.3</model> <vendor>IBM</vendor> </cpu> diff --git a/tests/cputestdata/ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml b/tests/cputestdata/ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml deleted file mode 100644 index 7e58361..0000000 --- a/tests/cputestdata/ppc64-host+guest-nofallback,ppc_models,POWER7_v2.1-result.xml +++ /dev/null @@ -1,5 +0,0 @@ -<cpu mode='custom' match='exact'> - <arch>ppc64</arch> - <model fallback='forbid'>POWER7_v2.1</model> - <vendor>IBM</vendor> -</cpu> -- 2.4.3

This ensures comparison of two CPU definitions will be consistent regardless of the fact that it is performed using cpuCompare() or cpuGuestData(). The x86 driver uses the same exact code. --- src/cpu/cpu_ppc64.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 0769956..efac739 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -438,16 +438,22 @@ ppc64DriverCompare(virCPUDefPtr host, virCPUDefPtr cpu, bool failIncompatible) { - if ((cpu->arch == VIR_ARCH_NONE || host->arch == cpu->arch) && - STREQ(host->model, cpu->model)) - return VIR_CPU_COMPARE_IDENTICAL; + virCPUCompareResult ret; + char *message = NULL; - if (failIncompatible) { - virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); - return VIR_CPU_COMPARE_ERROR; - } else { - return VIR_CPU_COMPARE_INCOMPATIBLE; + ret = ppc64Compute(host, cpu, NULL, &message); + + if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { + ret = VIR_CPU_COMPARE_ERROR; + if (message) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message); + } else { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); + } } + VIR_FREE(message); + + return ret; } static int -- 2.4.3

The upcoming commits will make heavy modifications to the ppc64 driver, split so that it's easier to review the changes. Instead of updating the test cases so that they pass, possibly only to update them again with the following commit, disable them for the time being. Another commit will update them all in one go once all required changes are in place. --- tests/cputest.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/cputest.c b/tests/cputest.c index 1e84fd3..4dbccfd 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -501,7 +501,9 @@ static const char *model486[] = { "486" }; static const char *nomodel[] = { "nomodel" }; static const char *models[] = { "qemu64", "core2duo", "Nehalem" }; static const char *haswell[] = { "SandyBridge", "Haswell" }; +/* XXX temporarily disabled static const char *ppc_models[] = { "POWER7", "POWER7_v2.1", "POWER7_v2.3", "POWER8_v1.0"}; +*/ static int mymain(void) @@ -595,8 +597,10 @@ mymain(void) DO_TEST_COMPARE("x86", "host-worse", "nehalem-force", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_COMPARE("x86", "host-SandyBridge", "exact-force-Haswell", VIR_CPU_COMPARE_IDENTICAL); + /* XXX temporarily disabled DO_TEST_COMPARE("ppc64", "host", "strict", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_COMPARE("ppc64", "host", "exact", VIR_CPU_COMPARE_INCOMPATIBLE); + */ /* guest updates for migration * automatically compares host CPU with the result */ @@ -625,8 +629,11 @@ mymain(void) DO_TEST_BASELINE("x86", "7", 0, 0); DO_TEST_BASELINE("x86", "8", 0, 0); + /* XXX temporarily disabled DO_TEST_BASELINE("ppc64", "incompatible-vendors", 0, -1); DO_TEST_BASELINE("ppc64", "no-vendor", 0, 0); + */ + /* CPU features */ DO_TEST_HASFEATURE("x86", "host", "vmx", YES); DO_TEST_HASFEATURE("x86", "host", "lm", YES); @@ -661,8 +668,10 @@ mymain(void) DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell-noTSX", NULL, "Haswell-noTSX", 0); + /* XXX temporarily disabled DO_TEST_GUESTDATA("ppc64", "host", "guest", ppc_models, NULL, 0); DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, "POWER7_v2.1", -1); + */ return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.4.3

Use a typedef instead of the plain struct and heap allocation. This will make it easier to extend the ppc64 specific CPU data later on. --- src/cpu/cpu.h | 2 +- src/cpu/cpu_ppc64.c | 86 ++++++++++++++++++++++++++++++++++++------------ src/cpu/cpu_ppc64_data.h | 3 +- 3 files changed, 68 insertions(+), 23 deletions(-) diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 49d4226..7375876 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -38,7 +38,7 @@ struct _virCPUData { virArch arch; union { virCPUx86Data *x86; - struct cpuPPC64Data ppc64; + virCPUppc64Data *ppc64; /* generic driver needs no data */ } data; }; diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index efac739..6c78ab8 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -48,7 +48,7 @@ struct ppc64_vendor { struct ppc64_model { char *name; const struct ppc64_vendor *vendor; - struct cpuPPC64Data data; + virCPUppc64Data *data; struct ppc64_model *next; }; @@ -58,6 +58,25 @@ struct ppc64_map { }; static void +ppc64DataFree(virCPUppc64Data *data) +{ + VIR_FREE(data); +} + +static virCPUppc64Data * +ppc64DataCopy(const virCPUppc64Data *data) +{ + virCPUppc64Data *copy; + + if (VIR_ALLOC(copy) < 0) + return NULL; + + copy->pvr = data->pvr; + + return copy; +} + +static void ppc64VendorFree(struct ppc64_vendor *vendor) { if (!vendor) @@ -90,6 +109,7 @@ ppc64ModelFree(struct ppc64_model *model) if (!model) return; + ppc64DataFree(model->data); VIR_FREE(model->name); VIR_FREE(model); } @@ -99,16 +119,22 @@ ppc64ModelCopy(const struct ppc64_model *model) { struct ppc64_model *copy; - if (VIR_ALLOC(copy) < 0 || - VIR_STRDUP(copy->name, model->name) < 0) { - ppc64ModelFree(copy); - return NULL; - } + if (VIR_ALLOC(copy) < 0) + goto error; + + if (VIR_STRDUP(copy->name, model->name) < 0) + goto error; + + if (!(copy->data = ppc64DataCopy(model->data))) + goto error; - copy->data.pvr = model->data.pvr; copy->vendor = model->vendor; return copy; + + error: + ppc64ModelFree(copy); + return NULL; } static struct ppc64_model * @@ -136,7 +162,7 @@ ppc64ModelFindPVR(const struct ppc64_map *map, model = map->models; while (model) { - if (model->data.pvr == pvr) + if (model->data->pvr == pvr) return model; model = model->next; @@ -237,6 +263,11 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt, if (VIR_ALLOC(model) < 0) return -1; + if (VIR_ALLOC(model->data) < 0) { + ppc64ModelFree(model); + return -1; + } + model->name = virXPathString("string(@name)", ctxt); if (!model->name) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -274,7 +305,7 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt, model->name); goto ignore; } - model->data.pvr = pvr; + model->data->pvr = pvr; if (!map->models) { map->models = model; @@ -318,7 +349,7 @@ ppc64LoadMap(void) struct ppc64_map *map; if (VIR_ALLOC(map) < 0) - return NULL; + goto error; if (cpuMapLoad("ppc64", ppc64MapLoadCallback, map) < 0) goto error; @@ -332,7 +363,7 @@ ppc64LoadMap(void) static virCPUDataPtr ppc64MakeCPUData(virArch arch, - struct cpuPPC64Data *data) + virCPUppc64Data *data) { virCPUDataPtr cpuData; @@ -340,7 +371,9 @@ ppc64MakeCPUData(virArch arch, return NULL; cpuData->arch = arch; - cpuData->data.ppc64 = *data; + + if (!(cpuData->data.ppc64 = ppc64DataCopy(data))) + VIR_FREE(cpuData); return cpuData; } @@ -421,7 +454,7 @@ ppc64Compute(virCPUDefPtr host, } if (guestData) - if (!(*guestData = ppc64MakeCPUData(arch, &guest_model->data))) + if (!(*guestData = ppc64MakeCPUData(arch, guest_model->data))) goto cleanup; ret = VIR_CPU_COMPARE_IDENTICAL; @@ -473,10 +506,10 @@ ppc64DriverDecode(virCPUDefPtr cpu, if (!data || !(map = ppc64LoadMap())) return -1; - if (!(model = ppc64ModelFindPVR(map, data->data.ppc64.pvr))) { + if (!(model = ppc64ModelFindPVR(map, data->data.ppc64->pvr))) { virReportError(VIR_ERR_OPERATION_FAILED, _("Cannot find CPU model with PVR 0x%08x"), - data->data.ppc64.pvr); + data->data.ppc64->pvr); goto cleanup; } @@ -506,25 +539,36 @@ ppc64DriverFree(virCPUDataPtr data) if (!data) return; + ppc64DataFree(data->data.ppc64); VIR_FREE(data); } static virCPUDataPtr ppc64DriverNodeData(virArch arch) { - virCPUDataPtr cpuData; + virCPUDataPtr nodeData; + virCPUppc64Data *data; - if (VIR_ALLOC(cpuData) < 0) - return NULL; + if (VIR_ALLOC(nodeData) < 0) + goto error; - cpuData->arch = arch; + if (VIR_ALLOC(data) < 0) + goto error; + + data = nodeData->data.ppc64; #if defined(__powerpc__) || defined(__powerpc64__) asm("mfpvr %0" - : "=r" (cpuData->data.ppc64.pvr)); + : "=r" (data->pvr)); #endif - return cpuData; + nodeData->arch = arch; + + return nodeData; + + error: + ppc64DriverFree(nodeData); + return NULL; } static virCPUCompareResult diff --git a/src/cpu/cpu_ppc64_data.h b/src/cpu/cpu_ppc64_data.h index 45152de..c18fc63 100644 --- a/src/cpu/cpu_ppc64_data.h +++ b/src/cpu/cpu_ppc64_data.h @@ -26,7 +26,8 @@ # include <stdint.h> -struct cpuPPC64Data { +typedef struct _virCPUppc64Data virCPUppc64Data; +struct _virCPUppc64Data { uint32_t pvr; }; -- 2.4.3

This will allow us to perform PVR matching more broadly, eg. consider both POWER8 and POWER8E CPUs to be the same even though they have different PVR values. --- src/cpu/cpu_ppc64.c | 72 ++++++++++++++++++++++++++++++++++++++++-------- src/cpu/cpu_ppc64_data.h | 8 +++++- 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 6c78ab8..db25f27 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -60,6 +60,10 @@ struct ppc64_map { static void ppc64DataFree(virCPUppc64Data *data) { + if (!data) + return; + + VIR_FREE(data->pvr); VIR_FREE(data); } @@ -67,13 +71,24 @@ static virCPUppc64Data * ppc64DataCopy(const virCPUppc64Data *data) { virCPUppc64Data *copy; + size_t i; if (VIR_ALLOC(copy) < 0) - return NULL; + goto error; + + if (VIR_ALLOC_N(copy->pvr, data->len) < 0) + goto error; + + copy->len = data->len; - copy->pvr = data->pvr; + for (i = 0; i < data->len; i++) + copy->pvr[i].value = data->pvr[i].value; return copy; + + error: + ppc64DataFree(copy); + return NULL; } static void @@ -159,12 +174,14 @@ ppc64ModelFindPVR(const struct ppc64_map *map, uint32_t pvr) { struct ppc64_model *model; + size_t i; model = map->models; while (model) { - if (model->data->pvr == pvr) - return model; - + for (i = 0; i < model->data->len; i++) { + if (model->data->pvr[i].value == pvr) + return model; + } model = model->next; } @@ -257,8 +274,16 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt, struct ppc64_map *map) { struct ppc64_model *model; + xmlNodePtr *nodes = NULL; + xmlNodePtr bookmark; char *vendor = NULL; unsigned long pvr; + size_t i; + int n; + + /* Save the node the context was pointing to, as we're going + * to change it later. It's going to be restored on exit */ + bookmark = ctxt->node; if (VIR_ALLOC(model) < 0) return -1; @@ -298,14 +323,30 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt, } } - if (!virXPathBoolean("boolean(./pvr)", ctxt) || - virXPathULongHex("string(./pvr/@value)", ctxt, &pvr) < 0) { + if ((n = virXPathNodeSet("./pvr", ctxt, &nodes)) <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Missing or invalid PVR value in CPU model %s"), + _("Missing PVR information for CPU model %s"), model->name); goto ignore; } - model->data->pvr = pvr; + + if (VIR_ALLOC_N(model->data->pvr, n) < 0) + goto ignore; + + model->data->len = n; + + for (i = 0; i < n; i++) { + ctxt->node = nodes[i]; + + if (!virXPathBoolean("boolean(./@value)", ctxt) || + virXPathULongHex("string(./@value)", ctxt, &pvr) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing or invalid PVR value in CPU model %s"), + model->name); + goto ignore; + } + model->data->pvr[i].value = pvr; + } if (!map->models) { map->models = model; @@ -315,7 +356,9 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt, } cleanup: + ctxt->node = bookmark; VIR_FREE(vendor); + VIR_FREE(nodes); return 0; ignore: @@ -506,10 +549,10 @@ ppc64DriverDecode(virCPUDefPtr cpu, if (!data || !(map = ppc64LoadMap())) return -1; - if (!(model = ppc64ModelFindPVR(map, data->data.ppc64->pvr))) { + if (!(model = ppc64ModelFindPVR(map, data->data.ppc64->pvr[0].value))) { virReportError(VIR_ERR_OPERATION_FAILED, _("Cannot find CPU model with PVR 0x%08x"), - data->data.ppc64->pvr); + data->data.ppc64->pvr[0].value); goto cleanup; } @@ -557,9 +600,14 @@ ppc64DriverNodeData(virArch arch) data = nodeData->data.ppc64; + if (VIR_ALLOC_N(data->pvr, 1) < 0) + goto error; + + data->len = 1; + #if defined(__powerpc__) || defined(__powerpc64__) asm("mfpvr %0" - : "=r" (data->pvr)); + : "=r" (data->pvr[0].value)); #endif nodeData->arch = arch; diff --git a/src/cpu/cpu_ppc64_data.h b/src/cpu/cpu_ppc64_data.h index c18fc63..0d3cb0b 100644 --- a/src/cpu/cpu_ppc64_data.h +++ b/src/cpu/cpu_ppc64_data.h @@ -26,9 +26,15 @@ # include <stdint.h> +typedef struct _virCPUppc64PVR virCPUppc64PVR; +struct _virCPUppc64PVR { + uint32_t value; +}; + typedef struct _virCPUppc64Data virCPUppc64Data; struct _virCPUppc64Data { - uint32_t pvr; + size_t len; + virCPUppc64PVR *pvr; }; #endif /* __VIR_CPU_PPC64_DATA_H__ */ -- 2.4.3

On Mon, Aug 10, 2015 at 10:55:58 +0200, Andrea Bolognani wrote: ACK Jirka

On Mon, Aug 10, 2015 at 10:55:59 +0200, Andrea Bolognani wrote: I think ppc64ConvertLegacyCPUDef would be a slightly better name. When reading the tests, I realized we don't do any compatibility hacks for *Baseline, which is correct because it never worked on Power, but we should explicitly document it inside ppc64Baseline to avoid confusion in the future. ACK with the comment added. Jirka

Unlike what happens on x86, on ppc64 you can't mix and match CPU features to obtain the guest CPU you want regardless of the host CPU, so the concept of model fallback doesn't apply. Make sure CPU definitions emitted by the driver, eg. as output of the cpuBaseline() and cpuUpdate() calls, reflect this fact. --- src/cpu/cpu_ppc64.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index d87e0d1..0f193cd 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -672,6 +672,7 @@ ppc64DriverUpdate(virCPUDefPtr guest, case VIR_CPU_MODE_HOST_MODEL: case VIR_CPU_MODE_HOST_PASSTHROUGH: guest->match = VIR_CPU_MATCH_EXACT; + guest->fallback = VIR_CPU_FALLBACK_FORBID; virCPUDefFreeModel(guest); return virCPUDefCopyModel(guest, host, true); @@ -759,6 +760,7 @@ ppc64DriverBaseline(virCPUDefPtr *cpus, cpu->type = VIR_CPU_TYPE_GUEST; cpu->match = VIR_CPU_MATCH_EXACT; + cpu->fallback = VIR_CPU_FALLBACK_FORBID; cleanup: ppc64MapFree(map); -- 2.4.3

Now that all the changes have been implemented we can run the test cases once again, after updating them to reflect the new behaviour. --- tests/cputest.c | 16 ++++------------ .../cputestdata/ppc64-baseline-incompatible-vendors.xml | 4 ++-- tests/cputestdata/ppc64-baseline-no-vendor-result.xml | 2 +- tests/cputestdata/ppc64-baseline-no-vendor.xml | 2 +- tests/cputestdata/ppc64-exact.xml | 3 --- tests/cputestdata/ppc64-guest-exact.xml | 3 +++ tests/cputestdata/ppc64-guest-nofallback.xml | 3 +-- tests/cputestdata/ppc64-guest-strict.xml | 3 +++ tests/cputestdata/ppc64-guest.xml | 3 +-- tests/cputestdata/ppc64-host+guest,ppc_models-result.xml | 2 +- tests/cputestdata/ppc64-host.xml | 2 +- tests/cputestdata/ppc64-strict.xml | 3 --- 12 files changed, 18 insertions(+), 28 deletions(-) delete mode 100644 tests/cputestdata/ppc64-exact.xml create mode 100644 tests/cputestdata/ppc64-guest-exact.xml create mode 100644 tests/cputestdata/ppc64-guest-strict.xml delete mode 100644 tests/cputestdata/ppc64-strict.xml diff --git a/tests/cputest.c b/tests/cputest.c index 4dbccfd..09f690a 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -501,9 +501,7 @@ static const char *model486[] = { "486" }; static const char *nomodel[] = { "nomodel" }; static const char *models[] = { "qemu64", "core2duo", "Nehalem" }; static const char *haswell[] = { "SandyBridge", "Haswell" }; -/* XXX temporarily disabled -static const char *ppc_models[] = { "POWER7", "POWER7_v2.1", "POWER7_v2.3", "POWER8_v1.0"}; -*/ +static const char *ppc_models[] = { "POWER6", "POWER7", "POWER8" }; static int mymain(void) @@ -597,10 +595,8 @@ mymain(void) DO_TEST_COMPARE("x86", "host-worse", "nehalem-force", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_COMPARE("x86", "host-SandyBridge", "exact-force-Haswell", VIR_CPU_COMPARE_IDENTICAL); - /* XXX temporarily disabled - DO_TEST_COMPARE("ppc64", "host", "strict", VIR_CPU_COMPARE_IDENTICAL); - DO_TEST_COMPARE("ppc64", "host", "exact", VIR_CPU_COMPARE_INCOMPATIBLE); - */ + DO_TEST_COMPARE("ppc64", "host", "guest-strict", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_COMPARE("ppc64", "host", "guest-exact", VIR_CPU_COMPARE_INCOMPATIBLE); /* guest updates for migration * automatically compares host CPU with the result */ @@ -629,10 +625,8 @@ mymain(void) DO_TEST_BASELINE("x86", "7", 0, 0); DO_TEST_BASELINE("x86", "8", 0, 0); - /* XXX temporarily disabled DO_TEST_BASELINE("ppc64", "incompatible-vendors", 0, -1); DO_TEST_BASELINE("ppc64", "no-vendor", 0, 0); - */ /* CPU features */ DO_TEST_HASFEATURE("x86", "host", "vmx", YES); @@ -668,10 +662,8 @@ mymain(void) DO_TEST_GUESTDATA("x86", "host-Haswell-noTSX", "Haswell-noTSX", NULL, "Haswell-noTSX", 0); - /* XXX temporarily disabled DO_TEST_GUESTDATA("ppc64", "host", "guest", ppc_models, NULL, 0); - DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, "POWER7_v2.1", -1); - */ + DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, "POWER8", -1); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/cputestdata/ppc64-baseline-incompatible-vendors.xml b/tests/cputestdata/ppc64-baseline-incompatible-vendors.xml index 97d3c9c..9e67e9d 100644 --- a/tests/cputestdata/ppc64-baseline-incompatible-vendors.xml +++ b/tests/cputestdata/ppc64-baseline-incompatible-vendors.xml @@ -1,13 +1,13 @@ <cpuTest> <cpu> <arch>ppc64</arch> - <model>POWER7+_v2.1</model> + <model>POWER7</model> <vendor>Intel</vendor> <topology sockets='2' cores='4' threads='1'/> </cpu> <cpu> <arch>ppc64</arch> - <model>POWER8_v1.0</model> + <model>POWER8</model> <vendor>Intel</vendor> <topology sockets='1' cores='1' threads='1'/> </cpu> diff --git a/tests/cputestdata/ppc64-baseline-no-vendor-result.xml b/tests/cputestdata/ppc64-baseline-no-vendor-result.xml index 36bae52..758099c 100644 --- a/tests/cputestdata/ppc64-baseline-no-vendor-result.xml +++ b/tests/cputestdata/ppc64-baseline-no-vendor-result.xml @@ -1,3 +1,3 @@ <cpu mode='custom' match='exact'> - <model fallback='allow'>POWER7_v2.3</model> + <model fallback='forbid'>POWER7</model> </cpu> diff --git a/tests/cputestdata/ppc64-baseline-no-vendor.xml b/tests/cputestdata/ppc64-baseline-no-vendor.xml index 5e69a62..6d8dd0d 100644 --- a/tests/cputestdata/ppc64-baseline-no-vendor.xml +++ b/tests/cputestdata/ppc64-baseline-no-vendor.xml @@ -1,7 +1,7 @@ <cpuTest> <cpu> <arch>ppc64</arch> - <model>POWER7_v2.3</model> + <model>POWER7</model> <topology sockets='2' cores='4' threads='1'/> </cpu> </cpuTest> diff --git a/tests/cputestdata/ppc64-exact.xml b/tests/cputestdata/ppc64-exact.xml deleted file mode 100644 index c84f16a..0000000 --- a/tests/cputestdata/ppc64-exact.xml +++ /dev/null @@ -1,3 +0,0 @@ -<cpu match='exact'> - <model>POWER8_v1.0</model> -</cpu> diff --git a/tests/cputestdata/ppc64-guest-exact.xml b/tests/cputestdata/ppc64-guest-exact.xml new file mode 100644 index 0000000..f416a59 --- /dev/null +++ b/tests/cputestdata/ppc64-guest-exact.xml @@ -0,0 +1,3 @@ +<cpu match='exact'> + <model>POWER8</model> +</cpu> diff --git a/tests/cputestdata/ppc64-guest-nofallback.xml b/tests/cputestdata/ppc64-guest-nofallback.xml index 42026b4..070f006 100644 --- a/tests/cputestdata/ppc64-guest-nofallback.xml +++ b/tests/cputestdata/ppc64-guest-nofallback.xml @@ -1,4 +1,3 @@ <cpu match='exact'> - <model fallback='forbid'>POWER7_v2.1</model> - <topology sockets='2' cores='4' threads='1'/> + <model fallback='forbid'>POWER8</model> </cpu> diff --git a/tests/cputestdata/ppc64-guest-strict.xml b/tests/cputestdata/ppc64-guest-strict.xml new file mode 100644 index 0000000..217dfc7 --- /dev/null +++ b/tests/cputestdata/ppc64-guest-strict.xml @@ -0,0 +1,3 @@ +<cpu match='strict'> + <model>POWER7</model> +</cpu> diff --git a/tests/cputestdata/ppc64-guest.xml b/tests/cputestdata/ppc64-guest.xml index 9e91501..a60c59f 100644 --- a/tests/cputestdata/ppc64-guest.xml +++ b/tests/cputestdata/ppc64-guest.xml @@ -1,4 +1,3 @@ <cpu match='exact'> - <model>POWER7_v2.3</model> - <topology sockets='2' cores='4' threads='1'/> + <model fallback='allow'>POWER7</model> </cpu> diff --git a/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml b/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml index 3e55f68..3548c0e 100644 --- a/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml +++ b/tests/cputestdata/ppc64-host+guest,ppc_models-result.xml @@ -1,5 +1,5 @@ <cpu mode='custom' match='exact'> <arch>ppc64</arch> - <model fallback='allow'>POWER7_v2.3</model> + <model fallback='allow'>POWER7</model> <vendor>IBM</vendor> </cpu> diff --git a/tests/cputestdata/ppc64-host.xml b/tests/cputestdata/ppc64-host.xml index 39cb741..0ac5c4e 100644 --- a/tests/cputestdata/ppc64-host.xml +++ b/tests/cputestdata/ppc64-host.xml @@ -1,6 +1,6 @@ <cpu> <arch>ppc64</arch> - <model>POWER7_v2.3</model> + <model>POWER7</model> <vendor>IBM</vendor> <topology sockets='1' cores='64' threads='1'/> </cpu> diff --git a/tests/cputestdata/ppc64-strict.xml b/tests/cputestdata/ppc64-strict.xml deleted file mode 100644 index e91c6e7..0000000 --- a/tests/cputestdata/ppc64-strict.xml +++ /dev/null @@ -1,3 +0,0 @@ -<cpu match='exact'> - <model>POWER7_v2.3</model> -</cpu> -- 2.4.3

The test cases cover the cpuCompare(), cpuBaseline() and cpuNodeData() implementation. --- tests/cputest.c | 16 ++++++++++++++++ tests/cputestdata/ppc64-baseline-incompatible-models.xml | 14 ++++++++++++++ tests/cputestdata/ppc64-baseline-legacy.xml | 14 ++++++++++++++ tests/cputestdata/ppc64-baseline-same-model-result.xml | 3 +++ tests/cputestdata/ppc64-baseline-same-model.xml | 14 ++++++++++++++ tests/cputestdata/ppc64-guest-legacy-incompatible.xml | 3 +++ tests/cputestdata/ppc64-guest-legacy-invalid.xml | 3 +++ tests/cputestdata/ppc64-guest-legacy.xml | 3 +++ .../ppc64-host+guest-legacy,ppc_models-result.xml | 5 +++++ tests/cputestdata/ppc64-host-better.xml | 6 ++++++ tests/cputestdata/ppc64-host-incomp-arch.xml | 6 ++++++ tests/cputestdata/ppc64-host-no-vendor.xml | 5 +++++ tests/cputestdata/ppc64-host-worse.xml | 6 ++++++ 13 files changed, 98 insertions(+) create mode 100644 tests/cputestdata/ppc64-baseline-incompatible-models.xml create mode 100644 tests/cputestdata/ppc64-baseline-legacy.xml create mode 100644 tests/cputestdata/ppc64-baseline-same-model-result.xml create mode 100644 tests/cputestdata/ppc64-baseline-same-model.xml create mode 100644 tests/cputestdata/ppc64-guest-legacy-incompatible.xml create mode 100644 tests/cputestdata/ppc64-guest-legacy-invalid.xml create mode 100644 tests/cputestdata/ppc64-guest-legacy.xml create mode 100644 tests/cputestdata/ppc64-host+guest-legacy,ppc_models-result.xml create mode 100644 tests/cputestdata/ppc64-host-better.xml create mode 100644 tests/cputestdata/ppc64-host-incomp-arch.xml create mode 100644 tests/cputestdata/ppc64-host-no-vendor.xml create mode 100644 tests/cputestdata/ppc64-host-worse.xml diff --git a/tests/cputest.c b/tests/cputest.c index 09f690a..5992dd0 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -571,6 +571,13 @@ mymain(void) DO_TEST_COMPARE("x86", "host", "host-no-vendor", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_COMPARE("x86", "host-no-vendor", "host", VIR_CPU_COMPARE_INCOMPATIBLE); + DO_TEST_COMPARE("ppc64", "host", "host", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_COMPARE("ppc64", "host", "host-better", VIR_CPU_COMPARE_INCOMPATIBLE); + DO_TEST_COMPARE("ppc64", "host", "host-worse", VIR_CPU_COMPARE_INCOMPATIBLE); + DO_TEST_COMPARE("ppc64", "host", "host-incomp-arch", VIR_CPU_COMPARE_INCOMPATIBLE); + DO_TEST_COMPARE("ppc64", "host", "host-no-vendor", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_COMPARE("ppc64", "host-no-vendor", "host", VIR_CPU_COMPARE_INCOMPATIBLE); + /* guest to host comparison */ DO_TEST_COMPARE("x86", "host", "bogus-model", VIR_CPU_COMPARE_ERROR); DO_TEST_COMPARE("x86", "host", "bogus-feature", VIR_CPU_COMPARE_ERROR); @@ -597,6 +604,9 @@ mymain(void) DO_TEST_COMPARE("ppc64", "host", "guest-strict", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_COMPARE("ppc64", "host", "guest-exact", VIR_CPU_COMPARE_INCOMPATIBLE); + DO_TEST_COMPARE("ppc64", "host", "guest-legacy", VIR_CPU_COMPARE_IDENTICAL); + DO_TEST_COMPARE("ppc64", "host", "guest-legacy-incompatible", VIR_CPU_COMPARE_INCOMPATIBLE); + DO_TEST_COMPARE("ppc64", "host", "guest-legacy-invalid", VIR_CPU_COMPARE_ERROR); /* guest updates for migration * automatically compares host CPU with the result */ @@ -627,6 +637,9 @@ mymain(void) DO_TEST_BASELINE("ppc64", "incompatible-vendors", 0, -1); DO_TEST_BASELINE("ppc64", "no-vendor", 0, 0); + DO_TEST_BASELINE("ppc64", "incompatible-models", 0, -1); + DO_TEST_BASELINE("ppc64", "same-model", 0, 0); + DO_TEST_BASELINE("ppc64", "legacy", 0, -1); /* CPU features */ DO_TEST_HASFEATURE("x86", "host", "vmx", YES); @@ -664,6 +677,9 @@ mymain(void) DO_TEST_GUESTDATA("ppc64", "host", "guest", ppc_models, NULL, 0); DO_TEST_GUESTDATA("ppc64", "host", "guest-nofallback", ppc_models, "POWER8", -1); + DO_TEST_GUESTDATA("ppc64", "host", "guest-legacy", ppc_models, NULL, 0); + DO_TEST_GUESTDATA("ppc64", "host", "guest-legacy-incompatible", ppc_models, NULL, -1); + DO_TEST_GUESTDATA("ppc64", "host", "guest-legacy-invalid", ppc_models, NULL, -1); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/cputestdata/ppc64-baseline-incompatible-models.xml b/tests/cputestdata/ppc64-baseline-incompatible-models.xml new file mode 100644 index 0000000..7e7b9a6 --- /dev/null +++ b/tests/cputestdata/ppc64-baseline-incompatible-models.xml @@ -0,0 +1,14 @@ +<cpuTest> +<cpu> + <arch>ppc64</arch> + <model>POWER7</model> + <vendor>IBM</vendor> + <topology sockets='2' cores='4' threads='1'/> +</cpu> +<cpu> + <arch>ppc64</arch> + <model>POWER8</model> + <vendor>IBM</vendor> + <topology sockets='1' cores='1' threads='1'/> +</cpu> +</cpuTest> diff --git a/tests/cputestdata/ppc64-baseline-legacy.xml b/tests/cputestdata/ppc64-baseline-legacy.xml new file mode 100644 index 0000000..b652497 --- /dev/null +++ b/tests/cputestdata/ppc64-baseline-legacy.xml @@ -0,0 +1,14 @@ +<cpuTest> +<cpu> + <arch>ppc64</arch> + <model>POWER8</model> + <vendor>IBM</vendor> + <topology sockets='2' cores='4' threads='1'/> +</cpu> +<cpu> + <arch>ppc64</arch> + <model>power8e</model> + <vendor>IBM</vendor> + <topology sockets='1' cores='1' threads='1'/> +</cpu> +</cpuTest> diff --git a/tests/cputestdata/ppc64-baseline-same-model-result.xml b/tests/cputestdata/ppc64-baseline-same-model-result.xml new file mode 100644 index 0000000..0223018 --- /dev/null +++ b/tests/cputestdata/ppc64-baseline-same-model-result.xml @@ -0,0 +1,3 @@ +<cpu mode='custom' match='exact'> + <model fallback='forbid'>POWER8</model> +</cpu> diff --git a/tests/cputestdata/ppc64-baseline-same-model.xml b/tests/cputestdata/ppc64-baseline-same-model.xml new file mode 100644 index 0000000..dceae83 --- /dev/null +++ b/tests/cputestdata/ppc64-baseline-same-model.xml @@ -0,0 +1,14 @@ +<cpuTest> +<cpu> + <arch>ppc64</arch> + <model>POWER8</model> + <vendor>IBM</vendor> + <topology sockets='2' cores='4' threads='1'/> +</cpu> +<cpu> + <arch>ppc64</arch> + <model>POWER8</model> + <vendor>IBM</vendor> + <topology sockets='1' cores='1' threads='1'/> +</cpu> +</cpuTest> diff --git a/tests/cputestdata/ppc64-guest-legacy-incompatible.xml b/tests/cputestdata/ppc64-guest-legacy-incompatible.xml new file mode 100644 index 0000000..f5b8384 --- /dev/null +++ b/tests/cputestdata/ppc64-guest-legacy-incompatible.xml @@ -0,0 +1,3 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>POWER8_v1.0</model> +</cpu> diff --git a/tests/cputestdata/ppc64-guest-legacy-invalid.xml b/tests/cputestdata/ppc64-guest-legacy-invalid.xml new file mode 100644 index 0000000..be059c3 --- /dev/null +++ b/tests/cputestdata/ppc64-guest-legacy-invalid.xml @@ -0,0 +1,3 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>POWER8E_v1.0</model> +</cpu> diff --git a/tests/cputestdata/ppc64-guest-legacy.xml b/tests/cputestdata/ppc64-guest-legacy.xml new file mode 100644 index 0000000..36bae52 --- /dev/null +++ b/tests/cputestdata/ppc64-guest-legacy.xml @@ -0,0 +1,3 @@ +<cpu mode='custom' match='exact'> + <model fallback='allow'>POWER7_v2.3</model> +</cpu> diff --git a/tests/cputestdata/ppc64-host+guest-legacy,ppc_models-result.xml b/tests/cputestdata/ppc64-host+guest-legacy,ppc_models-result.xml new file mode 100644 index 0000000..3548c0e --- /dev/null +++ b/tests/cputestdata/ppc64-host+guest-legacy,ppc_models-result.xml @@ -0,0 +1,5 @@ +<cpu mode='custom' match='exact'> + <arch>ppc64</arch> + <model fallback='allow'>POWER7</model> + <vendor>IBM</vendor> +</cpu> diff --git a/tests/cputestdata/ppc64-host-better.xml b/tests/cputestdata/ppc64-host-better.xml new file mode 100644 index 0000000..af87412 --- /dev/null +++ b/tests/cputestdata/ppc64-host-better.xml @@ -0,0 +1,6 @@ +<cpu> + <arch>ppc64</arch> + <model>POWER8</model> + <vendor>IBM</vendor> + <topology sockets='1' cores='64' threads='1'/> +</cpu> diff --git a/tests/cputestdata/ppc64-host-incomp-arch.xml b/tests/cputestdata/ppc64-host-incomp-arch.xml new file mode 100644 index 0000000..195f436 --- /dev/null +++ b/tests/cputestdata/ppc64-host-incomp-arch.xml @@ -0,0 +1,6 @@ +<cpu> + <arch>x86_64</arch> + <model>POWER7</model> + <vendor>IBM</vendor> + <topology sockets='1' cores='64' threads='1'/> +</cpu> diff --git a/tests/cputestdata/ppc64-host-no-vendor.xml b/tests/cputestdata/ppc64-host-no-vendor.xml new file mode 100644 index 0000000..de73006 --- /dev/null +++ b/tests/cputestdata/ppc64-host-no-vendor.xml @@ -0,0 +1,5 @@ +<cpu> + <arch>ppc64</arch> + <model>POWER7</model> + <topology sockets='1' cores='64' threads='1'/> +</cpu> diff --git a/tests/cputestdata/ppc64-host-worse.xml b/tests/cputestdata/ppc64-host-worse.xml new file mode 100644 index 0000000..ba1806b --- /dev/null +++ b/tests/cputestdata/ppc64-host-worse.xml @@ -0,0 +1,6 @@ +<cpu> + <arch>ppc64</arch> + <model>POWER6</model> + <vendor>IBM</vendor> + <topology sockets='1' cores='64' threads='1'/> +</cpu> -- 2.4.3