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 nik...@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/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 02/13] Adding virDomainSetMemoryParameters and virDomainGetMemoryParameters API
On 10/12/2010 08:01 AM, Daniel Veillard wrote: 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; } Or even one step further, and use annotations to mark the function arguments as ATTRIBUTE_NONNULL to get compiler checking of your logic. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 02/13] Adding virDomainSetMemoryParameters and virDomainGetMemoryParameters API
On Tue, Oct 12, 2010 at 08:16:11AM -0600, Eric Blake wrote: On 10/12/2010 08:01 AM, Daniel Veillard wrote: 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; } Or even one step further, and use annotations to mark the function arguments as ATTRIBUTE_NONNULL to get compiler checking of your logic. That's only true if the client application has the neccessary compile time warnings enabled during their build. If you annotate a function with nonnull, the docs say this activates further compiler optimizations. So I'd be concerned that annotating the public APIs with nonnull might let the compiler optimize away that 'params == NULL' check to nothing. At which point the app using libvirt would be at risk if they had not enabled compile warnings to activate the annotation Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 02/13] Adding virDomainSetMemoryParameters and virDomainGetMemoryParameters API
From: Nikunj A. Dadhania nik...@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. Signed-off-by: Nikunj A. Dadhania nik...@linux.vnet.ibm.com --- src/driver.h | 14 ++ src/esx/esx_driver.c |2 + src/libvirt.c | 104 src/libvirt_public.syms|6 +++ src/lxc/lxc_driver.c |2 + src/openvz/openvz_driver.c |2 + src/phyp/phyp_driver.c |2 + src/qemu/qemu_driver.c |2 + src/remote/remote_driver.c |2 + src/test/test_driver.c |2 + src/uml/uml_driver.c |2 + src/xen/xen_driver.c |2 + 12 files changed, 142 insertions(+), 0 deletions(-) diff --git a/src/driver.h b/src/driver.h index e443c1c..32aeb04 100644 --- a/src/driver.h +++ b/src/driver.h @@ -128,6 +128,18 @@ typedef int (*virDrvDomainSetMemory) (virDomainPtr domain, unsigned long memory); typedef int +(*virDrvDomainSetMemoryParameters) +(virDomainPtr domain, + virMemoryParameterPtr params, + int nparams, + unsigned int flags); +typedef int +(*virDrvDomainGetMemoryParameters) +(virDomainPtr domain, + virMemoryParameterPtr params, + int *nparams, + unsigned int flags); +typedef int (*virDrvDomainGetInfo) (virDomainPtr domain, virDomainInfoPtr info); typedef int @@ -575,6 +587,8 @@ struct _virDriver { virDrvDomainRevertToSnapshot domainRevertToSnapshot; virDrvDomainSnapshotDelete domainSnapshotDelete; virDrvQemuDomainMonitorCommand qemuDomainMonitorCommand; +virDrvDomainSetMemoryParameters domainSetMemoryParameters; +virDrvDomainGetMemoryParameters domainGetMemoryParameters; }; typedef int diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index e382950..e959be2 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4217,6 +4217,8 @@ static virDriver esxDriver = { esxDomainRevertToSnapshot, /* domainRevertToSnapshot */ esxDomainSnapshotDelete, /* domainSnapshotDelete */ NULL,/* qemuDomainMonitorCommand */ +NULL,/* domainSetMemoryParameters */ +NULL,/* domainGetMemoryParameters */ }; diff --git a/src/libvirt.c b/src/libvirt.c index ca383ba..d964a44 100644 --- 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) +{ +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; +} +conn = domain-conn; + +if (conn-driver-domainSetMemoryParameters) { +int ret; +ret = conn-driver-domainSetMemoryParameters (domain, params, nparams, flags); +if (ret 0) +goto error; +return ret; +} + +virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: +virDispatchError(domain-conn); +return -1; +} + +/** + * 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