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

Re: [libvirt] [PATCHv2 5/9] libvirt.h.in: Add new cpumap macro VIR_CPU_USED



On 10/16/2012 08:05 AM, Viktor Mihajlovski wrote:
> New macro VIR_CPU_USED added to facilitate the interpretation of
> cpu maps.
> Further, hardened the other cpumap macros against invocations
> like VIR_CPU_USE(cpumap + 1, cpu)
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov linux vnet ibm com>
> ---
>  include/libvirt/libvirt.h.in |   20 ++++++++++++++++----
>  1 files changed, 16 insertions(+), 4 deletions(-)

Hmm - should we rewrite VIR_CPU_USABLE in terms of VIR_CPU_USED()?

You also missed hardening VIR_COPY_CPUMAP and VIR_GET_CPUMAP.

> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 6b72159..3e8e4f8 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1954,7 +1954,7 @@ int                     virDomainGetEmulatorPinInfo (virDomainPtr domain,
>   * USE_CPU macro set the bit (CPU usable) of the related cpu in cpumap.

While we are touching here, lets fix the poor grammar:
s/USE_CPU macro set/It sets/

>   */
>  
> -#define VIR_USE_CPU(cpumap,cpu) (cpumap[(cpu)/8] |= (1<<((cpu)%8)))
> +#define VIR_USE_CPU(cpumap,cpu)   ((cpumap)[(cpu)/8] |= (1<<((cpu)%8)))

As long as we are touching this, we might as well use conventional
spacing (space after comma and around operators).

>  
>  /**
>   * VIR_UNUSE_CPU:
> @@ -1965,7 +1965,19 @@ int                     virDomainGetEmulatorPinInfo (virDomainPtr domain,
>   * USE_CPU macro reset the bit (CPU not usable) of the related cpu in cpumap.

Again, bad grammar.

>   */
>  
> -#define VIR_UNUSE_CPU(cpumap,cpu)       (cpumap[(cpu)/8] &= ~(1<<((cpu)%8)))
> +#define VIR_UNUSE_CPU(cpumap,cpu) ((cpumap)[(cpu)/8] &= ~(1<<((cpu)%8)))

And again, odd spacing.

> +
> +/**
> + * VIR_CPU_USED:
> + * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN)
> + * @cpu: the physical CPU number
> + *
> + * This macro can be used in conjunction with virNodeGetCPUMapFlags() API.
> + * VIR_CPU_USED returns true if the bit of the related CPU is set.

Must it return true (== 1), or should we document merely that it returns
non-zero...

> + *
> + */
> +
> +#define VIR_CPU_USED(cpumap,cpu)  ((cpumap)[(cpu)/8] & (1<<((cpu)%8)))

...given that you aren't returning a bool here?  But VIR_CPU_USABLE
didn't bother converting to bool, so this is just a docs issue.

>  
>  /**
>   * VIR_CPU_MAPLEN:
> @@ -1976,7 +1988,7 @@ int                     virDomainGetEmulatorPinInfo (virDomainPtr domain,
>   * CPU map between a single virtual & all physical CPUs of a domain.
>   */
>  
> -#define VIR_CPU_MAPLEN(cpu)      (((cpu)+7)/8)
> +#define VIR_CPU_MAPLEN(cpu)       (((cpu)+7)/8)

Why the whitespace change?  But we might as well fix operator spacing.

>  
>  
>  int                     virDomainGetVcpus       (virDomainPtr domain,
> @@ -1998,7 +2010,7 @@ int                     virDomainGetVcpus       (virDomainPtr domain,
>   */
>  
>  #define VIR_CPU_USABLE(cpumaps,maplen,vcpu,cpu) \
> -        (cpumaps[((vcpu)*(maplen))+((cpu)/8)] & (1<<((cpu)%8)))
> +    ((cpumaps)[((vcpu)*(maplen))+((cpu)/8)] & (1<<((cpu)%8)))
>  
>  /**
>   * VIR_COPY_CPUMAP:
> 

Here's what I'm squashing:

diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in
index adf5afb..428db39 100644
--- i/include/libvirt/libvirt.h.in
+++ w/include/libvirt/libvirt.h.in
@@ -2045,10 +2045,10 @@ int
virDomainGetEmulatorPinInfo (virDomainPtr domain,
  * @cpu: the physical CPU number
  *
  * This macro is to be used in conjunction with virDomainPinVcpu() API.
- * USE_CPU macro set the bit (CPU usable) of the related cpu in cpumap.
+ * It sets the bit (CPU usable) of the related cpu in cpumap.
  */

-#define VIR_USE_CPU(cpumap,cpu)   ((cpumap)[(cpu)/8] |= (1<<((cpu)%8)))
+#define VIR_USE_CPU(cpumap, cpu) ((cpumap)[(cpu) / 8] |= (1 << ((cpu) %
8)))

 /**
  * VIR_UNUSE_CPU:
@@ -2056,10 +2056,10 @@ int
virDomainGetEmulatorPinInfo (virDomainPtr domain,
  * @cpu: the physical CPU number
  *
  * This macro is to be used in conjunction with virDomainPinVcpu() API.
- * USE_CPU macro reset the bit (CPU not usable) of the related cpu in
cpumap.
+ * It resets the bit (CPU not usable) of the related cpu in cpumap.
  */

-#define VIR_UNUSE_CPU(cpumap,cpu) ((cpumap)[(cpu)/8] &= ~(1<<((cpu)%8)))
+#define VIR_UNUSE_CPU(cpumap, cpu) ((cpumap)[(cpu) / 8] &= ~(1 <<
((cpu) % 8)))

 /**
  * VIR_CPU_USED:
@@ -2067,11 +2067,10 @@ int
virDomainGetEmulatorPinInfo (virDomainPtr domain,
  * @cpu: the physical CPU number
  *
  * This macro can be used in conjunction with virNodeGetCPUMapFlags() API.
- * VIR_CPU_USED returns true if the bit of the related CPU is set.
- *
+ * It returns non-zero if the bit of the related CPU is set.
  */

-#define VIR_CPU_USED(cpumap,cpu)  ((cpumap)[(cpu)/8] & (1<<((cpu)%8)))
+#define VIR_CPU_USED(cpumap, cpu) ((cpumap)[(cpu) / 8] & (1 << ((cpu) %
8)))

 /**
  * VIR_CPU_MAPLEN:
@@ -2082,7 +2081,7 @@ int
virDomainGetEmulatorPinInfo (virDomainPtr domain,
  * CPU map between a single virtual & all physical CPUs of a domain.
  */

-#define VIR_CPU_MAPLEN(cpu)       (((cpu)+7)/8)
+#define VIR_CPU_MAPLEN(cpu) (((cpu) + 7) / 8)


 int                     virDomainGetVcpus       (virDomainPtr domain,
@@ -2099,12 +2098,12 @@ int                     virDomainGetVcpus
(virDomainPtr domain,
  * @cpu: the physical CPU number
  *
  * This macro is to be used in conjunction with virDomainGetVcpus() API.
- * VIR_CPU_USABLE macro returns a non zero value (true) if the cpu
+ * VIR_CPU_USABLE macro returns a non-zero value (true) if the cpu
  * is usable by the vcpu, and 0 otherwise.
  */

-#define VIR_CPU_USABLE(cpumaps,maplen,vcpu,cpu) \
-    ((cpumaps)[((vcpu)*(maplen))+((cpu)/8)] & (1<<((cpu)%8)))
+#define VIR_CPU_USABLE(cpumaps, maplen, vcpu, cpu) \
+    VIR_CPU_USED(VIR_GET_CPUMAP(cpumaps, maplen, vcpu), cpu)

 /**
  * VIR_COPY_CPUMAP:
@@ -2116,12 +2115,12 @@ int                     virDomainGetVcpus
(virDomainPtr domain,
  *      (ie: malloc(maplen))
  *
  * This macro is to be used in conjunction with virDomainGetVcpus() and
- * virDomainPinVcpu() APIs. VIR_COPY_CPUMAP macro extract the cpumap of
- * the specified vcpu from cpumaps array and copy it into cpumap to be used
+ * virDomainPinVcpu() APIs. VIR_COPY_CPUMAP macro extracts the cpumap of
+ * the specified vcpu from cpumaps array and copies it into cpumap to
be used
  * later by virDomainPinVcpu() API.
  */
-#define VIR_COPY_CPUMAP(cpumaps,maplen,vcpu,cpumap) \
-        memcpy(cpumap, &(cpumaps[(vcpu)*(maplen)]), (maplen))
+#define VIR_COPY_CPUMAP(cpumaps, maplen, vcpu, cpumap) \
+    memcpy(cpumap, VIR_GET_CPUMAP(cpumaps, maplen, vcpu), maplen)


 /**
@@ -2134,7 +2133,7 @@ int                     virDomainGetVcpus
(virDomainPtr domain,
  * virDomainPinVcpu() APIs. VIR_GET_CPUMAP macro returns a pointer to the
  * cpumap of the specified vcpu from cpumaps array.
  */
-#define VIR_GET_CPUMAP(cpumaps,maplen,vcpu)     &(cpumaps[(vcpu)*(maplen)])
+#define VIR_GET_CPUMAP(cpumaps, maplen, vcpu) (&((cpumaps)[(vcpu) *
(maplen)]))


 typedef enum {


-- 
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]