Re: [libvirt] [PATCH 1/8] cpu: Introduce cpuModelIsAllowed internal API
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
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)