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