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

Re: [libvirt] [PATCHv2 1/2] Add a new flag VIR_DOMAIN_CPU_STATS_VCPU to virDomainGetCPUStats



On 05/09/2012 02:38 AM, Hu Tao wrote:
>> Question: is it better to make the user call virDomainGetCPUStats twice
>> (once for total with flags==0, once for vcpu with flags=VCPU), where
>> there is a race between the two calls?  Or should we instead return both
> 
> As Richard said, we already have had many two-stage-call APIs: one for
> the size, another for the result. Do we also face the same race there?

The two-stage-call API is an optimization - so that you can minimally
size your array.  You can always oversize your API to begin with, and
libvirt should make it obvious (from return value, NULL entries at the
end of the array, or some other means) if it did not fill in all the
oversized entries you passed in.  If you go with this approach
(oversized arrays), then you need only call the API once.  With
oversized arrays, you risk running out of memory or exceeding RPC limits
if you size too big, as well as extra overhead in determining what you
got back.

Also, if you are ever worried about a race (what happens if some other
thread manages to change things so that between the time you get the
size hint, then use that size), the solution is to always oversize by 1
more than the hint, then verify after you use your new size that you
were indeed oversized.  That is, if the hint says you should size with 2
elements, then you pass in 3 elements, you will either see 2 or fewer
elements (no race occurred) or 3 elements (the race occurred, someone
else was able to inject a new element between you grabbing the hint and
actually using it, so try again with 4 elements).

Some of our interfaces are not very race-prone - for example, the number
of statistics returned by virDomainGetCPUStats is hard-coded in the
driver returning stats (you have to recompile libvirtd to change from
returning 1 stat into returning 2 stats); while others really are
run-time variables (the number of online domains for
virConnectListDomains can easily change between the hint and usage).  We
probably should do a better job of documenting when a hint will be
unreliable and the caller should use the oversize technique to ensure
that all values are returned.

Also, I really want to add a new API that returns a list of virDomainPtr
rather than ids or names, with a proper flags argument (so that you
_don't_ have the race currently mandated by our poor design of
virConnectListDomains/virConnectListDefinedDomains).  When we add that
API, we can do it properly (have the return list allocated by libvirt
rather than pre-allocated by the caller).

> Anyway, this is another story.

Agreed.

> 
>> counts in the same call, by having two separate virTypedParameter stat
>> names and no flag?
>>
>> In other words, I'm thinking it might be better to keep "cpu_time"
>> (VIR_DOMAIN_CPU_STATS_CPUTIME) as the first stat, and add a second stat
>> "vcpu_time" (VIR_DOMAIN_CPU_STATS_VCPUTIME) as a second stat, all within
>> the same call, with no need to add a flag.
> 
> But I like this way because it seems more natural. See v4.

At first glance, I already like v4 better, but I think it's worth
waiting until after the 0.9.12 release to actually include it.  I'll
review it soon.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
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]