On 03/07/2012 02:46 PM, Osier Yang wrote:
On 03/07/2012 12:48 PM, Eric Blake wrote:The RPC code assumed that the array returned by the driver would be fully populated; that is, ncpus on entry resulted in ncpus * return value on exit. However, while we don't support holes in the middle of ncpus, we do want to permit the case of ncpus on entry being longer than the array returned by the driver (that is, it should be safe for the caller to pass ncpus=128 on entry, and the driver will stop populating the array when it hits max_id). There are now three cases: server 0.9.10 and client 0.9.10 or newer: No impact - there were no hypervisor drivers that supported cpu stats server 0.9.11 or newer and client 0.9.10: if the client calls with ncpus beyond the max, then the rpc call will fail on the client side and disconnect the client, but the server is no worse for the wear server 0.9.11 or newer and client 0.9.11: the server can return a truncated array and the client will do just fine I reproduced the problem by using a host with 2 CPUs, and doing: virsh cpu-stats $dom --start 1 --count 2 * daemon/remote.c (remoteDispatchDomainGetCPUStats): Allow driver to omit tail of array. * src/remote/remote_driver.c (remoteDomainGetCPUStats): Accommodate driver that omits tail of array. --- daemon/remote.c | 10 ++++++++-- src/remote/remote_driver.c | 6 ++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 74a5f16..39302cc 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -3574,11 +3574,17 @@ remoteDispatchDomainGetCPUStats(virNetServerPtr server ATTRIBUTE_UNUSED, args->flags)< 0) goto cleanup; - percpu_len = ret->params.params_len / args->ncpus; - success: rv = 0; ret->nparams = percpu_len; + if (args->nparams&& !(args->flags& VIR_TYPED_PARAM_STRING_OKAY)) { + int i; + + for (i = 0; i< percpu_len; i++) { + if (params[i].type == VIR_TYPED_PARAM_STRING) + ret->nparams--; + } + } cleanup: if (rv< 0) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 9e74cea..031167a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2382,7 +2382,7 @@ static int remoteDomainGetCPUStats(virDomainPtr domain, /* Check the length of the returned list carefully. */ if (ret.params.params_len> nparams * ncpus || (ret.params.params_len&& - ret.nparams * ncpus != ret.params.params_len)) { + ((ret.params.params_len % ret.nparams) || ret.nparams> nparams))) { remoteError(VIR_ERR_RPC, "%s", _("remoteDomainGetCPUStats: " "returned number of stats exceeds limit")); @@ -2399,9 +2399,11 @@ static int remoteDomainGetCPUStats(virDomainPtr domain, } /* The remote side did not send back any zero entries, so we have - * to expand things back into a possibly sparse array. + * to expand things back into a possibly sparse array, where the + * tail of the array may be omitted. */ memset(params, 0, sizeof(*params) * nparams * ncpus); + ncpus = ret.params.params_len / ret.nparams; for (cpu = 0; cpu< ncpus; cpu++) { int tmp = nparams; remote_typed_param *stride =&ret.params.params_val[cpu * ret.nparams];Make sense, and ACK. But do we want to add document to declare the returned array will be truncated among the API implementation. Not neccessary though.
Perhaps something like: * whole). Otherwise, @start_cpu represents which cpu to start * with, and @ncpus represents how many consecutive processors to * query, with statistics attributable per processor (such as - * per-cpu usage). + * per-cpu usage). If @ncpus is larger than the number of host + * CPUs, the exceeded one(s) will be just ignored.
Osier -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list