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

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



On 03/01/2012 07:54 PM, Lai Jiangshan wrote:
> From: KAMEZAWA Hiroyuki <kamezawa hiroyu jp fujitsu com>
> 
> Changelog:
>  - fixed typos.
>  - fixed string scan routine.

While useful for reviews, information like this doesn't help the
permanent libvirt.git log, so it is best to place it after the --- divider.

> 
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa hiroyu jp fujitsu com>
> Signed-off-by: Lai Jiangshan <laijs cn fujitsu com>
> ---
>  src/libvirt_private.syms |    1 +
>  src/nodeinfo.c           |   66 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/nodeinfo.h           |    3 ++
>  3 files changed, 70 insertions(+), 0 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index a104e70..0f4e64c 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -793,6 +793,7 @@ virNodeDeviceObjUnlock;
>  
>  # nodeinfo.h
>  nodeCapsInitNUMA;
> +nodeGetCPUmap;
>  nodeGetCPUStats;
>  nodeGetCellsFreeMemory;
>  nodeGetFreeMemory;
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index e0b66f7..2950306 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"

I'm not convinced that this cross-directory include is safe, but my
recent syntax check doesn't flag it (which may point to a weakness in
the syntax check).  But if it needs fixing, we can do that later.

> +const char *
> +nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED,
> +              int *max_id ATTRIBUTE_UNUSED,
> +              const char *mapname ATTRIBUTE_UNUSED)

This function is returning malloc'd memory; our convention is to type it
'char *', not 'const char *', to make it obvious that the caller is
responsible to free the data ('const char *' should be reserved for
static data that the caller need not free).

> +    return cpumap;
> +#else
> +     nodeReportError(VIR_ERR_NO_SUPPORT, "%s",

Indentation.

ACK with this squashed in:

diff --git i/src/nodeinfo.c w/src/nodeinfo.c
index 9b19440..709e94a 100644
--- i/src/nodeinfo.c
+++ w/src/nodeinfo.c
@@ -1,7 +1,7 @@
 /*
  * nodeinfo.c: Helper routines for OS specific node information
  *
- * Copyright (C) 2006-2008, 2010-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2008, 2010-2012 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -576,7 +576,7 @@ cleanup:
  * and max cpu is 7. The map file shows 0-4,6-7. This function parses
  * it and returns cpumap.
  */
-static const char *
+static char *
 linuxParseCPUmap(int *max_cpuid, const char *path)
 {
     char *map = NULL;
@@ -754,14 +754,14 @@ int nodeGetMemoryStats(virConnectPtr conn
ATTRIBUTE_UNUSED,
 #endif
 }

-const char *
+char *
 nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED,
               int *max_id ATTRIBUTE_UNUSED,
               const char *mapname ATTRIBUTE_UNUSED)
 {
 #ifdef __linux__
     char *path;
-    const char *cpumap;
+    char *cpumap;

     if (virAsprintf(&path, CPU_SYS_PATH "/%s", mapname) < 0) {
         virReportOOMError();
@@ -772,9 +772,9 @@ nodeGetCPUmap(virConnectPtr conn ATTRIBUTE_UNUSED,
     VIR_FREE(path);
     return cpumap;
 #else
-     nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
-                     _("node cpumap not implemented on this platform"));
-     return NULL;
+    nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
+                    _("node cpumap not implemented on this platform"));
+    return NULL;
 #endif
 }

diff --git i/src/nodeinfo.h w/src/nodeinfo.h
index 852e19d..7d2ef1d 100644
--- i/src/nodeinfo.h
+++ w/src/nodeinfo.h
@@ -1,7 +1,7 @@
 /*
  * nodeinfo.c: Helper routines for OS specific node information
  *
- * Copyright (C) 2006-2008, 2011 Red Hat, Inc.
+ * Copyright (C) 2006-2008, 2011-2012 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -46,7 +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);
+char *nodeGetCPUmap(virConnectPtr conn,
+                    int *max_id,
+                    const char *mapname);
 #endif /* __VIR_NODEINFO_H__*/


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