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

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 */ + VIR_DOMAIN_STATS_STATE = (1 << 1), /* return domain state */ +} virDomainStatsTypes; + +int virConnectGetAllDomainStats(virConnectPtr conn, + unsigned int stats, + virDomainStatsRecordPtr **retStats, + unsigned int flags); + +int virDomainListGetStats(virDomainPtr *doms, + unsigned int stats, + virDomainStatsRecordPtr **retStats, + unsigned int flags); + +void virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats); + /* * BlockJob API */ diff --git a/src/driver.h b/src/driver.h index ba7c1fc..d5596ab 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1191,6 +1191,14 @@ typedef int unsigned int flags);

+typedef int +(*virDrvDomainListGetStats)(virConnectPtr conn, + virDomainPtr *doms, + unsigned int ndoms, + unsigned int stats, + virDomainStatsRecordPtr **retStats, + unsigned int flags); + typedef struct _virDriver virDriver; typedef virDriver *virDriverPtr;

@@ -1411,6 +1419,7 @@ struct _virDriver { virDrvDomainSetTime domainSetTime; virDrvNodeGetFreePages nodeGetFreePages; virDrvConnectGetDomainCapabilities connectGetDomainCapabilities; + virDrvDomainListGetStats domainListGetStats; };

diff --git a/src/libvirt.c b/src/libvirt.c index 8349261..bbbc023 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21341,3 +21341,182 @@ virConnectGetDomainCapabilities(virConnectPtr conn, virDispatchError(conn); return NULL; } + + +/** + * virConnectGetAllDomainStats: + * @conn: pointer to the hypervisor connection + * @stats: stats to return, binary-OR of virDomainStatsTypes + * @retStats: Pointer that will be filled with the array of returned stats. + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Query statistics for all domains on a given connection. + * + * Report statistics of various parameters for a running VM according to @stats + * field. The statistics are returned as an array of structures for each queried + * domain. The structure contains an array of typed parameters containing the + * individual statistics. The typed parameter name for each statistic field + * consists of a dot-separated string containing name of the requested group + * followed by a group specific description of the statistic value. + * + * The statistic groups are enabled using the @stats parameter which is a + * binary-OR of enum virDomainStatsTypes. The following groups are available + * (although not necessarily implemented for each storage driver): + * + * VIR_DOMAIN_STATS_ALL: Return all statistics supported by the hypervisor + * driver. This allows to query everything the driver supports without getting + * an error. + * + * VIR_DOMAIN_STATS_STATE: Return domain state and reason for entering that + * state. The typed parameter keys are in format: + * "state.state" - state of the VM, returned as int from virDomainState enum + * "state.reason" - reason for entering given state, returned as in from + * virDomain*Reason enmum corresponding to given state. + * + * Returns the count of returned statistics strucutres on success, -1 on error. + * The requested data are returned in the @retStats parameter. The returned + * array should be freed by the caller. See virDomainStatsRecordListFree. + */ +int +virConnectGetAllDomainStats(virConnectPtr conn, + unsigned int stats, + virDomainStatsRecordPtr **retStats, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("conn=%p, stats=0x%x, retStats=%p, flags=0x%x", + conn, stats, retStats, flags); + + virResetLastError(); + + virCheckConnectReturn(conn, -1); + + if (!conn->driver->domainListGetStats) { + virReportUnsupportedError(); + goto cleanup; + } + + ret = conn->driver->domainListGetStats(conn, NULL, 0, stats,

You mean we pass a NULL as doms to retrieve stats for all domains of this conn? how about generate the domain list here to avoid implement it in every individual drivers(although this need (de)serialize domains via rpc)? Thanks, Li Wei

+ retStats, flags); + + cleanup: + if (ret < 0) + virDispatchError(conn); + + return ret; +} + + +/** + * virDomainListGetStats: + * @doms: NULL terminated array of domains + * @stats: stats to return, binary-OR of virDomainStatsTypes + * @retStats: Pointer that will be filled with the array of returned stats. + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Query statistics for domains provided by @doms. Note that all domains in + * @doms must share the same connection. + * + * Report statistics of various parameters for a running VM according to @stats + * field. The statistics are returned as an array of structures for each queried + * domain. The structure contains an array of typed parameters containing the + * individual statistics. The typed parameter name for each statistic field + * consists of a dot-separated string containing name of the requested group + * followed by a group specific description of the statistic value. + * + * The statistic groups are enabled using the @stats parameter which is a + * binary-OR of enum virDomainStatsTypes. The following groups are available + * (although not necessarily implemented for each storage driver): + * + * VIR_DOMAIN_STATS_ALL: Return all statistics supported by the hypervisor + * driver. This allows to query everything the driver supports without getting + * an error. + * + * VIR_DOMAIN_STATS_STATE: Return domain state and reason for entering that + * state. The typed parameter keys are in format: + * "state.state" - state of the VM, returned as int from virDomainState enum + * "state.reason" - reason for entering given state, returned as in from + * virDomain*Reason enmum corresponding to given state. + * + * Returns the count of returned statistics strucutres on success, -1 on error. + * The requested data are returned in the @retStats parameter. The returned + * array should be freed by the caller. See virDomainStatsRecordListFree. + */ +int +virDomainListGetStats(virDomainPtr *doms, + unsigned int stats, + virDomainStatsRecordPtr **retStats, + unsigned int flags) +{ + virConnectPtr conn = NULL; + virDomainPtr *nextdom = doms; + unsigned int ndoms = 0; + int ret = -1; + + VIR_DEBUG("doms=%p, stats=0x%x, retStats=%p, flags=0x%x", + doms, stats, retStats, flags); + + virResetLastError(); + + if (!*doms) { + virReportError(VIR_ERR_INVALID_ARG, + _("doms array in %s must contain at least one domain"), + __FUNCTION__); + goto cleanup; + } + + conn = doms[0]->conn; + virCheckConnectReturn(conn, -1); + + if (!conn->driver->domainListGetStats) { + virReportUnsupportedError(); + goto cleanup; + } + + while (*(++nextdom)) { + virDomainPtr dom = *nextdom; + + virCheckDomainGoto(dom, cleanup); + + if (dom->conn != conn) { + virReportError(VIR_ERR_INVALID_ARG, + _("domains in 'doms' array must belong to a " + "single connection in %s"), __FUNCTION__); + goto cleanup; + } + + ndoms++; + } + + ret = conn->driver->domainListGetStats(conn, doms, ndoms, stats, retStats, + flags); + + cleanup: + if (ret < 0) + virDispatchError(conn); + return ret; +} + + +/** + * virDomainStatsRecordListFree: + * @stats: NULL terminated array of virDomainStatsRecords to free + * + * Convenience function to free a list of domain stats returned by + * virDomainListGetStats and virConnectGetAllDomainStats. + */ +void +virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats) +{ + virDomainStatsRecordPtr *next = stats; + + while (*next) { + virTypedParamsFree((*next)->params, (*next)->nparams); + virDomainFree((*next)->dom); + VIR_FREE((*next)); + next++; + } + + VIR_FREE(stats); +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 9f4016a..3714159 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -670,4 +670,11 @@ LIBVIRT_1.2.7 { virConnectGetDomainCapabilities; } LIBVIRT_1.2.6;

+LIBVIRT_1.2.8 { + global: + virDomainListGetStats; + virConnectGetAllDomainStats; + virDomainStatsRecordListFree; +} LIBVIRT_1.2.7; + # .... define new API here using predicted next version number ....

On 08/26/2014 03:46 PM, Peter Krempa wrote:

On 08/26/14 05:21, 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 */ + VIR_DOMAIN_STATS_STATE = (1 << 1), /* return domain state */ +} virDomainStatsTypes; + +int virConnectGetAllDomainStats(virConnectPtr conn, + unsigned int stats, + virDomainStatsRecordPtr **retStats, + unsigned int flags); + +int virDomainListGetStats(virDomainPtr *doms, + unsigned int stats, + virDomainStatsRecordPtr **retStats, + unsigned int flags); + +void virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats); + /* * BlockJob API */ diff --git a/src/driver.h b/src/driver.h index ba7c1fc..d5596ab 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1191,6 +1191,14 @@ typedef int unsigned int flags);

+typedef int +(*virDrvDomainListGetStats)(virConnectPtr conn, + virDomainPtr *doms, + unsigned int ndoms, + unsigned int stats, + virDomainStatsRecordPtr **retStats, + unsigned int flags); + typedef struct _virDriver virDriver; typedef virDriver *virDriverPtr;

@@ -1411,6 +1419,7 @@ struct _virDriver { virDrvDomainSetTime domainSetTime; virDrvNodeGetFreePages nodeGetFreePages; virDrvConnectGetDomainCapabilities connectGetDomainCapabilities; + virDrvDomainListGetStats domainListGetStats; };

diff --git a/src/libvirt.c b/src/libvirt.c index 8349261..bbbc023 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21341,3 +21341,182 @@ virConnectGetDomainCapabilities(virConnectPtr conn, virDispatchError(conn); return NULL; } + + +/** + * virConnectGetAllDomainStats: + * @conn: pointer to the hypervisor connection + * @stats: stats to return, binary-OR of virDomainStatsTypes + * @retStats: Pointer that will be filled with the array of returned stats. + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Query statistics for all domains on a given connection. + * + * Report statistics of various parameters for a running VM according to @stats + * field. The statistics are returned as an array of structures for each queried + * domain. The structure contains an array of typed parameters containing the + * individual statistics. The typed parameter name for each statistic field + * consists of a dot-separated string containing name of the requested group + * followed by a group specific description of the statistic value. + * + * The statistic groups are enabled using the @stats parameter which is a + * binary-OR of enum virDomainStatsTypes. The following groups are available + * (although not necessarily implemented for each storage driver): + * + * VIR_DOMAIN_STATS_ALL: Return all statistics supported by the hypervisor + * driver. This allows to query everything the driver supports without getting + * an error. + * + * VIR_DOMAIN_STATS_STATE: Return domain state and reason for entering that + * state. The typed parameter keys are in format: + * "state.state" - state of the VM, returned as int from virDomainState enum + * "state.reason" - reason for entering given state, returned as in from + * virDomain*Reason enmum corresponding to given state. + * + * Returns the count of returned statistics strucutres on success, -1 on error. + * The requested data are returned in the @retStats parameter. The returned + * array should be freed by the caller. See virDomainStatsRecordListFree. + */ +int +virConnectGetAllDomainStats(virConnectPtr conn, + unsigned int stats, + virDomainStatsRecordPtr **retStats, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("conn=%p, stats=0x%x, retStats=%p, flags=0x%x", + conn, stats, retStats, flags); + + virResetLastError(); + + virCheckConnectReturn(conn, -1); + + if (!conn->driver->domainListGetStats) { + virReportUnsupportedError(); + goto cleanup; + } + + ret = conn->driver->domainListGetStats(conn, NULL, 0, stats,

You mean we pass a NULL as doms to retrieve stats for all domains of this conn? how about generate the domain list here to avoid implement it in every individual drivers(although this need (de)serialize domains via rpc)?

You'd need to transport the domain list over the RPC two more times if the client would retrieve it. It's more optimal to do it in the driver itself as it stores the domain list already.

Oh, I missed the transport of domain list when client retrieving it, thanks for your explanation. Thanks

Peter

On 08/26/2014 01:05 AM, Peter Krempa wrote:

Implement the remote driver support for shuffling the domain stats around. --- daemon/remote.c | 91 ++++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 88 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 26 ++++++++++++- src/remote_protocol-structs | 23 +++++++++++ 4 files changed, 227 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c index ea16789..34e6950 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6337,6 +6337,97 @@ remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED, }

+static int +remoteDispatchDomainListGetStats(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_list_get_stats_args *args, + remote_domain_list_get_stats_ret *ret) +{ + int rv = -1; + size_t i; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virDomainStatsRecordPtr *retStats = NULL; + int nrecords = 0; + virDomainPtr *doms = NULL; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (args->doms.doms_len) { + if (VIR_ALLOC_N(doms, args->doms.doms_len + 1) < 0) + goto cleanup; + + for (i = 0; i < args->doms.doms_len; i++) { + if (!(doms[i] = get_nonnull_domain(priv->conn, args->doms.doms_val[i]))) + goto cleanup; + } + + if ((nrecords = virDomainListGetStats(doms, + args->stats, + &retStats, + args->flags)) < 0) + goto cleanup; + } else { + if (!(virConnectGetAllDomainStats(priv->conn, + args->stats, + &retStats, + args->flags)) < 0)

if ((nrecords = virConnectGetAllDomainStats(priv->conn, args->stats, &retStats, args->flags)) < 0)

+ goto cleanup; + } + + if (nrecords > REMOTE_DOMAIN_LIST_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of domain stats records is %d, " + "which exceeds max limit: %d"), + nrecords, REMOTE_DOMAIN_LIST_MAX); + goto cleanup; + } + + if (retStats && nrecords) { + ret->retStats.retStats_len = nrecords; + + if (VIR_ALLOC_N(ret->retStats.retStats_val, nrecords) < 0) + goto cleanup; + + for (i = 0; i < nrecords; i++) { + virDomainStatsRecordPtr src = retStats[i]; + remote_domain_stats_record *dst = ret->retStats.retStats_val + i; + + make_nonnull_domain(&dst->dom, src->dom); + + if (remoteSerializeTypedParameters(src->params, src->nparams, + &dst->params.params_val, + &dst->params.params_len, + VIR_TYPED_PARAM_STRING_OKAY) < 0) + goto cleanup; + } + } else { + ret->retStats.retStats_len = 0; + ret->retStats.retStats_val = NULL; + } + + ret->ret = nrecords; + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (retStats) + virDomainStatsRecordListFree(retStats); + if (args->doms.doms_len) { + for (i = 0; i < args->doms.doms_len; i++) + virDomainFree(doms[i]); + + VIR_FREE(doms); + } + return rv; +} + + /*----- Helpers. -----*/

/* get_nonnull_domain and get_nonnull_network turn an on-wire diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 9a1d78f..41c807a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7670,6 +7670,93 @@ remoteNetworkGetDHCPLeases(virNetworkPtr net, }

+static int +remoteDomainListGetStats(virConnectPtr conn, + virDomainPtr *doms, + unsigned int ndoms, + unsigned int stats, + virDomainStatsRecordPtr **retStats, + unsigned int flags) +{ + struct private_data *priv = conn->networkPrivateData; + int rv = -1; + size_t i; + remote_domain_list_get_stats_args args; + remote_domain_list_get_stats_ret ret; + + virDomainStatsRecordPtr *tmpret = NULL; + + if (ndoms) { + if (VIR_ALLOC_N(args.doms.doms_val, ndoms) < 0) + goto cleanup; + + for (i = 0; i < ndoms; i++) + make_nonnull_domain(args.doms.doms_val + i, doms[i]); + } + args.doms.doms_len = ndoms; + + args.stats = stats; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + + remoteDriverLock(priv); + if (call(conn, priv, 0, REMOTE_PROC_DOMAIN_LIST_GET_STATS, + (xdrproc_t)xdr_remote_domain_list_get_stats_args, (char *)&args, + (xdrproc_t)xdr_remote_domain_list_get_stats_ret, (char *)&ret) == -1) { + remoteDriverUnlock(priv); + goto cleanup; + } + remoteDriverUnlock(priv); + + if (ret.retStats.retStats_len > REMOTE_DOMAIN_LIST_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of stats entries is %d, which exceeds max limit: %d"), + ret.retStats.retStats_len, REMOTE_DOMAIN_LIST_MAX); + goto cleanup; + } + + *retStats = NULL; + + if (ret.retStats.retStats_len) { + if (VIR_ALLOC_N(tmpret, ret.retStats.retStats_len) < 0)

Shouldn't we need to allocate one plus pointer here to act as a NULL sentinel for virDomainStatsRecordListFree()? Thanks, Li Wei

+ goto cleanup; + + for (i = 0; i < ret.retStats.retStats_len; i++) { + virDomainStatsRecordPtr elem; + remote_domain_stats_record *rec = ret.retStats.retStats_val + i; + + if (VIR_ALLOC(elem) < 0) + goto cleanup; + + if (!(elem->dom = get_nonnull_domain(conn, rec->dom))) + goto cleanup; + + if (remoteDeserializeTypedParameters(rec->params.params_val, + rec->params.params_len, + REMOTE_PROC_DOMAIN_LIST_GET_STATS, + &elem->params, + &elem->nparams)) + goto cleanup; + + tmpret[i] = elem; + } + + *retStats = tmpret; + tmpret = NULL; + } + + rv = ret.ret; + + cleanup: + if (tmpret) + virDomainStatsRecordListFree(tmpret); + xdr_free((xdrproc_t)xdr_remote_domain_list_get_stats_ret, + (char *) &ret); + + return rv; +} + /* get_nonnull_domain and get_nonnull_network turn an on-wire * (name, uuid) pair into virDomainPtr or virNetworkPtr object. * These can return NULL if underlying memory allocations fail, @@ -8008,6 +8095,7 @@ static virDriver remote_driver = { .domainSetTime = remoteDomainSetTime, /* 1.2.5 */ .nodeGetFreePages = remoteNodeGetFreePages, /* 1.2.6 */ .connectGetDomainCapabilities = remoteConnectGetDomainCapabilities, /* 1.2.7 */ + .domainListGetStats = remoteDomainListGetStats, /* 1.2.8 */ };

static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 5c316fb..9729392 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -241,6 +241,9 @@ const REMOTE_DOMAIN_FSFREEZE_MOUNTPOINTS_MAX = 256; /* Upper limit on the maximum number of leases in one lease file */ const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536;

+/* Upper limit on count of parameters returned via bulk stats API */ +const REMOTE_DOMAIN_STATS_PARAMS_MAX = 2048; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN];

@@ -3057,6 +3060,21 @@ struct remote_network_get_dhcp_leases_ret { unsigned int ret; };

+struct remote_domain_stats_record { + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_DOMAIN_STATS_PARAMS_MAX>; +}; + +struct remote_domain_list_get_stats_args { + remote_nonnull_domain doms<REMOTE_DOMAIN_LIST_MAX>; + unsigned int stats; + unsigned int flags; +}; + +struct remote_domain_list_get_stats_ret { + remote_domain_stats_record retStats<REMOTE_DOMAIN_LIST_MAX>; + int ret; +}; /*----- Protocol. -----*/

/* Define the program number, protocol version and procedure numbers here. */ @@ -5420,5 +5438,11 @@ enum remote_procedure { * @generate: both * @acl: connect:write */ - REMOTE_PROC_CONNECT_GET_DOMAIN_CAPABILITIES = 342 + REMOTE_PROC_CONNECT_GET_DOMAIN_CAPABILITIES = 342, + + /** + * @generate: none + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_LIST_GET_STATS = 343 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 9bf09b8..3d9f5e7 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2519,6 +2519,28 @@ struct remote_network_get_dhcp_leases_ret { } leases; u_int ret; }; +struct remote_domain_stats_record { + remote_nonnull_domain dom; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; +}; +struct remote_domain_list_get_stats_args { + struct { + u_int doms_len; + remote_nonnull_domain * doms_val; + } doms; + u_int stats; + u_int flags; +}; +struct remote_domain_list_get_stats_ret { + struct { + u_int retStats_len; + remote_domain_stats_record * retStats_val; + } retStats; + int ret; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -2862,4 +2884,5 @@ enum remote_procedure { REMOTE_PROC_NODE_GET_FREE_PAGES = 340, REMOTE_PROC_NETWORK_GET_DHCP_LEASES = 341, REMOTE_PROC_CONNECT_GET_DOMAIN_CAPABILITIES = 342, + REMOTE_PROC_DOMAIN_LIST_GET_STATS = 343, };