On 03/07/2012 12:18 AM, Osier Yang wrote:
> 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).
>>>
>>> /* 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.
I realized that qemu_driver.c also needs this memset to guarantee that
unused slots are returned empty on success.
>>
>> 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.
Good idea. Here's what I squashed in before pushing:
diff --git i/src/libvirt.c w/src/libvirt.c
index d98741b..39ebb52 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -18534,7 +18534,9 @@ error:
* 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 cpus
+ * available to query, then the trailing part of the array will
+ * be unpopulated.
*
* The remote driver imposes a limit of 128 @ncpus and 16 @nparams;
* the number of parameters per cpu should not exceed 16, but if you
@@ -18579,8 +18581,10 @@ error:
* number of populated @params, unless @ncpus was 1; and may be
* less than @nparams). The populated parameters start at each
* stride of @nparams, which means the results may be discontiguous;
- * any unpopulated parameters will be zeroed on success. The caller
- * is responsible for freeing any returned string parameters.
+ * any unpopulated parameters will be zeroed on success (this includes
+ * skipped elements if @nparams is too large, and tail elements if
+ * @ncpus is too large). The caller is responsible for freeing any
+ * returned string parameters.
*/
int virDomainGetCPUStats(virDomainPtr domain,
virTypedParameterPtr params,
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 538a419..ddb381a 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -12162,6 +12162,7 @@ qemuDomainGetPercpuStats(virDomainPtr domain,
if (virCgroupGetCpuacctPercpuUsage(group, &buf))
goto cleanup;
pos = buf;
+ memset(params, 0, nparams * ncpus);
if (max_id - start_cpu > ncpus - 1)
max_id = start_cpu + ncpus - 1;
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list