On 12/20/2012 05:01 PM, Jiri Denemark wrote:
The API can be used to check if the model is on the supported models list, which needs to be done in several places. --- src/cpu/cpu.c | 17 +++++++++++++++++ src/cpu/cpu.h | 5 +++++ src/cpu/cpu_generic.c | 19 +++++-------------- src/cpu/cpu_x86.c | 11 +---------- 4 files changed, 28 insertions(+), 24 deletions(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 53c4cc3..b41fb38 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -442,3 +442,20 @@ cpuHasFeature(virArch arch,
return driver->hasFeature(data, feature); } + +bool +cpuModelIsAllowed(const char *model, + const char **models, + unsigned int nmodels)
Is size_t any better than unsigned int for nmodels?
+{ + unsigned int i; + + if (!models || !nmodels) + return true;
Should this case be false? Or more specifically, in the old code...
+ + for (i = 0; i < nmodels; i++) { + if (models[i] && STREQ(models[i], model)) + return true; + } + return false; +}
+++ b/src/cpu/cpu_generic.c @@ -123,20 +123,11 @@ genericBaseline(virCPUDefPtr *cpus, unsigned int count; unsigned int i, j;
- if (models) {
!models skipped the error message, but allocated models with nmodels==0 errored out. You have a silent change in behavior by not erroring where you used to; meanwhile, if you return false instead of true for both branches of the ||, you would have a change in behavior of erroring where you previously did not. I think that our current mix of half-and-half erroring is not useful, but it's probably worth deciding what we meant, and documenting in the commit message that the change in error policy for (!models || !nmodels) is intentional.
- bool found = false; - for (i = 0; i < nmodels; i++) { - if (STREQ(cpus[0]->model, models[i])) { - found = true; - break; - } - } - if (!found) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("CPU model '%s' is not support by hypervisor"), - cpus[0]->model); - goto error; - } + if (!cpuModelIsAllowed(cpus[0]->model, models, nmodels)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("CPU model %s is not supported by hypervisor"), + cpus[0]->model); + goto error; }
+++ b/src/cpu/cpu_x86.c @@ -1325,16 +1325,7 @@ x86Decode(virCPUDefPtr cpu,
candidate = map->models; while (candidate != NULL) { - bool allowed = (models == NULL); - - for (i = 0; i < nmodels; i++) { - if (models && models[i] && STREQ(models[i], candidate->name)) { - allowed = true; - break; - } - }
Hmm, here the behavior was different for (!models || !nmodels).
- - if (!allowed) { + if (!cpuModelIsAllowed(candidate->name, models, nmodels)) { if (preferred && STREQ(candidate->name, preferred)) { if (cpu->fallback != VIR_CPU_FALLBACK_ALLOW) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org