On Fri, Oct 08, 2010 at 05:45:00PM +0530, Nikunj A. Dadhania wrote:
From: Nikunj A. Dadhania <nikunj@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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/