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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
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) {
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
