Re: [libvirt] [PATCH] rpc: allow truncated return for virDomainGetCPUStats
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)