On 01/21/13 12:31, Martin Kletzander wrote:
On 01/19/2013 12:06 AM, Peter Krempa wrote:
This will allow storing additional topology data in the NUMA topology
definition.
This patch changes the storage type and fixes fallback of the change
across the drivers using it.
This patch also changes semantics of adding new NUMA cell information.
Until now the data were re-allocated and copied to the topology
definition. This patch changes the addition function to steal the
pointer to a pre-allocated structure to simplify the code.
---
src/conf/capabilities.c | 29 ++++++++++++++++++-----------
src/conf/capabilities.h | 14 ++++++++++++--
src/nodeinfo.c | 22 ++++++++++++----------
src/qemu/qemu_process.c | 2 +-
src/test/test_driver.c | 15 ++++++++++++---
src/xen/xend_internal.c | 24 +++++++++++++-----------
6 files changed, 68 insertions(+), 38 deletions(-)
[...]
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index 19b99c6..124d8c1 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -84,12 +84,21 @@ struct _virCapsGuest {
virCapsGuestFeaturePtr *features;
};
+typedef struct _virCapsHostNUMACellCPU virCapsHostNUMACellCPU;
+typedef virCapsHostNUMACellCPU *virCapsHostNUMACellCPUPtr;
+struct _virCapsHostNUMACellCPU {
+ int id;
+ int socket_id;
+ int core_id;
+ char *siblings_list;
We don't actually need this for anything right now, but it would be nice
to have it stored in virBitmap in case we'll want to work with it in the
future.
[...]
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 477104f..dffe7d1 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -79,9 +79,11 @@ freebsdNodeGetCPUCount(void)
#ifdef __linux__
# define CPUINFO_PATH "/proc/cpuinfo"
# define SYSFS_SYSTEM_PATH "/sys/devices/system"
+# define SYSFS_CPU_PATH SYSFS_SYSTEM_PATH"/cpu"
s/PATH"/PATH "/
# define PROCSTAT_PATH "/proc/stat"
# define MEMINFO_PATH "/proc/meminfo"
# define SYSFS_MEMORY_SHARED_PATH "/sys/kernel/mm/ksm"
+# define SYSFS_THREAD_SIBLINGS_LIST_LENGHT_MAX 1024
# define LINUX_NB_CPU_STATS 4
# define LINUX_NB_MEMORY_STATS_ALL 4
@@ -1476,9 +1478,10 @@ nodeCapsInitNUMA(virCapsPtr caps)
int n;
unsigned long *mask = NULL;
unsigned long *allonesmask = NULL;
- int *cpus = NULL;
+ virCapsHostNUMACellCPUPtr cpus = NULL;
int ret = -1;
int max_n_cpus = NUMA_MAX_N_CPUS;
+ int ncpus = 0;
if (numa_available() < 0)
return 0;
@@ -1492,7 +1495,6 @@ nodeCapsInitNUMA(virCapsPtr caps)
for (n = 0 ; n <= numa_max_node() ; n++) {
int i;
- int ncpus;
/* The first time this returns -1, ENOENT if node doesn't exist... */
if (numa_node_to_cpus(n, mask, mask_n_bytes) < 0) {
VIR_WARN("NUMA topology for cell %d of %d not available, ignoring",
@@ -1515,21 +1517,21 @@ nodeCapsInitNUMA(virCapsPtr caps)
for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++)
if (MASK_CPU_ISSET(mask, i))
- cpus[ncpus++] = i;
+ cpus[ncpus++].id = i;
- if (virCapabilitiesAddHostNUMACell(caps,
- n,
- ncpus,
- cpus) < 0)
+ if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, cpus) < 0)
goto cleanup;
-
- VIR_FREE(cpus);
+ cpus = NULL;
I generally don't like stealing pointers because of similar
contraptions. Even though it is clear from memory allocation POV and it
saves a lot of allocations in this case, it's a bit harder to read IMHO.
Well, yeah, it might be harder to read, but I wanted to avoid doing a
deep copy on each iteration. And the callers of this function freed the
original array anyways so I think it was really a waste of operations.
Being the devil's advocate, I must say I'd be nice to have the tests for
this since you've started modifying the test driver. Also documentation
for all the new information should be added into
'docs/formatcaps.html.in' with some explanation.
I'm planing on adding docs and tests, but I was waiting for a review to
settle the format before doing so. (And I forgot to write about that)
Xen part of this patch looks OK, although I don't have any XEN machine
to try it on.
Because this and [PATCH 5/5] both require [PATCH 3/5] to be in, I'm
cond-ACKing this patch with the same condition as [PATCH 3/5].
I assume that you mean patch 2/5 :)
Martin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list