[libvirt] [PATCH 07/23] cpu: Return model from virCPUModelIsAllowed
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.
>> 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.
More information about the libvir-list