On 10/31/2011 07:33 PM, Eric Blake wrote:
virDomainBlockStatsFlags was missing a check that was present in
virDomainGetMemoryParameters. Additionally, I found that the
existing descriptions were a bit hard to read. A later patch
will fix qemu to return fewer than max parameters if @nparams
was too small on input, rather than outright fail.
* src/libvirt.c (virDomainGetMemoryParameters)
(virDomainGetBlkioParameters, virDomainGetSchedulerParameters)
(virDomainGetSchedulerParametersFlags):
Tweak documentation wording.
(virDomainBlockStatsFlags): Likewise, and add sanity check.
---
src/libvirt.c | 100 +++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 65 insertions(+), 35 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index e9d1a29..f9cddef 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -3639,15 +3639,17 @@ error:
* @domain: pointer to domain object
* @params: pointer to memory parameter object
* (return value, allocated by the caller)
- * @nparams: pointer to number of memory parameters
+ * @nparams: pointer to number of memory parameters; input and output
* @flags: one of virDomainModificationImpact
*
- * Get all memory parameters, the @params array will be filled with the values
- * equal to the number of parameters suggested by @nparams
+ * Get all memory parameters. On input, @nparams gives the size of the
+ * @params array; on output, @nparams gives how many slots were filled
+ * with parameter information, which might be less but will not exceed
+ * the input value.
*
- * As the value of @nparams is dynamic, call the API setting @nparams to 0 and
- * @params as NULL, the API returns the number of parameters supported by the
- * HV by updating @nparams on SUCCESS. The caller should then allocate @params
+ * As a special case, calling with @params as NULL and @nparams as 0 on
+ * input will cause @nparams on output to contain the number of parameters
+ * supported by the hypervisor. The caller should then allocate @params
* array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API
* again.
*
@@ -3765,12 +3767,21 @@ error:
* @domain: pointer to domain object
* @params: pointer to blkio parameter object
* (return value, allocated by the caller)
- * @nparams: pointer to number of blkio parameters
+ * @nparams: pointer to number of blkio parameters; input and output
* @flags: an OR'ed set of virDomainModificationImpact
*
- * Get all blkio parameters, the @params array will be filled with the values
- * equal to the number of parameters suggested by @nparams.
- * See virDomainGetMemoryParameters for an equivalent usage example.
+ * Get all blkio parameters. On input, @nparams gives the size of the
+ * @params array; on output, @nparams gives how many slots were filled
+ * with parameter information, which might be less but will not exceed
+ * the input value.
+ *
+ * As a special case, calling with @params as NULL and @nparams as 0 on
+ * input will cause @nparams on output to contain the number of parameters
+ * supported by the hypervisor. The caller should then allocate @params
+ * array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API
+ * again.
+ *
+ * See virDomainGetMemoryParameters() for an equivalent usage example.
*
* This function may require privileged access to the hypervisor. This function
* expects the caller to allocate the @params.
@@ -6335,14 +6346,17 @@ error:
* @params: pointer to scheduler parameter objects
* (return value)
* @nparams: pointer to number of scheduler parameter objects
- * (this value must be at least as large as the returned value
- * nparams of virDomainGetSchedulerType)
+ * (this value should generally be as large as the returned value
+ * nparams of virDomainGetSchedulerType()); input and output
+ *
+ * Get all scheduler parameters. On input, @nparams gives the size of the
+ * @params array; on output, @nparams gives how many slots were filled
+ * with parameter information, which might be less but will not exceed
+ * the input value. @nparams cannot be 0.
*
- * Get all scheduler parameters, the @params array will be filled with the
- * values and @nparams will be updated to the number of valid elements in
- * @params. It is hypervisor specific whether this returns the live or
+ * It is hypervisor specific whether this returns the live or
* persistent state; for more control, use
- * virDomainGetSchedulerParametersFlags.
+ * virDomainGetSchedulerParametersFlags().
*
* Returns -1 in case of error, 0 in case of success.
*/
@@ -6391,15 +6405,28 @@ error:
* (return value)
* @nparams: pointer to number of scheduler parameter
* (this value should be same than the returned value
- * nparams of virDomainGetSchedulerType)
+ * nparams of virDomainGetSchedulerType()); input and output
* @flags: one of virDomainModificationImpact
*
- * Get the scheduler parameters, the @params array will be filled with the
- * values.
+ * Get all scheduler parameters. On input, @nparams gives the size of the
+ * @params array; on output, @nparams gives how many slots were filled
+ * with parameter information, which might be less but will not exceed
+ * the input value. @nparams cannot be 0.
*
* The value of @flags can be exactly VIR_DOMAIN_AFFECT_CURRENT,
* VIR_DOMAIN_AFFECT_LIVE, or VIR_DOMAIN_AFFECT_CONFIG.
*
+ * Here is a sample code snippet:
+ *
+ * char *ret = virDomainGetSchedulerType(dom,&nparams);
+ * if (ret&& nparams != 0) {
+ * if ((params = malloc(sizeof(*params) * nparams)) == NULL)
+ * goto error;
+ * memset(params, 0, sizeof(*params) * nparams);
+ * if (virDomainGetSchedulerParametersFlags(dom, params,&nparams, 0))
+ * goto error;
+ * }
+ *
* Returns -1 in case of error, 0 in case of success.
*/
int
@@ -6633,8 +6660,8 @@ error:
* @path: path to the block device
* @params: pointer to block stats parameter object
* (return value)
- * @nparams: pointer to number of block stats
- * @flags: unused, always passes 0
+ * @nparams: pointer to number of block stats; input and output
+ * @flags: unused, always pass 0
*
* This function is to get block stats parameters for block
* devices attached to the domain.
@@ -6646,24 +6673,26 @@ error:
* Domains may have more than one block device. To get stats for
* each you should make multiple calls to this function.
*
- * The @params array will be filled with the value equal to the number of
- * parameters suggested by @nparams.
+ * On input, @nparams gives the size of the @params array; on output,
+ * @nparams gives how many slots were filled with parameter
+ * information, which might be less but will not exceed the input
+ * value.
*
- * As the value of @nparams is dynamic, call the API setting @nparams to 0 and
- * @params as NULL, the API returns the number of parameters supported by the
- * HV by updating @nparams on SUCCESS. (Note that block device of different type
- * might support different parameters numbers, so it might be necessary to compute
- * @nparams for each block device type). The caller should then allocate @params
+ * As a special case, calling with @params as NULL and @nparams as 0 on
+ * input will cause @nparams on output to contain the number of parameters
+ * supported by the hypervisor. (Note that block devices of different types
+ * might support different parameters, so it might be necessary to compute
+ * @nparams for each block device). The caller should then allocate @params
* array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API
- * again. See virDomainGetMemoryParameters for more details.
+ * again. See virDomainGetMemoryParameters() for more details.
*
* Returns -1 in case of error, 0 in case of success.
*/
-int virDomainBlockStatsFlags (virDomainPtr dom,
- const char *path,
- virTypedParameterPtr params,
- int *nparams,
- unsigned int flags)
+int virDomainBlockStatsFlags(virDomainPtr dom,
+ const char *path,
+ virTypedParameterPtr params,
+ int *nparams,
+ unsigned int flags)
{
virConnectPtr conn;
@@ -6677,7 +6706,8 @@ int virDomainBlockStatsFlags (virDomainPtr dom,
virDispatchError(NULL);
return -1;
}
- if (!path || (nparams == NULL) || (*nparams< 0)) {
+ if (!path || (nparams == NULL) || (*nparams< 0) ||
+ (params == NULL&& *nparams != 0)) {
virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
goto error;
}
Looks good to me. ACK.