Re: [libvirt] [PATCH 07/23] cpu: Return model from virCPUModelIsAllowed
On 10/04/2017 10:58 AM, Jiri Denemark wrote:
If a given CPU model is supported by the hypervisor, we want to know
more about it, e.g., what features may block its usage on the current
host and such details are stored in the virDomainCapsCPUModelsPtr list
which virCPUModelIsAllowed uses to check whether the CPU model is
supported. Thus if the CPU model is found in the list we can directly
return a pointer to the corresponding virDomainCapsCPUModel if the
caller needs to look at the details.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu.c | 18 ++++++++++++++----
src/cpu/cpu.h | 3 ++-
src/cpu/cpu_ppc64.c | 2 +-
src/cpu/cpu_x86.c | 2 +-
4 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index b815ed383a..48290a471b 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -819,24 +819,34 @@ virCPUDataParse(const char *xmlStr)
*
* @model: CPU model to be checked
* @models: list of supported CPU models
+ * @hvModel: pointer to matching model from @models will be returned here
As later pointed out - 'hv' == hypervisor version.
In any case, the 'ModelIsAllowed" now feels overloaded returning more
than just true/false that a typical "Is" type function would return.
Sorry, I don't have suggestions, so unless someone else is looking and
has a better name, then just go with it.
The only other concern is that since you're returning a pointer in the
middle of some array, any concerns over some other thread changing
things and freeing what you're looking at?
e.g., should there be a way to deep copy the model information and force
the caller to free when it's done?
As long as you're comfortable with pointing in the middle of an array
(and I didn't see anything existing that would seem to cause a problem),
then fine go with what you have.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
*
* Checks whether @model can be found in the list of supported @models.
- * If @models is NULL, all models are supported.
+ * If @models is NULL, all models are supported. If both @models and @hvModel
+ * are non-NULL and @model is found in the list of supported models, @hvModel
+ * will be filled with the pointer to the matching CPU model from @models.
*
* Returns true if @model is supported, false otherwise.
*/
bool
virCPUModelIsAllowed(const char *model,
- virDomainCapsCPUModelsPtr models)
+ virDomainCapsCPUModelsPtr models,
+ virDomainCapsCPUModelPtr *hvModel)
{
size_t i;
+ if (hvModel)
+ *hvModel = NULL;
+
if (!models)
return true;
for (i = 0; i < models->nmodels; i++) {
- if (STREQ(models->models[i].name, model))
+ if (STREQ(models->models[i].name, model)) {
+ if (hvModel)
+ *hvModel = models->models + i;
return true;
+ }
}
return false;
}
@@ -908,7 +918,7 @@ virCPUTranslate(virArch arch,
cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH)
return 0;
- if (virCPUModelIsAllowed(cpu->model, models))
+ if (virCPUModelIsAllowed(cpu->model, models, NULL))
return 0;
if (cpu->fallback != VIR_CPU_FALLBACK_ALLOW) {
[...]