[libvirt] [PATCH 1/2] docs: improve typed parameter documentation
Stefan Berger
stefanb at linux.vnet.ibm.com
Tue Nov 1 10:29:25 UTC 2011
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.
More information about the libvir-list
mailing list