[libvirt] [PATCH 1/2] lib: Introduce API for retrieving bulk domain stats

[Li Wei]

On 08/26/2014 03:43 PM, Peter Krempa wrote:
> On 08/26/14 03:45, Li Wei wrote:
>>
>>
>> On 08/26/2014 01:05 AM, Peter Krempa wrote:
>>> The motivation for this API is that management layers that use libvirt
>>> usually poll for statistics using various split up APIs we currently
>>> provide. To get all the necessary stuff, the app needs to issue a lot of
>>> calls and agregate the results.
>>>
>>> The APIs I'm introducing here:
>>> 1) Returns data in a format that we can expand in the future and is
>>> (pseudo) hierarchical. The data is returned as typed parameters where
>>> the fields are constructed as dot-separated strings containing names and
>>> other stuff in a list of typed params.
>>>
>>> 2) Stats for multiple (all) domains can be queried at once and are
>>> returned in one call. This will allow to decrease the overhead necessary
>>> to issue multiple calls per domain multiplied by the count of domains.
>>>
>>> 3) Selectable (bit mask) fields in the returned format. This will allow
>>> to retrieve only specific stats according to the apps need.
>>>
>>> The stats groups will be enabled using a bit field @stats passed as the
>>> function argument. A few sample stats groups that this API will support:
>>>
>>> VIR_DOMAIN_STATS_STATE
>>> VIR_DOMAIN_STATS_CPU
>>> VIR_DOMAIN_STATS_BLOCK
>>> VIR_DOMAIN_STATS_INTERFACE
>>>
>>> the returned typed params will use the following scheme
>>>
>>> state.state = running
>>> state.reason = started
>>> cpu.count = 8
>>> cpu.0.state = running
>>> cpu.0.time = 1234
>>> ---
>>>  include/libvirt/libvirt.h.in |  26 +++++++
>>>  src/driver.h                 |   9 +++
>>>  src/libvirt.c                | 179 +++++++++++++++++++++++++++++++++++++++++++
>>>  src/libvirt_public.syms      |   7 ++
>>>  4 files changed, 221 insertions(+)
>>>
>>> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
>>> index 47ea695..962f740 100644
>>> --- a/include/libvirt/libvirt.h.in
>>> +++ b/include/libvirt/libvirt.h.in
>>> @@ -2501,6 +2501,32 @@ int virDomainDetachDeviceFlags(virDomainPtr domain,
>>>  int virDomainUpdateDeviceFlags(virDomainPtr domain,
>>>                                 const char *xml, unsigned int flags);
>>>
>>> +typedef struct _virDomainStatsRecord virDomainStatsRecord;
>>> +typedef virDomainStatsRecord *virDomainStatsRecordPtr;
>>> +struct _virDomainStatsRecord {
>>> +    virDomainPtr dom;
>>> +    unsigned int nparams;
>>> +    virTypedParameterPtr params;
>>> +};
>>> +
>>> +typedef enum {
>>> +    VIR_DOMAIN_STATS_ALL = (1 << 0), /* return all stats fields
>>> +                                       implemented in the daemon */
>>
>> Why not define VIR_DOMAIN_STATS_ALL as the bitwise or of each other
>> individual VIR_DOMAIN_STATS_XXX so we no need to make a special path
>> for VIR_DOMAIN_STATS_ALL in the implementation?
> 
> This will allow us to separately return everything the daemon supports
> and still allow the caller to be notified if one of the requested stats
> fields isn't supported.

Don't understand this very well, did you mean with
VIR_DOMAIN_STATS_ALL | VIR_DOMAIN_STATS_BLOCK
libvirt will return an error if VIR_DOMAIN_STATS_BLOCK not supported?
not sure if this scenario is useful.

Thanks

> 
> Peter
> 
>