[libvirt] [PATCH 07/23] cpu: Return model from virCPUModelIsAllowed

John Ferlan jferlan at redhat.com
Fri Oct 13 18:45:05 UTC 2017



On 10/13/2017 11:31 AM, Jiri Denemark wrote:
> On Thu, Oct 12, 2017 at 15:50:29 -0400, John Ferlan wrote:
>>
>>
>> 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 at 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.
> 
> Hmm, I have a suggestion. I should just create a separate function
> virDomainCapsCPUModelsGet or *Find which would just return a pointer to
> the CPU model structure from the models array and change
> virCPUModelIsAllowed into a simple wrapper around the new function. The
> callers which would pass non-NULL hvModel pointer would just call the
> underlying virDomainCapsCPUModelsGet function directly.
> 

That would seem to work and Get would perhaps also imply it's not a copy
but rather a pointer to something existing.

John

>> 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?
> 
> No. If there were any concerns like this even going through the array
> would be dangerous. Anyway, virDomainCapsCPUModelsPtr is a reference
> counted object which never changes once it's created. It can only go
> away and be replaced with another one.
> 
> Jirka
> 




More information about the libvir-list mailing list