[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/5] vcpupin: introduce the new libvirt API (virDomainGetVcpupinInfo)



On 06/24/2011 09:12 AM, Eric Blake wrote:
> On 06/24/2011 08:53 AM, Daniel Veillard wrote:
>>>  
>>> +int                     virDomainGetVcpupinInfo (virDomainPtr domain,
>>> +                                                 int maxinfo,
>>> +                                                 unsigned char *cpumaps,
>>> +                                                 int maplen,
>>> +                                                 unsigned int flags);
>>> +
>>
>>  API looks fine to me
> 
> I'm not quite sure, yet.
> 
>>

>>> +virDomainGetVcpupinInfo (virDomainPtr domain, int maxinfo,
>>> +                         unsigned char *cpumaps, int maplen, unsigned int flags)
> 
> 
> What is maxinfo for?  The counterpart is:
> 
> virDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo,
>                   unsigned char *cpumaps, int maplen)
> 
> where maxinfo relates to how large the 'info' array is.  But
> virDomainGetVcpupinInfo has no 'info' array.

On further thought, I guess the issue is that:

cpumaps is an array (maxinfo entries), and each entry of that array is a
cpumap (maplen bytes).  This is the same for virDomainGetVcpus, but that
API also had an additional array of virVcpuInfoPtr, where you could then
ignore cpumas and maplen.

Our normal protocol is to do array/len, which works for info/maxinfo,
but doesn't fit for maxinfo/cpumaps/maplen.

On the other hand, since we can't change the signature of
virDomainGetVcpus, it probably really is better to keep
virDomainGetVcpupinInfo consistent.  My only suggestion, therefore, is
to s/maxinfo/ncpumaps/ to make it obvious that we really are passing
array length prior to the array.  What a yucky interface we've locked
ourselves into, but I can't see any better approach.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]