Re: [libvirt] [PATCH 1/3 V6] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.

2012-02-22 Thread Eric Blake 
On 02/16/2012 12:15 AM, Lai Jiangshan wrote:
 From: KAMEZAWA Hiroyuki kamezawa.hir...@jp.fujitsu.com
 
 Changelog:
  - fixed typos.
  - fixed string scan routine.
 
 Signed-off-by: KAMEZAWA Hiroyuki kamezawa.hir...@jp.fujitsu.com
 Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com
 ---
  src/libvirt_private.syms |1 +
  src/nodeinfo.c   |   92 
 ++
  src/nodeinfo.h   |3 +
  3 files changed, 96 insertions(+), 0 deletions(-)
 
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 0c22dec..6e99243 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -792,6 +792,7 @@ virNodeDeviceObjUnlock;
  
  # nodeinfo.h
  nodeCapsInitNUMA;
 +nodeGetCPUmap;
  nodeGetCPUStats;
  nodeGetCellsFreeMemory;
  nodeGetFreeMemory;
 diff --git a/src/nodeinfo.c b/src/nodeinfo.c
 index e0b66f7..fc1aaea 100644
 --- a/src/nodeinfo.c
 +++ b/src/nodeinfo.c
 @@ -31,6 +31,7 @@
  #include dirent.h
  #include sys/utsname.h
  #include sched.h
 +#include conf/domain_conf.h

This is a header internal to libvirt, so it should use , not .  And
somehow, introducing this header pulled in enough other stuff to make
the compiler complain about a later use of socket as a local variable name:

nodeinfo.c: In function 'linuxNodeInfoCPUPopulate':
nodeinfo.c:210:25: error: declaration of 'socket' shadows a global
declaration [-Werror=shadow]
cc1: all warnings being treated as errors

  
  #if HAVE_NUMACTL
  # define NUMA_VERSION1_COMPATIBILITY 1
 @@ -569,6 +570,73 @@ int linuxNodeGetMemoryStats(FILE *meminfo,
  cleanup:
  return ret;
  }
 +
 +/*
 + * Linux maintains cpu bit map. For example, if cpuid=5's flag is not set
 + * and max cpu is 7. The map file shows 0-4,6-7. This function parses
 + * it and returns cpumap.
 + */
 +static const char *
 +linuxParseCPUmap(int *max_cpuid, const char *path)
 +{
 +FILE *fp;
 +char *map = NULL;
 +char *str = NULL;
 +size_t size = 128, pos = 0;
 +int max_id, i;
 +
 +fp = fopen(path, r);
 +if (!fp) {
 +virReportSystemError(errno,  _(cannot open %s), path);
 +goto error;
 +}
 +
 +if (VIR_ALLOC_N(str, size)  0) {
 +virReportOOMError();
 +goto error;
 +}
 +for (;;) {
 +pos += fread(str + pos, 1, size - pos, fp);

Rather than using an fread() loop, I think we can simplify things by
using virFileReadAll().

 +if (pos  size)
 +break;
 +
 +size = size  1;
 +if (VIR_REALLOC_N(str, size)  0) {
 +virReportOOMError();
 +goto error;
 +}
 +}
 +if (pos == 0) {
 +virReportSystemError(errno, _(cannot read from %s), path);
 +goto error;
 +}
 +str[pos - 1] = 0;
 +
 +if (VIR_ALLOC_N(map, VIR_DOMAIN_CPUMASK_LEN)  0) {
 +virReportOOMError();
 +goto error;
 +}
 +if (virDomainCpuSetParse(str, 0, map,
 + VIR_DOMAIN_CPUMASK_LEN)  0) {
 +goto error;
 +}
 +
 +for (i = 0; i  VIR_DOMAIN_CPUMASK_LEN; i++) {
 +if (map[i]) {
 +max_id = i;

That's off by a factor of 8 - map[i] means that you have visited i*8
bits in cpumask.

 +}
 +}
 +*max_cpuid = max_id;
 +
 +VIR_FORCE_FCLOSE(fp);
 +return map;
 +
 +error:
 +VIR_FORCE_FCLOSE(fp);
 +VIR_FREE(str);
 +VIR_FREE(map);
 +return NULL;
 +}
  #endif
  
  int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr 
 nodeinfo) {
 @@ -712,6 +780,30 @@ int nodeGetMemoryStats(virConnectPtr conn 
 ATTRIBUTE_UNUSED,
  #endif
  }
  
 +const char *
 +nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED,
 +  int *max_id ATTRIBUTE_UNUSED,
 +  const char *mapname ATTRIBUTE_UNUSED)
 +{
 +#ifdef __linux__
 +char *path;
 +const char *cpumap;
 +
 +if (virAsprintf(path, CPU_SYS_PATH /%s, mapname)  0) {
 +virReportOOMError();
 +return NULL;
 +}
 +
 +cpumap = linuxParseCPUmap(max_id, path);
 +VIR_FREE(path);
 +return cpumap;
 +#else
 + nodeReportError(VIR_ERR_NO_SUPPORT, %s,
 + _(node cpumap not implemented on this platform));
 + return -1;

Returning -1 as a const char * won't compile.  You want NULL here.

 +#endif
 +}
 +
  #if HAVE_NUMACTL
  # if LIBNUMA_API_VERSION = 1
  #  define NUMA_MAX_N_CPUS 4096
 diff --git a/src/nodeinfo.h b/src/nodeinfo.h
 index 4766152..852e19d 100644
 --- a/src/nodeinfo.h
 +++ b/src/nodeinfo.h
 @@ -46,4 +46,7 @@ int nodeGetCellsFreeMemory(virConnectPtr conn,
 int maxCells);
  unsigned long long nodeGetFreeMemory(virConnectPtr conn);
  
 +const char *nodeGetCPUmap(virConnectPtr conn,
 +  int *max_id,
 +  const char *mapname);
  #endif /* __VIR_NODEINFO_H__*/

I'll see if I can fix these things myself - I'm anxious to get this
pushed sooner rather than later.  I'll continue my review, and if I
polish the

Re: [libvirt] [PATCH 1/3 V6] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.

2012-02-22 Thread Lai Jiangshan 
On 02/23/2012 08:10 AM, Eric Blake wrote:

 +if (virDomainCpuSetParse(str, 0, map,
 + VIR_DOMAIN_CPUMASK_LEN)  0) {
 +goto error;
 +}
 +
 +for (i = 0; i  VIR_DOMAIN_CPUMASK_LEN; i++) {
 +if (map[i]) {
 +max_id = i;
 
 That's off by a factor of 8 - map[i] means that you have visited i*8
 bits in cpumask.

@map is not bitmask, virDomainCpuSetParse() filled it a char per a cpu.
And the return of this function is also cpumap(byte per cpu).

I will rework the patches soon after you comments them.

Thank you very much.

--lai.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3 V6] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.

2012-02-21 Thread Lai Jiangshan 
Hi, Eric

Did you received these new patches?

Thanks,
Lai


On 02/16/2012 03:15 PM, Lai Jiangshan wrote:
 From: KAMEZAWA Hiroyuki kamezawa.hir...@jp.fujitsu.com
 
 Changelog:
  - fixed typos.
  - fixed string scan routine.
 
 Signed-off-by: KAMEZAWA Hiroyuki kamezawa.hir...@jp.fujitsu.com
 Signed-off-by: Lai Jiangshan la...@cn.fujitsu.com
 ---
  src/libvirt_private.syms |1 +
  src/nodeinfo.c   |   92 
 ++
  src/nodeinfo.h   |3 +
  3 files changed, 96 insertions(+), 0 deletions(-)
 
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 0c22dec..6e99243 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -792,6 +792,7 @@ virNodeDeviceObjUnlock;
  
  # nodeinfo.h
  nodeCapsInitNUMA;
 +nodeGetCPUmap;
  nodeGetCPUStats;
  nodeGetCellsFreeMemory;
  nodeGetFreeMemory;
 diff --git a/src/nodeinfo.c b/src/nodeinfo.c
 index e0b66f7..fc1aaea 100644
 --- a/src/nodeinfo.c
 +++ b/src/nodeinfo.c
 @@ -31,6 +31,7 @@
  #include dirent.h
  #include sys/utsname.h
  #include sched.h
 +#include conf/domain_conf.h
  
  #if HAVE_NUMACTL
  # define NUMA_VERSION1_COMPATIBILITY 1
 @@ -569,6 +570,73 @@ int linuxNodeGetMemoryStats(FILE *meminfo,
  cleanup:
  return ret;
  }
 +
 +/*
 + * Linux maintains cpu bit map. For example, if cpuid=5's flag is not set
 + * and max cpu is 7. The map file shows 0-4,6-7. This function parses
 + * it and returns cpumap.
 + */
 +static const char *
 +linuxParseCPUmap(int *max_cpuid, const char *path)
 +{
 +FILE *fp;
 +char *map = NULL;
 +char *str = NULL;
 +size_t size = 128, pos = 0;
 +int max_id, i;
 +
 +fp = fopen(path, r);
 +if (!fp) {
 +virReportSystemError(errno,  _(cannot open %s), path);
 +goto error;
 +}
 +
 +if (VIR_ALLOC_N(str, size)  0) {
 +virReportOOMError();
 +goto error;
 +}
 +for (;;) {
 +pos += fread(str + pos, 1, size - pos, fp);
 +if (pos  size)
 +break;
 +
 +size = size  1;
 +if (VIR_REALLOC_N(str, size)  0) {
 +virReportOOMError();
 +goto error;
 +}
 +}
 +if (pos == 0) {
 +virReportSystemError(errno, _(cannot read from %s), path);
 +goto error;
 +}
 +str[pos - 1] = 0;
 +
 +if (VIR_ALLOC_N(map, VIR_DOMAIN_CPUMASK_LEN)  0) {
 +virReportOOMError();
 +goto error;
 +}
 +if (virDomainCpuSetParse(str, 0, map,
 + VIR_DOMAIN_CPUMASK_LEN)  0) {
 +goto error;
 +}
 +
 +for (i = 0; i  VIR_DOMAIN_CPUMASK_LEN; i++) {
 +if (map[i]) {
 +max_id = i;
 +}
 +}
 +*max_cpuid = max_id;
 +
 +VIR_FORCE_FCLOSE(fp);
 +return map;
 +
 +error:
 +VIR_FORCE_FCLOSE(fp);
 +VIR_FREE(str);
 +VIR_FREE(map);
 +return NULL;
 +}
  #endif
  
  int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr 
 nodeinfo) {
 @@ -712,6 +780,30 @@ int nodeGetMemoryStats(virConnectPtr conn 
 ATTRIBUTE_UNUSED,
  #endif
  }
  
 +const char *
 +nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED,
 +  int *max_id ATTRIBUTE_UNUSED,
 +  const char *mapname ATTRIBUTE_UNUSED)
 +{
 +#ifdef __linux__
 +char *path;
 +const char *cpumap;
 +
 +if (virAsprintf(path, CPU_SYS_PATH /%s, mapname)  0) {
 +virReportOOMError();
 +return NULL;
 +}
 +
 +cpumap = linuxParseCPUmap(max_id, path);
 +VIR_FREE(path);
 +return cpumap;
 +#else
 + nodeReportError(VIR_ERR_NO_SUPPORT, %s,
 + _(node cpumap not implemented on this platform));
 + return -1;
 +#endif
 +}
 +
  #if HAVE_NUMACTL
  # if LIBNUMA_API_VERSION = 1
  #  define NUMA_MAX_N_CPUS 4096
 diff --git a/src/nodeinfo.h b/src/nodeinfo.h
 index 4766152..852e19d 100644
 --- a/src/nodeinfo.h
 +++ b/src/nodeinfo.h
 @@ -46,4 +46,7 @@ int nodeGetCellsFreeMemory(virConnectPtr conn,
 int maxCells);
  unsigned long long nodeGetFreeMemory(virConnectPtr conn);
  
 +const char *nodeGetCPUmap(virConnectPtr conn,
 +  int *max_id,
 +  const char *mapname);
  #endif /* __VIR_NODEINFO_H__*/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list