Re: [libvirt] [PATCH v4 02/13] Adding virDomainSetMemoryParameters and virDomainGetMemoryParameters API
On Fri, Oct 08, 2010 at 05:45:00PM +0530, Nikunj A. Dadhania wrote:
From: Nikunj A. Dadhania <nikunj(a)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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/