[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 1/9] virNodeGetCPUMapFlags: Define public API.



On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
> Adding a new API to obtain information about the
> host node's present, online and offline CPUs.
> 
> int virNodeGetCPUMapFlags(virConnectPtr conn,
>                           unsigned char **cpumap,
>     			  unsigned int *online,

TAB damage.

>                           unsigned int flags);

No need for the Flags suffix in the API name.  The suffix is only needed
when we are adding a new API to fix shortcomings in an older one that
forgot a flags argument.

> 
> The function will return the number of CPUs present on the host
> or -1 on failure;
> If cpumap is non-NULL virNodeGetCPUMapFlags will allocate an array
> containing a bit map representation of the online CPUs. It's
> the callers responsibility to deallocate cpumap using free().
> If online is non-NULL, the variable pointed to will contain
> the number of online host node CPUs.
> The variable flags has been added to support future extensions
> and must be set to 0.
> 
> Note: the name virNodeGetCPUMapFlags has been chosen to avoid
> confusion with the nodeinfo function nodeGetCPUmap.

Not necessary - nodeGetCPUmap is an internal function, which we can
rename at will.  Public API naming should not be held hostage to
internal names.

> 
> Signed-off-by: Viktor Mihajlovski <mihajlov linux vnet ibm com>
> ---
>  include/libvirt/libvirt.h.in |    8 ++++++++
>  python/generator.py          |    1 +
>  src/libvirt_public.syms      |    5 +++++
>  3 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index a4e8ca9..6b72159 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -4500,6 +4500,14 @@ int virNodeSetMemoryParameters(virConnectPtr conn,
>                                 int nparams,
>                                 unsigned int flags);
>  
> +/*
> + *  node CPU map
> + */
> +int virNodeGetCPUMapFlags(virConnectPtr conn,
> +                          unsigned char **cpumap,
> +                          unsigned int *online,
> +                          unsigned int flags);
> +
>  #ifdef __cplusplus
>  }
>  #endif

Too low in the file - you ended up declaring it in the section for
deprecated names.  Not entirely your fault; virNodeSetMemoryParameters
was also declared in the wrong place, so I prepared a separate patch to
hoist that up first:
https://www.redhat.com/archives/libvir-list/2012-October/msg01360.html

> diff --git a/python/generator.py b/python/generator.py
> index ced7e41..8591b8d 100755
> --- a/python/generator.py
> +++ b/python/generator.py
> @@ -429,6 +429,7 @@ skip_impl = (
>      'virConnectRegisterCloseCallback',
>      'virNodeGetMemoryParameters',
>      'virNodeSetMemoryParameters',
> +    'virNodeGetCPUMapFlags',
>  )
>  
>  qemu_skip_impl = (
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 2c924d5..0c8f4ff 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -569,4 +569,9 @@ LIBVIRT_0.10.2 {
>          virStoragePoolListAllVolumes;
>  } LIBVIRT_0.10.0;
>  
> +LIBVIRT_1.0.0 {
> +    global:
> +        virNodeGetCPUMapFlags;
> +} LIBVIRT_0.10.2;
> +
>  # .... define new API here using predicted next version number ....
> 

ACK to the idea, but the documentation should be present in
src/libvirt.c at the time the function is introduced.  In addition to
touching up the commit message, I will probably squash in your later
patch to libvirt.c, as well as these changes (on top of my patch
mentioned above for reordering the header file):

diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in
index 2b33ba8..368e014 100644
--- i/include/libvirt/libvirt.h.in
+++ w/include/libvirt/libvirt.h.in
@@ -750,6 +750,14 @@ int virNodeSetMemoryParameters(virConnectPtr conn,
                                int nparams,
                                unsigned int flags);

+/*
+ *  node CPU map
+ */
+int virNodeGetCPUMap(virConnectPtr conn,
+                     unsigned char **cpumap,
+                     unsigned int *online,
+                     unsigned int flags);
+
 /* Management of scheduler parameters */

 /**
@@ -4546,14 +4554,6 @@ typedef virMemoryParameter *virMemoryParameterPtr;
 /* Add new interfaces to the appropriate sections earlier in this
  * file; the end of the file is reserved for deprecated names.  */

-/*
- *  node CPU map
- */
-int virNodeGetCPUMapFlags(virConnectPtr conn,
-                          unsigned char **cpumap,
-                          unsigned int *online,
-                          unsigned int flags);
-
 #ifdef __cplusplus
 }
 #endif

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]