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

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.

Peter