[libvirt] [PATCH v4 02/13] Adding virDomainSetMemoryParameters and virDomainGetMemoryParameters API
Daniel Veillard
veillard at redhat.com
Tue Oct 12 14:01:53 UTC 2010
On Fri, Oct 08, 2010 at 05:45:00PM +0530, Nikunj A. Dadhania wrote:
> From: Nikunj A. Dadhania <nikunj at linux.vnet.ibm.com>
>
> Public api to set/get memory tunables supported by the hypervisors.
> RFC: https://www.redhat.com/archives/libvir-list/2010-August/msg00607.html
>
> v4:
> * Move exporting public API to this patch
> * Add unsigned int flags to the public api for future extensions
>
> v3:
> * Add domainGetMemoryParamters and NULL in all the driver interface
>
> v2:
> * Initialize domainSetMemoryParameters to NULL in all the driver interface
> structure.
>
[...]
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -3000,6 +3000,110 @@ error:
> }
>
> /**
> + * virDomainSetMemoryParameters:
> + * @domain: pointer to domain object
> + * @params: pointer to memory parameter objects
> + * @nparams: number of memory parameter (this value should be same or
> + * less than the number of parameters supported)
> + * @flags: currently unused, for future extension
> + *
> + * Change the memory tunables
> + * This function requires privileged access to the hypervisor.
> + *
> + * Returns -1 in case of error, 0 in case of success.
> + */
> +int
> +virDomainSetMemoryParameters(virDomainPtr domain,
> + virMemoryParameterPtr params,
> + int nparams, unsigned int flags)
I had to remove tabs and trailing spaces at end of lines here.
> +{
> + virConnectPtr conn;
> + DEBUG("domain=%p, params=%p, nparams=%d, flags=%u", domain, params, nparams, flags);
> +
> + virResetLastError();
> +
> + if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virDispatchError(NULL);
> + return -1;
> + }
> + if (domain->conn->flags & VIR_CONNECT_RO) {
> + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> + goto error;
> + }
Seems that params <= 0 or a NULL params are errors in this function
based on the API description, so I prefer to catch those here and added
if ((nparams <= 0) || (params == NULL)) {
virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__);
goto error;
}
> + conn = domain->conn;
> +
> + if (conn->driver->domainSetMemoryParameters) {
[...]
> +/**
> + * virDomainGetMemoryParameters:
> + * @domain: pointer to domain object
> + * @params: pointer to memory parameter object
> + * (return value, allocated by the caller)
> + * @nparams: pointer to number of memory parameters
> + * @flags: currently unused, for future extension
> + *
> + * Get the memory parameters, the @params array will be filled with the values
> + * equal to the number of parameters suggested by @nparams
> + *
> + * As a special case, if @nparams is zero and @params is NULL, the API will
> + * set the number of parameters supported by the HV in @nparams and return
> + * SUCCESS.
> + *
> + * This function requires privileged access to the hypervisor. This function
> + * expects the caller to allocate the
unterminated comment. I fixed this as
* expects the caller to allocate the @param
> + * Returns -1 in case of error, 0 in case of success.
> + */
> +int
> +virDomainGetMemoryParameters(virDomainPtr domain,
> + virMemoryParameterPtr params,
> + int *nparams, unsigned int flags)
> +{
same tabs and trailing spaces problems
> + virConnectPtr conn;
> + DEBUG("domain=%p, params=%p, nparams=%d, flags=%u", domain, params, (nparams)?*nparams:-1, flags);
> +
> + virResetLastError();
> +
> + if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virDispatchError(NULL);
> + return -1;
> + }
> + if (domain->conn->flags & VIR_CONNECT_RO) {
> + virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> + goto error;
> + }
in that case params can be NULL, but nparams must not, and we can't
have *nparams < 0 strictly so I'm adding:
if ((nparams == NULL) || (*nparams < 0)) {
virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__);
goto error;
}
> + conn = domain->conn;
That done, patch is fine,
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel at veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
More information about the libvir-list
mailing list