3:30 p.m.
This version fixes most of coments on v1 by DanPB and Martin.
The two things that are not yet done are:
1) docs and
2) support for the xend driver
I will follow up with those tomorrow after I create a testbed for xen.
There are 2 new patches:
1/7: fixes bad comment for a bitmap function
7/7: adds data for the test driver
Peter Krempa (7):
util: Fix docs for virBitmapParse
schema: Make the cpuset type reusable across schema files
schemas: Add schemas for more CPU topology information in the caps XML
conf: Split out NUMA topology formatting to simplify access to data
capabilities: Switch CPU data in NUMA topology to a struct
capabilities: Add additional data to the NUMA topology info
test: Add support for thread and core information for the test driver
docs/schemas/basictypes.rng | 6 +++
docs/schemas/capability.rng | 11 ++++++
docs/schemas/domaincommon.rng | 5 ---
src/conf/capabilities.c | 92 ++++++++++++++++++++++++++++++-------------
src/conf/capabilities.h | 14 ++++++-
src/nodeinfo.c | 75 +++++++++++++++++++++++++++++------
src/qemu/qemu_process.c | 2 +-
src/test/test_driver.c | 24 +++++++++--
src/util/virbitmap.c | 16 ++++----
src/xen/xend_internal.c | 25 ++++++------
10 files changed, 201 insertions(+), 69 deletions(-)
--
1.8.1.1
3:30 p.m.
New subject: [libvirt] [PATCHv2 1/7] util: Fix docs for virBitmapParse
The documentation comment virBitmapParse didn't document the @sep
parameter.
---
src/util/virbitmap.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index ca82d1b..e3ca4ff 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -265,23 +265,25 @@ char *virBitmapFormat(virBitmapPtr bitmap)
/**
* virBitmapParse:
* @str: points to a string representing a human-readable bitmap
+ * @sep: separator character
* @bitmap: a bitmap created from @str
* @bitmapSize: the upper limit of num of bits in created bitmap
*
* This function is the counterpart of virBitmapFormat. This function creates
* a bitmap, in which bits are set according to the content of @str.
*
- * @str is a comma separated string of fields N, which means a number of bit
- * to set, and ^N, which means to unset the bit, and N-M for ranges of bits
- * to set.
+ * @str is a string of fields N separated by a character @sep, which means a
+ * number of bit to set, and ^N, which means to unset the bit, and N-M for
+ * ranges of bits to set.
*
* Returns the number of bits set in @bitmap, or -1 in case of error.
*/
-int virBitmapParse(const char *str,
- char sep,
- virBitmapPtr *bitmap,
- size_t bitmapSize)
+int
+virBitmapParse(const char *str,
+ char sep,
+ virBitmapPtr *bitmap,
+ size_t bitmapSize)
{
int ret = 0;
bool neg = false;
--
1.8.1.1
4:25 p.m.
New subject: [libvirt] [PATCHv2 1/7] util: Fix docs for virBitmapParse
On 01/22/2013 02:30 PM, Peter Krempa wrote:
The documentation comment virBitmapParse didn't document the
@sep
parameter.
---
src/util/virbitmap.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index ca82d1b..e3ca4ff 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -265,23 +265,25 @@ char *virBitmapFormat(virBitmapPtr bitmap)
/**
* virBitmapParse:
* @str: points to a string representing a human-readable bitmap
+ * @sep: separator character
* @bitmap: a bitmap created from @str
* @bitmapSize: the upper limit of num of bits in created bitmap
*
* This function is the counterpart of virBitmapFormat. This function creates
* a bitmap, in which bits are set according to the content of @str.
*
- * @str is a comma separated string of fields N, which means a number of bit
- * to set, and ^N, which means to unset the bit, and N-M for ranges of bits
- * to set.
+ * @str is a string of fields N separated by a character @sep, which means a
+ * number of bit to set, and ^N, which means to unset the bit, and N-M for
+ * ranges of bits to set.
This wording confused me. @sep isn't between each field, so much as at
the end of all of the fields. Most callers pass 0, but there are some
callers that pass ',' and even one that passes 'n'; so the use of
'sep'
is when you are parsing a bitmap out of a larger string, and want to
stop at the separator that says where the bitmap ends and the rest of
the string continues (and NOT that it separates fields, unless 'sep'
happens to be ',' to parse exactly one field).
But I don't know if I have any better wording, so weak ACK, especially
if you can improve it.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:18 p.m.
New subject: [libvirt] [PATCHv2 1/7] util: Fix docs for virBitmapParse
On 01/22/13 23:25, Eric Blake wrote:
On 01/22/2013 02:30 PM, Peter Krempa wrote:
> The documentation comment virBitmapParse didn't document the @sep
> parameter.
> ---
> src/util/virbitmap.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
> diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
> index ca82d1b..e3ca4ff 100644
> --- a/src/util/virbitmap.c
> +++ b/src/util/virbitmap.c
> @@ -265,23 +265,25 @@ char *virBitmapFormat(virBitmapPtr bitmap)
> /**
> * virBitmapParse:
> * @str: points to a string representing a human-readable bitmap
> + * @sep: separator character
> * @bitmap: a bitmap created from @str
> * @bitmapSize: the upper limit of num of bits in created bitmap
> *
> * This function is the counterpart of virBitmapFormat. This function creates
> * a bitmap, in which bits are set according to the content of @str.
> *
> - * @str is a comma separated string of fields N, which means a number of bit
> - * to set, and ^N, which means to unset the bit, and N-M for ranges of bits
> - * to set.
> + * @str is a string of fields N separated by a character @sep, which means a
> + * number of bit to set, and ^N, which means to unset the bit, and N-M for
> + * ranges of bits to set.
This wording confused me. @sep isn't between each field, so much as at
the end of all of the fields. Most callers pass 0, but there are some
callers that pass ',' and even one that passes 'n'; so the use of
'sep'
is when you are parsing a bitmap out of a larger string, and want to
stop at the separator that says where the bitmap ends and the rest of
the string continues (and NOT that it separates fields, unless 'sep'
happens to be ',' to parse exactly one field).
But I don't know if I have any better wording, so weak ACK, especially
if you can improve it.
Uhm, now that you pointed out I realized that I don't really know what
is the argument used for. I will need to think about it a little more.
If it's used for separating the bitmap from a larger string I will need
to tweak the comment. I thought it's used to allow other separators than
commas in the bitmap that is being parsed itself. I honestly didn't read
the code carefully enough.
I'll try to reverse engineer it in the morning.
Peter
8:24 a.m.
New subject: [libvirt] [PATCHv2 1/7] util: Fix docs for virBitmapParse
On 01/23/2013 12:18 AM, Peter Krempa wrote:
On 01/22/13 23:25, Eric Blake wrote:
> On 01/22/2013 02:30 PM, Peter Krempa wrote:
>> The documentation comment virBitmapParse didn't document the @sep
>> parameter.
>> ---
>> src/util/virbitmap.c | 16 +++++++++-------
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
>> index ca82d1b..e3ca4ff 100644
>> --- a/src/util/virbitmap.c
>> +++ b/src/util/virbitmap.c
>> @@ -265,23 +265,25 @@ char *virBitmapFormat(virBitmapPtr bitmap)
>> /**
>> * virBitmapParse:
>> * @str: points to a string representing a human-readable bitmap
>> + * @sep: separator character
>> * @bitmap: a bitmap created from @str
>> * @bitmapSize: the upper limit of num of bits in created bitmap
>> *
>> * This function is the counterpart of virBitmapFormat. This
>> function creates
>> * a bitmap, in which bits are set according to the content of @str.
>> *
>> - * @str is a comma separated string of fields N, which means a
>> number of bit
>> - * to set, and ^N, which means to unset the bit, and N-M for ranges
>> of bits
>> - * to set.
>> + * @str is a string of fields N separated by a character @sep, which
>> means a
>> + * number of bit to set, and ^N, which means to unset the bit, and
>> N-M for
>> + * ranges of bits to set.
>
> This wording confused me. @sep isn't between each field, so much as at
> the end of all of the fields. Most callers pass 0, but there are some
> callers that pass ',' and even one that passes 'n'; so the use of
'sep'
> is when you are parsing a bitmap out of a larger string, and want to
> stop at the separator that says where the bitmap ends and the rest of
> the string continues (and NOT that it separates fields, unless 'sep'
> happens to be ',' to parse exactly one field).
>
> But I don't know if I have any better wording, so weak ACK, especially
> if you can improve it.
Uhm, now that you pointed out I realized that I don't really know what
is the argument used for. I will need to think about it a little more.
If it's used for separating the bitmap from a larger string I will need
to tweak the comment. I thought it's used to allow other separators than
commas in the bitmap that is being parsed itself. I honestly didn't read
the code carefully enough.
I'll try to reverse engineer it in the morning.
From what I knew, the @sep is used as a character that should not be
parsed, delimiter let's say. You can see it's being checked for
occurrence whenever the check is made for '\0'. I saw it in another
function associated with file loading, which I can't find even though I
looked throughout the whole libvirt codebase.
I'd suggest changing it to delim(iter). And if I'll find the other part
where it wasn't documented as well, I'll change it there too.
Martin
8:28 a.m.
New subject: [libvirt] [PATCHv2 1/7] util: Fix docs for virBitmapParse
On Tue, Jan 22, 2013 at 03:25:04PM -0700, Eric Blake wrote:
On 01/22/2013 02:30 PM, Peter Krempa wrote:
> The documentation comment virBitmapParse didn't document the @sep
> parameter.
> ---
> src/util/virbitmap.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
> index ca82d1b..e3ca4ff 100644
> --- a/src/util/virbitmap.c
> +++ b/src/util/virbitmap.c
> @@ -265,23 +265,25 @@ char *virBitmapFormat(virBitmapPtr bitmap)
> /**
> * virBitmapParse:
> * @str: points to a string representing a human-readable bitmap
> + * @sep: separator character
> * @bitmap: a bitmap created from @str
> * @bitmapSize: the upper limit of num of bits in created bitmap
> *
> * This function is the counterpart of virBitmapFormat. This function creates
> * a bitmap, in which bits are set according to the content of @str.
> *
> - * @str is a comma separated string of fields N, which means a number of bit
> - * to set, and ^N, which means to unset the bit, and N-M for ranges of bits
> - * to set.
> + * @str is a string of fields N separated by a character @sep, which means a
> + * number of bit to set, and ^N, which means to unset the bit, and N-M for
> + * ranges of bits to set.
This wording confused me. @sep isn't between each field, so much as at
the end of all of the fields. Most callers pass 0, but there are some
callers that pass ',' and even one that passes 'n'; so the use of
'sep'
is when you are parsing a bitmap out of a larger string, and want to
stop at the separator that says where the bitmap ends and the rest of
the string continues (and NOT that it separates fields, unless 'sep'
happens to be ',' to parse exactly one field).
But I don't know if I have any better wording, so weak ACK, especially
if you can improve it.
In other words it is a "terminator", rather than a "separator".
Passing
'0' is basically indicating that the string is NULL terminated.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
8:48 a.m.
New subject: [libvirt] [PATCHv2 1/7] util: Fix docs for virBitmapParse
On 01/23/13 15:28, Daniel P. Berrange wrote:
On Tue, Jan 22, 2013 at 03:25:04PM -0700, Eric Blake wrote:
> On 01/22/2013 02:30 PM, Peter Krempa wrote:
>> The documentation comment virBitmapParse didn't document the @sep
>> parameter.
>> ---
>> src/util/virbitmap.c | 16 +++++++++-------
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
>> index ca82d1b..e3ca4ff 100644
>> --- a/src/util/virbitmap.c
>> +++ b/src/util/virbitmap.c
>> @@ -265,23 +265,25 @@ char *virBitmapFormat(virBitmapPtr bitmap)
>> /**
>> * virBitmapParse:
>> * @str: points to a string representing a human-readable bitmap
>> + * @sep: separator character
>> * @bitmap: a bitmap created from @str
>> * @bitmapSize: the upper limit of num of bits in created bitmap
>> *
>> * This function is the counterpart of virBitmapFormat. This function creates
>> * a bitmap, in which bits are set according to the content of @str.
>> *
>> - * @str is a comma separated string of fields N, which means a number of bit
>> - * to set, and ^N, which means to unset the bit, and N-M for ranges of bits
>> - * to set.
>> + * @str is a string of fields N separated by a character @sep, which means a
>> + * number of bit to set, and ^N, which means to unset the bit, and N-M for
>> + * ranges of bits to set.
>
> This wording confused me. @sep isn't between each field, so much as at
> the end of all of the fields. Most callers pass 0, but there are some
> callers that pass ',' and even one that passes 'n'; so the use of
'sep'
> is when you are parsing a bitmap out of a larger string, and want to
> stop at the separator that says where the bitmap ends and the rest of
> the string continues (and NOT that it separates fields, unless 'sep'
> happens to be ',' to parse exactly one field).
>
> But I don't know if I have any better wording, so weak ACK, especially
> if you can improve it.
In other words it is a "terminator", rather than a "separator".
Passing
'0' is basically indicating that the string is NULL terminated.
Daniel
I posted a separate series to address this issue.
Peter
3:30 p.m.
New subject: [libvirt] [PATCHv2 2/7] schema: Make the cpuset type reusable across schema files
---
docs/schemas/basictypes.rng | 6 ++++++
docs/schemas/domaincommon.rng | 5 -----
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 38cab16..8e44e8d 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -219,4 +219,10 @@
</data>
</define>
+ <define name="cpuset">
+ <data type="string">
+ <param
name="pattern">([0-9]+(-[0-9]+)?|\^[0-9]+)(,([0-9]+(-[0-9]+)?|\^[0-9]+))*</param>
+ </data>
+ </define>
+
</grammar>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 67ae864..008a62f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3672,11 +3672,6 @@
<!--
Type library
-->
- <define name="cpuset">
- <data type="string">
- <param
name="pattern">([0-9]+(-[0-9]+)?|\^[0-9]+)(,([0-9]+(-[0-9]+)?|\^[0-9]+))*</param>
- </data>
- </define>
<define name="countCPU">
<data type="unsignedShort">
<param name="pattern">[0-9]+</param>
--
1.8.1.1
6:44 p.m.
New subject: [libvirt] [PATCHv2 2/7] schema: Make the cpuset type reusable across schema files
On 01/22/2013 02:30 PM, Peter Krempa wrote:
---
docs/schemas/basictypes.rng | 6 ++++++
docs/schemas/domaincommon.rng | 5 -----
2 files changed, 6 insertions(+), 5 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:30 p.m.
New subject: [libvirt] [PATCHv2 3/7] schemas: Add schemas for more CPU topology information in the caps XML
This patch adds RNG schemas for adding more information in the topology
output of the NUMA section in the capabilities XML.
The added elements are designed to provide more information about the
placement and topology of the processors in the system to management
applications.
A demonstration of supported XML added by this patch:
<capabilities>
<host>
<topology>
<cells num='3'>
<cell id='0'>
<cpus num='4'> <!-- this is node with Hyperthreading -->
<cpu id='0' socket_id='0' core_id='0'
siblings='0-1'/>
<cpu id='1' socket_id='0' core_id='0'
siblings='0-1'/>
<cpu id='2' socket_id='0' core_id='1'
siblings='2-3'/>
<cpu id='3' socket_id='0' core_id='1'
siblings='2-3'/>
</cpus>
</cell>
<cell id='1'>
<cpus num='4'> <!-- this is node with modules (Bulldozer)
-->
<cpu id='4' socket_id='0' core_id='2'
siblings='4-5'/>
<cpu id='5' socket_id='0' core_id='3'
siblings='4-5'/>
<cpu id='6' socket_id='0' core_id='4'
siblings='6-7'/>
<cpu id='7' socket_id='0' core_id='5'
siblings='6-7'/>
</cpus>
</cell>
<cell id='2'>
<cpus num='4'> <!-- this is a normal multi-core node -->
<cpu id='8' socket_id='1' core_id='0'
siblings='8'/>
<cpu id='9' socket_id='1' core_id='1'
siblings='9'/>
<cpu id='10' socket_id='1' core_id='2'
siblings='10'/>
<cpu id='11' socket_id='1' core_id='3'
siblings='11'/>
</cpus>
</cell>
</cells>
</topology>
</host>
</capabilities>
The socket_id field represents identification of the physical socket the
CPU is plugged in. This ID may not be identical to the physical socket
ID reported by the kernel.
The core_id identifies a core within a socket. Also this field may not
accurately represent physical ID's.
The core_id is guaranteed to be unique within a cell and a socket. There
may be duplicates between sockets. Only cores sharing core_id within one
cell and one socket can be considered as threads. Cores sharing core_id
within sparate cells are distinct cores.
The siblings field is a list of CPU id's the cpu id's the CPU is sibling
with - thus a thread. The list is in the cpuset format.
---
docs/schemas/capability.rng | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index 8c928bc..734ed81 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -193,6 +193,17 @@
<attribute name='id'>
<ref name='unsignedInt'/>
</attribute>
+ <optional>
+ <attribute name='socket_id'>
+ <ref name='unsignedInt'/>
+ </attribute>
+ <attribute name='core_id'>
+ <ref name='unsignedInt'/>
+ </attribute>
+ <attribute name='siblings'>
+ <ref name='cpuset'/>
+ </attribute>
+ </optional>
</element>
</define>
--
1.8.1.1
6:47 p.m.
New subject: [libvirt] [PATCHv2 3/7] schemas: Add schemas for more CPU topology information in the caps XML
On 01/22/2013 02:30 PM, Peter Krempa wrote:
This patch adds RNG schemas for adding more information in the
topology
output of the NUMA section in the capabilities XML.
The added elements are designed to provide more information about the
placement and topology of the processors in the system to management
applications.
The socket_id field represents identification of the physical socket
the
CPU is plugged in. This ID may not be identical to the physical socket
ID reported by the kernel.
The core_id identifies a core within a socket. Also this field may not
accurately represent physical ID's.
The core_id is guaranteed to be unique within a cell and a socket. There
may be duplicates between sockets. Only cores sharing core_id within one
cell and one socket can be considered as threads. Cores sharing core_id
within sparate cells are distinct cores.
The siblings field is a list of CPU id's the cpu id's the CPU is sibling
with - thus a thread. The list is in the cpuset format.
---
docs/schemas/capability.rng | 11 +++++++++++
1 file changed, 11 insertions(+)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:52 p.m.
New subject: [libvirt] [PATCHv2 3/7] schemas: Add schemas for more CPU topology information in the caps XML
On 01/22/2013 02:30 PM, Peter Krempa wrote:
This patch adds RNG schemas for adding more information in the
topology
output of the NUMA section in the capabilities XML.
The added elements are designed to provide more information about the
placement and topology of the processors in the system to management
applications.
The socket_id field represents identification of the physical socket
the
CPU is plugged in. This ID may not be identical to the physical socket
ID reported by the kernel.
The core_id identifies a core within a socket. Also this field may not
accurately represent physical ID's.
The core_id is guaranteed to be unique within a cell and a socket. There
may be duplicates between sockets. Only cores sharing core_id within one
cell and one socket can be considered as threads. Cores sharing core_id
within sparate cells are distinct cores.
The siblings field is a list of CPU id's the cpu id's the CPU is sibling
with - thus a thread. The list is in the cpuset format.
---
docs/schemas/capability.rng | 11 +++++++++++
1 file changed, 11 insertions(+)
The RNG addition looks fine, but a LOT of this commit message should
instead be living somewhere inside docs/formatcaps.html.in file, ideally
as part of this same commit. Hmm, that file is already woefully
incomplete, as it doesn't even mention how we have <cells> as a
subelement of <topology>, so I guess I can live with formal
documentation in a followup patch (since it is a bigger job).
Weak ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:30 p.m.
New subject: [libvirt] [PATCHv2 4/7] conf: Split out NUMA topology formatting to simplify access to data
---
src/conf/capabilities.c | 43 +++++++++++++++++++++++++------------------
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 365c511..0d2512e 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -678,6 +678,28 @@ virCapabilitiesDefaultGuestEmulator(virCapsPtr caps,
return NULL;
}
+static void
+virCapabilitiesFormatNUMATopology(virBufferPtr xml,
+ size_t ncells,
+ virCapsHostNUMACellPtr *cells)
+{
+ int i;
+ int j;
+
+ virBufferAddLit(xml, " <topology>\n");
+ virBufferAsprintf(xml, " <cells num='%zu'>\n", ncells);
+ for (i = 0; i < ncells; i++) {
+ virBufferAsprintf(xml, " <cell id='%d'>\n",
cells[i]->num);
+ virBufferAsprintf(xml, " <cpus num='%d'>\n",
cells[i]->ncpus);
+ for (j = 0; j < cells[i]->ncpus; j++)
+ virBufferAsprintf(xml, " <cpu
id='%d'/>\n",
+ cells[i]->cpus[j]);
+ virBufferAddLit(xml, " </cpus>\n");
+ virBufferAddLit(xml, " </cell>\n");
+ }
+ virBufferAddLit(xml, " </cells>\n");
+ virBufferAddLit(xml, " </topology>\n");
+}
/**
* virCapabilitiesFormatXML:
@@ -752,24 +774,9 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virBufferAddLit(&xml, " </migration_features>\n");
}
- if (caps->host.nnumaCell) {
- virBufferAddLit(&xml, " <topology>\n");
- virBufferAsprintf(&xml, " <cells num='%zu'>\n",
- caps->host.nnumaCell);
- for (i = 0 ; i < caps->host.nnumaCell ; i++) {
- virBufferAsprintf(&xml, " <cell
id='%d'>\n",
- caps->host.numaCell[i]->num);
- virBufferAsprintf(&xml, " <cpus
num='%d'>\n",
- caps->host.numaCell[i]->ncpus);
- for (j = 0 ; j < caps->host.numaCell[i]->ncpus ; j++)
- virBufferAsprintf(&xml, " <cpu
id='%d'/>\n",
- caps->host.numaCell[i]->cpus[j]);
- virBufferAddLit(&xml, " </cpus>\n");
- virBufferAddLit(&xml, " </cell>\n");
- }
- virBufferAddLit(&xml, " </cells>\n");
- virBufferAddLit(&xml, " </topology>\n");
- }
+ if (caps->host.nnumaCell)
+ virCapabilitiesFormatNUMATopology(&xml, caps->host.nnumaCell,
+ caps->host.numaCell);
for (i = 0; i < caps->host.nsecModels; i++) {
virBufferAddLit(&xml, " <secmodel>\n");
--
1.8.1.1
5:09 p.m.
New subject: [libvirt] [PATCHv2 4/7] conf: Split out NUMA topology formatting to simplify access to data
On 01/22/2013 02:30 PM, Peter Krempa wrote:
---
src/conf/capabilities.c | 43 +++++++++++++++++++++++++------------------
1 file changed, 25 insertions(+), 18 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:30 p.m.
New subject: [libvirt] [PATCHv2 5/7] capabilities: Switch CPU data in NUMA topology to a struct
This will allow storing additional topology data in the NUMA topology
definition.
This patch changes the storage type and fixes fallout 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 | 30 +++++++++++++++++++-----------
src/conf/capabilities.h | 14 ++++++++++++--
src/nodeinfo.c | 20 ++++++++++----------
src/qemu/qemu_process.c | 2 +-
src/test/test_driver.c | 15 ++++++++++++---
src/xen/xend_internal.c | 24 +++++++++++++-----------
6 files changed, 67 insertions(+), 38 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 0d2512e..14d3498 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -65,12 +65,27 @@ virCapabilitiesNew(virArch hostarch,
return caps;
}
+void
+virCapabilitiesFreeHostNUMACellCPU(virCapsHostNUMACellCPUPtr cpu)
+{
+ if (!cpu)
+ return;
+
+ virBitmapFree(cpu->siblings);
+ cpu->siblings = NULL;
+}
+
static void
virCapabilitiesFreeHostNUMACell(virCapsHostNUMACellPtr cell)
{
+ size_t i;
+
if (cell == NULL)
return;
+ for (i = 0; i < cell->ncpus; i++)
+ virCapabilitiesFreeHostNUMACellCPU(cell->cpus + i);
+
VIR_FREE(cell->cpus);
VIR_FREE(cell);
}
@@ -236,7 +251,7 @@ virCapabilitiesAddHostMigrateTransport(virCapsPtr caps,
* @caps: capabilities to extend
* @num: ID number of NUMA cell
* @ncpus: number of CPUs in cell
- * @cpus: array of CPU ID numbers for cell
+ * @cpus: array of CPU definition structures, the pointer is stolen
*
* Registers a new NUMA cell for a host, passing in a
* array of CPU IDs belonging to the cell
@@ -245,7 +260,7 @@ int
virCapabilitiesAddHostNUMACell(virCapsPtr caps,
int num,
int ncpus,
- const int *cpus)
+ virCapsHostNUMACellCPUPtr cpus)
{
virCapsHostNUMACellPtr cell;
@@ -256,16 +271,9 @@ virCapabilitiesAddHostNUMACell(virCapsPtr caps,
if (VIR_ALLOC(cell) < 0)
return -1;
- if (VIR_ALLOC_N(cell->cpus, ncpus) < 0) {
- VIR_FREE(cell);
- return -1;
- }
- memcpy(cell->cpus,
- cpus,
- ncpus * sizeof(*cpus));
-
cell->ncpus = ncpus;
cell->num = num;
+ cell->cpus = cpus;
caps->host.numaCell[caps->host.nnumaCell++] = cell;
@@ -693,7 +701,7 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml,
virBufferAsprintf(xml, " <cpus num='%d'>\n",
cells[i]->ncpus);
for (j = 0; j < cells[i]->ncpus; j++)
virBufferAsprintf(xml, " <cpu
id='%d'/>\n",
- cells[i]->cpus[j]);
+ cells[i]->cpus[j].id);
virBufferAddLit(xml, " </cpus>\n");
virBufferAddLit(xml, " </cell>\n");
}
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index 19b99c6..2dd8ee1 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 {
+ unsigned int id;
+ unsigned int socket_id;
+ unsigned int core_id;
+ virBitmapPtr siblings;
+};
+
typedef struct _virCapsHostNUMACell virCapsHostNUMACell;
typedef virCapsHostNUMACell *virCapsHostNUMACellPtr;
struct _virCapsHostNUMACell {
int num;
int ncpus;
- int *cpus;
+ virCapsHostNUMACellCPUPtr cpus;
};
typedef struct _virCapsHostSecModel virCapsHostSecModel;
@@ -201,7 +210,7 @@ extern int
virCapabilitiesAddHostNUMACell(virCapsPtr caps,
int num,
int ncpus,
- const int *cpus);
+ virCapsHostNUMACellCPUPtr cpus);
extern int
@@ -250,6 +259,7 @@ virCapabilitiesSupportsGuestOSTypeArch(virCapsPtr caps,
const char *ostype,
virArch arch);
+void virCapabilitiesFreeHostNUMACellCPU(virCapsHostNUMACellCPUPtr cpu);
extern virArch
virCapabilitiesDefaultGuestArch(virCapsPtr caps,
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index a05159c..5febcfb 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1479,9 +1479,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;
@@ -1495,7 +1496,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",
@@ -1518,21 +1518,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;
}
ret = 0;
cleanup:
- VIR_FREE(cpus);
+ if (cpus) {
+ while (ncpus--)
+ virCapabilitiesFreeHostNUMACellCPU(cpus + ncpus);
+ VIR_FREE(cpus);
+ }
VIR_FREE(mask);
VIR_FREE(allonesmask);
return ret;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2f08215..509cc39 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2017,7 +2017,7 @@ qemuPrepareCpumap(virQEMUDriverPtr driver,
if (result) {
for (j = 0; j < cur_ncpus; j++)
ignore_value(virBitmapSetBit(cpumap,
-
driver->caps->host.numaCell[i]->cpus[j]));
+
driver->caps->host.numaCell[i]->cpus[j].id));
}
}
}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index a10a377..6909fa4 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -69,7 +69,7 @@ typedef struct _testDomainObjPrivate *testDomainObjPrivatePtr;
struct _testCell {
unsigned long mem;
int numCpus;
- int cpus[MAX_CPUS];
+ virCapsHostNUMACellCPU cpus[MAX_CPUS];
};
typedef struct _testCell testCell;
typedef struct _testCell *testCellPtr;
@@ -174,8 +174,17 @@ testBuildCapabilities(virConnectPtr conn) {
goto no_memory;
for (i = 0; i < privconn->numCells; i++) {
+ virCapsHostNUMACellCPUPtr cpu_cells;
+
+ if (VIR_ALLOC_N(cpu_cells, privconn->cells[i].numCpus) < 0)
+ goto no_memory;
+
+ memcpy(cpu_cells, privconn->cells[i].cpus,
+ sizeof(*cpu_cells) * privconn->cells[i].numCpus);
+
+
if (virCapabilitiesAddHostNUMACell(caps, i, privconn->cells[i].numCpus,
- privconn->cells[i].cpus) < 0)
+ cpu_cells) < 0)
goto no_memory;
}
@@ -549,7 +558,7 @@ static int testOpenDefault(virConnectPtr conn) {
privconn->cells[u].mem = (u + 1) * 2048 * 1024;
}
for (u = 0 ; u < 16 ; u++) {
- privconn->cells[u % 2].cpus[(u / 2)] = u;
+ privconn->cells[u % 2].cpus[(u / 2)].id = u;
}
if (!(privconn->caps = testBuildCapabilities(conn)))
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 00d1c9b..57d8325 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1112,7 +1112,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
{
const char *nodeToCpu;
const char *cur;
- int *cpuNums = NULL;
+ virCapsHostNUMACellCPUPtr cpuInfo = NULL;
int cell, cpu, nb_cpus;
int n = 0;
int numCpus;
@@ -1124,9 +1124,6 @@ sexpr_to_xend_topology(const struct sexpr *root,
numCpus = sexpr_int(root, "node/nr_cpus");
- if (VIR_ALLOC_N(cpuNums, numCpus) < 0)
- goto memory_error;
-
cur = nodeToCpu;
while (*cur != 0) {
virBitmapPtr cpuset = NULL;
@@ -1155,31 +1152,36 @@ sexpr_to_xend_topology(const struct sexpr *root,
goto error;
}
+ if (VIR_ALLOC_N(cpuInfo, numCpus) < 0)
+ goto memory_error;
+
for (n = 0, cpu = 0; cpu < numCpus; cpu++) {
bool used;
ignore_value(virBitmapGetBit(cpuset, cpu, &used));
- if (used)
- cpuNums[n++] = cpu;
+ if (used) {
+ cpuInfo[n].id = cpu;
+ n++;
+ }
}
virBitmapFree(cpuset);
- if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, cpuNums) < 0)
+ if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, cpuInfo) < 0)
goto memory_error;
+ cpuInfo = NULL;
}
- VIR_FREE(cpuNums);
+
return 0;
parse_error:
virReportError(VIR_ERR_XEN_CALL, "%s", _("topology syntax
error"));
error:
- VIR_FREE(cpuNums);
+ VIR_FREE(cpuInfo);
return -1;
memory_error:
- VIR_FREE(cpuNums);
virReportOOMError();
- return -1;
+ goto error;
}
--
1.8.1.1
3:30 p.m.
New subject: [libvirt] [PATCHv2 6/7] capabilities: Add additional data to the NUMA topology info
This patch adds data gathering to the NUMA gathering files and adds
support for outputting the data. The test driver and xend driver need to
be adapted to fill sensible data to the structure.
---
src/conf/capabilities.c | 31 ++++++++++++++++++++++-----
src/nodeinfo.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++---
src/test/test_driver.c | 3 +++
src/xen/xend_internal.c | 1 +
4 files changed, 84 insertions(+), 8 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 14d3498..c9036f4 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -686,27 +686,47 @@ virCapabilitiesDefaultGuestEmulator(virCapsPtr caps,
return NULL;
}
-static void
+static int
virCapabilitiesFormatNUMATopology(virBufferPtr xml,
size_t ncells,
virCapsHostNUMACellPtr *cells)
{
int i;
int j;
+ char *siblings;
virBufferAddLit(xml, " <topology>\n");
virBufferAsprintf(xml, " <cells num='%zu'>\n", ncells);
for (i = 0; i < ncells; i++) {
virBufferAsprintf(xml, " <cell id='%d'>\n",
cells[i]->num);
virBufferAsprintf(xml, " <cpus num='%d'>\n",
cells[i]->ncpus);
- for (j = 0; j < cells[i]->ncpus; j++)
- virBufferAsprintf(xml, " <cpu
id='%d'/>\n",
+ for (j = 0; j < cells[i]->ncpus; j++) {
+ virBufferAsprintf(xml, " <cpu id='%d'",
cells[i]->cpus[j].id);
+
+ if (cells[i]->cpus[j].siblings) {
+ if (!(siblings = virBitmapFormat(cells[i]->cpus[j].siblings))) {
+ virReportOOMError();
+ return -1;
+ }
+
+ virBufferAsprintf(xml,
+ " socket_id='%d' core_id='%d'
siblings='%s'",
+ cells[i]->cpus[j].socket_id,
+ cells[i]->cpus[j].core_id,
+ siblings);
+ VIR_FREE(siblings);
+ }
+ virBufferAddLit(xml, "/>\n");
+ }
+
virBufferAddLit(xml, " </cpus>\n");
virBufferAddLit(xml, " </cell>\n");
}
virBufferAddLit(xml, " </cells>\n");
virBufferAddLit(xml, " </topology>\n");
+
+ return 0;
}
/**
@@ -782,9 +802,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virBufferAddLit(&xml, " </migration_features>\n");
}
- if (caps->host.nnumaCell)
+ if (caps->host.nnumaCell &&
virCapabilitiesFormatNUMATopology(&xml, caps->host.nnumaCell,
- caps->host.numaCell);
+ caps->host.numaCell) < 0)
+ return NULL;
for (i = 0; i < caps->host.nsecModels; i++) {
virBufferAddLit(&xml, " <secmodel>\n");
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 5febcfb..dc6771a 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"
# 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
@@ -1473,6 +1475,52 @@ cleanup:
# define MASK_CPU_ISSET(mask, cpu) \
(((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask)))) & 1)
+static virBitmapPtr
+virNodeGetSiblingsList(const char *dir, int cpu_id)
+{
+ char *path = NULL;
+ char *buf = NULL;
+ virBitmapPtr ret = NULL;
+
+ if (virAsprintf(&path, "%s/cpu%u/topology/thread_siblings_list",
+ dir, cpu_id) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (virFileReadAll(path, SYSFS_THREAD_SIBLINGS_LIST_LENGHT_MAX, &buf) < 0)
+ goto cleanup;
+
+ if (virBitmapParse(buf, ',', &ret, NUMA_MAX_N_CPUS) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to parse thread siblings"));
+ goto cleanup;
+ }
+
+cleanup:
+ VIR_FREE(buf);
+ VIR_FREE(path);
+ return ret;
+}
+
+static int
+virNodeCapsFillCPUInfo(int cpu_id, virCapsHostNUMACellCPUPtr cpu)
+{
+ cpu->id = cpu_id;
+ cpu->socket_id = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id,
+ "topology/physical_package_id", -1);
+ cpu->core_id = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id,
+ "topology/core_id", -1);
+
+ if (cpu->socket_id == -1 || cpu->core_id == -1)
+ return 0;
+
+ if (!(cpu->siblings = virNodeGetSiblingsList(SYSFS_CPU_PATH, cpu_id)))
+ return -1;
+
+ return 0;
+}
+
int
nodeCapsInitNUMA(virCapsPtr caps)
{
@@ -1516,9 +1564,12 @@ nodeCapsInitNUMA(virCapsPtr caps)
if (VIR_ALLOC_N(cpus, ncpus) < 0)
goto cleanup;
- for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++)
- if (MASK_CPU_ISSET(mask, i))
- cpus[ncpus++].id = i;
+ for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++) {
+ if (MASK_CPU_ISSET(mask, i)) {
+ if (virNodeCapsFillCPUInfo(i, cpus + ncpus++) < 0)
+ goto cleanup;
+ }
+ }
if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, cpus) < 0)
goto cleanup;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 6909fa4..038f627 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -559,6 +559,9 @@ static int testOpenDefault(virConnectPtr conn) {
}
for (u = 0 ; u < 16 ; u++) {
privconn->cells[u % 2].cpus[(u / 2)].id = u;
+ privconn->cells[u % 2].cpus[(u / 2)].socket_id = -1;
+ privconn->cells[u % 2].cpus[(u / 2)].core_id = -1;
+ privconn->cells[u % 2].cpus[(u / 2)].siblings = NULL;
}
if (!(privconn->caps = testBuildCapabilities(conn)))
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 57d8325..434f558 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1161,6 +1161,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
ignore_value(virBitmapGetBit(cpuset, cpu, &used));
if (used) {
cpuInfo[n].id = cpu;
+ cpuInfo[n].siblings = NULL;
n++;
}
}
--
1.8.1.1
4:04 a.m.
New subject: [libvirt] [PATCHv2 6/7] capabilities: Add additional data to the NUMA topology info
On Tue, Jan 22, 2013 at 10:30:25PM +0100, Peter Krempa wrote:
This patch adds data gathering to the NUMA gathering files and adds
support for outputting the data. The test driver and xend driver need to
be adapted to fill sensible data to the structure.
---
src/conf/capabilities.c | 31 ++++++++++++++++++++++-----
src/nodeinfo.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++---
src/test/test_driver.c | 3 +++
src/xen/xend_internal.c | 1 +
4 files changed, 84 insertions(+), 8 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 14d3498..c9036f4 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -686,27 +686,47 @@ virCapabilitiesDefaultGuestEmulator(virCapsPtr caps,
return NULL;
}
-static void
+static int
virCapabilitiesFormatNUMATopology(virBufferPtr xml,
size_t ncells,
virCapsHostNUMACellPtr *cells)
{
int i;
int j;
+ char *siblings;
virBufferAddLit(xml, " <topology>\n");
virBufferAsprintf(xml, " <cells num='%zu'>\n",
ncells);
for (i = 0; i < ncells; i++) {
virBufferAsprintf(xml, " <cell id='%d'>\n",
cells[i]->num);
virBufferAsprintf(xml, " <cpus num='%d'>\n",
cells[i]->ncpus);
- for (j = 0; j < cells[i]->ncpus; j++)
- virBufferAsprintf(xml, " <cpu
id='%d'/>\n",
+ for (j = 0; j < cells[i]->ncpus; j++) {
+ virBufferAsprintf(xml, " <cpu id='%d'",
cells[i]->cpus[j].id);
+
+ if (cells[i]->cpus[j].siblings) {
+ if (!(siblings = virBitmapFormat(cells[i]->cpus[j].siblings))) {
+ virReportOOMError();
+ return -1;
+ }
+
+ virBufferAsprintf(xml,
+ " socket_id='%d' core_id='%d'
siblings='%s'",
+ cells[i]->cpus[j].socket_id,
+ cells[i]->cpus[j].core_id,
+ siblings);
+ VIR_FREE(siblings);
+ }
+ virBufferAddLit(xml, "/>\n");
+ }
+
virBufferAddLit(xml, " </cpus>\n");
virBufferAddLit(xml, " </cell>\n");
}
virBufferAddLit(xml, " </cells>\n");
virBufferAddLit(xml, " </topology>\n");
+
+ return 0;
}
/**
@@ -782,9 +802,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virBufferAddLit(&xml, " </migration_features>\n");
}
- if (caps->host.nnumaCell)
+ if (caps->host.nnumaCell &&
virCapabilitiesFormatNUMATopology(&xml, caps->host.nnumaCell,
- caps->host.numaCell);
+ caps->host.numaCell) < 0)
+ return NULL;
for (i = 0; i < caps->host.nsecModels; i++) {
virBufferAddLit(&xml, " <secmodel>\n");
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 5febcfb..dc6771a 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"
# 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
@@ -1473,6 +1475,52 @@ cleanup:
# define MASK_CPU_ISSET(mask, cpu) \
(((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask)))) & 1)
+static virBitmapPtr
+virNodeGetSiblingsList(const char *dir, int cpu_id)
+{
+ char *path = NULL;
+ char *buf = NULL;
+ virBitmapPtr ret = NULL;
+
+ if (virAsprintf(&path, "%s/cpu%u/topology/thread_siblings_list",
+ dir, cpu_id) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (virFileReadAll(path, SYSFS_THREAD_SIBLINGS_LIST_LENGHT_MAX, &buf) < 0)
+ goto cleanup;
+
+ if (virBitmapParse(buf, ',', &ret, NUMA_MAX_N_CPUS) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to parse thread siblings"));
+ goto cleanup;
+ }
+
+cleanup:
+ VIR_FREE(buf);
+ VIR_FREE(path);
+ return ret;
+}
+
+static int
+virNodeCapsFillCPUInfo(int cpu_id, virCapsHostNUMACellCPUPtr cpu)
+{
+ cpu->id = cpu_id;
+ cpu->socket_id = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id,
+ "topology/physical_package_id", -1);
+ cpu->core_id = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id,
+ "topology/core_id", -1);
+
+ if (cpu->socket_id == -1 || cpu->core_id == -1)
+ return 0;
+
+ if (!(cpu->siblings = virNodeGetSiblingsList(SYSFS_CPU_PATH, cpu_id)))
+ return -1;
+
+ return 0;
+}
+
int
nodeCapsInitNUMA(virCapsPtr caps)
{
@@ -1516,9 +1564,12 @@ nodeCapsInitNUMA(virCapsPtr caps)
if (VIR_ALLOC_N(cpus, ncpus) < 0)
goto cleanup;
- for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++)
- if (MASK_CPU_ISSET(mask, i))
- cpus[ncpus++].id = i;
+ for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++) {
+ if (MASK_CPU_ISSET(mask, i)) {
+ if (virNodeCapsFillCPUInfo(i, cpus + ncpus++) < 0)
+ goto cleanup;
+ }
+ }
if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, cpus) < 0)
goto cleanup;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 6909fa4..038f627 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -559,6 +559,9 @@ static int testOpenDefault(virConnectPtr conn) {
}
for (u = 0 ; u < 16 ; u++) {
privconn->cells[u % 2].cpus[(u / 2)].id = u;
+ privconn->cells[u % 2].cpus[(u / 2)].socket_id = -1;
+ privconn->cells[u % 2].cpus[(u / 2)].core_id = -1;
+ privconn->cells[u % 2].cpus[(u / 2)].siblings = NULL;
}
This is wrong because these fields are unsigned int.
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 57d8325..434f558 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1161,6 +1161,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
ignore_value(virBitmapGetBit(cpuset, cpu, &used));
if (used) {
cpuInfo[n].id = cpu;
+ cpuInfo[n].siblings = NULL;
As mentioned before, this should be initializing based on the nodeinfo.
Here you've allowed socket_id + core_id to all initialize to 0 which
is wrong.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:12 a.m.
New subject: [libvirt] [PATCHv2 6/7] capabilities: Add additional data to the NUMA topology info
On 01/23/13 11:04, Daniel P. Berrange wrote:
On Tue, Jan 22, 2013 at 10:30:25PM +0100, Peter Krempa wrote:
> This patch adds data gathering to the NUMA gathering files and adds
> support for outputting the data. The test driver and xend driver need to
> be adapted to fill sensible data to the structure.
> ---
> src/conf/capabilities.c | 31 ++++++++++++++++++++++-----
> src/nodeinfo.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++---
> src/test/test_driver.c | 3 +++
> src/xen/xend_internal.c | 1 +
> 4 files changed, 84 insertions(+), 8 deletions(-)
>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 14d3498..c9036f4 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -686,27 +686,47 @@ virCapabilitiesDefaultGuestEmulator(virCapsPtr caps,
> return NULL;
> }
>
> -static void
> +static int
> virCapabilitiesFormatNUMATopology(virBufferPtr xml,
> size_t ncells,
> virCapsHostNUMACellPtr *cells)
> {
> int i;
> int j;
> + char *siblings;
>
> virBufferAddLit(xml, " <topology>\n");
> virBufferAsprintf(xml, " <cells num='%zu'>\n",
ncells);
> for (i = 0; i < ncells; i++) {
> virBufferAsprintf(xml, " <cell id='%d'>\n",
cells[i]->num);
> virBufferAsprintf(xml, " <cpus
num='%d'>\n", cells[i]->ncpus);
> - for (j = 0; j < cells[i]->ncpus; j++)
> - virBufferAsprintf(xml, " <cpu
id='%d'/>\n",
> + for (j = 0; j < cells[i]->ncpus; j++) {
> + virBufferAsprintf(xml, " <cpu id='%d'",
> cells[i]->cpus[j].id);
> +
> + if (cells[i]->cpus[j].siblings) {
> + if (!(siblings = virBitmapFormat(cells[i]->cpus[j].siblings))) {
> + virReportOOMError();
> + return -1;
> + }
> +
> + virBufferAsprintf(xml,
> + " socket_id='%d' core_id='%d'
siblings='%s'",
> + cells[i]->cpus[j].socket_id,
> + cells[i]->cpus[j].core_id,
> + siblings);
> + VIR_FREE(siblings);
> + }
> + virBufferAddLit(xml, "/>\n");
> + }
> +
> virBufferAddLit(xml, " </cpus>\n");
> virBufferAddLit(xml, " </cell>\n");
> }
> virBufferAddLit(xml, " </cells>\n");
> virBufferAddLit(xml, " </topology>\n");
> +
> + return 0;
> }
>
> /**
> @@ -782,9 +802,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> virBufferAddLit(&xml, " </migration_features>\n");
> }
>
> - if (caps->host.nnumaCell)
> + if (caps->host.nnumaCell &&
> virCapabilitiesFormatNUMATopology(&xml, caps->host.nnumaCell,
> - caps->host.numaCell);
> + caps->host.numaCell) < 0)
> + return NULL;
>
> for (i = 0; i < caps->host.nsecModels; i++) {
> virBufferAddLit(&xml, " <secmodel>\n");
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index 5febcfb..dc6771a 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"
> # 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
> @@ -1473,6 +1475,52 @@ cleanup:
> # define MASK_CPU_ISSET(mask, cpu) \
> (((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask)))) & 1)
>
> +static virBitmapPtr
> +virNodeGetSiblingsList(const char *dir, int cpu_id)
> +{
> + char *path = NULL;
> + char *buf = NULL;
> + virBitmapPtr ret = NULL;
> +
> + if (virAsprintf(&path, "%s/cpu%u/topology/thread_siblings_list",
> + dir, cpu_id) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + if (virFileReadAll(path, SYSFS_THREAD_SIBLINGS_LIST_LENGHT_MAX, &buf) <
0)
> + goto cleanup;
> +
> + if (virBitmapParse(buf, ',', &ret, NUMA_MAX_N_CPUS) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Failed to parse thread siblings"));
> + goto cleanup;
> + }
> +
> +cleanup:
> + VIR_FREE(buf);
> + VIR_FREE(path);
> + return ret;
> +}
> +
> +static int
> +virNodeCapsFillCPUInfo(int cpu_id, virCapsHostNUMACellCPUPtr cpu)
> +{
> + cpu->id = cpu_id;
> + cpu->socket_id = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id,
> + "topology/physical_package_id",
-1);
> + cpu->core_id = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id,
> + "topology/core_id", -1);
> +
> + if (cpu->socket_id == -1 || cpu->core_id == -1)
> + return 0;
> +
> + if (!(cpu->siblings = virNodeGetSiblingsList(SYSFS_CPU_PATH, cpu_id)))
> + return -1;
> +
> + return 0;
> +}
> +
> int
> nodeCapsInitNUMA(virCapsPtr caps)
> {
> @@ -1516,9 +1564,12 @@ nodeCapsInitNUMA(virCapsPtr caps)
> if (VIR_ALLOC_N(cpus, ncpus) < 0)
> goto cleanup;
>
> - for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++)
> - if (MASK_CPU_ISSET(mask, i))
> - cpus[ncpus++].id = i;
> + for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++) {
> + if (MASK_CPU_ISSET(mask, i)) {
> + if (virNodeCapsFillCPUInfo(i, cpus + ncpus++) < 0)
> + goto cleanup;
> + }
> + }
>
> if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, cpus) < 0)
> goto cleanup;
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 6909fa4..038f627 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -559,6 +559,9 @@ static int testOpenDefault(virConnectPtr conn) {
> }
> for (u = 0 ; u < 16 ; u++) {
> privconn->cells[u % 2].cpus[(u / 2)].id = u;
> + privconn->cells[u % 2].cpus[(u / 2)].socket_id = -1;
> + privconn->cells[u % 2].cpus[(u / 2)].core_id = -1;
> + privconn->cells[u % 2].cpus[(u / 2)].siblings = NULL;
> }
This is wrong because these fields are unsigned int.
Hm, yeah, I forgot to update this. Anyways, the data doesn't pose
problem here as the output isn't enhanced until siblings is non-NULL.
This code gets fixed in 7/7.
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index 57d8325..434f558 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -1161,6 +1161,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
> ignore_value(virBitmapGetBit(cpuset, cpu, &used));
> if (used) {
> cpuInfo[n].id = cpu;
> + cpuInfo[n].siblings = NULL;
As mentioned before, this should be initializing based on the nodeinfo.
Here you've allowed socket_id + core_id to all initialize to 0 which
is wrong.
Also here, the siblings are NULL so the new output isn't used at all. I
added the condition so that the new code could be avoided until I
prepare means to test the XEN support.
Daniel
4:29 a.m.
New subject: [libvirt] [PATCHv2 6/7] capabilities: Add additional data to the NUMA topology info
On Wed, Jan 23, 2013 at 11:12:35AM +0100, Peter Krempa wrote:
On 01/23/13 11:04, Daniel P. Berrange wrote:
>>diff --git a/src/test/test_driver.c b/src/test/test_driver.c
>>index 6909fa4..038f627 100644
>>--- a/src/test/test_driver.c
>>+++ b/src/test/test_driver.c
>>@@ -559,6 +559,9 @@ static int testOpenDefault(virConnectPtr conn) {
>> }
>> for (u = 0 ; u < 16 ; u++) {
>> privconn->cells[u % 2].cpus[(u / 2)].id = u;
>>+ privconn->cells[u % 2].cpus[(u / 2)].socket_id = -1;
>>+ privconn->cells[u % 2].cpus[(u / 2)].core_id = -1;
>>+ privconn->cells[u % 2].cpus[(u / 2)].siblings = NULL;
>> }
>
>This is wrong because these fields are unsigned int.
Hm, yeah, I forgot to update this. Anyways, the data doesn't pose
problem here as the output isn't enhanced until siblings is
non-NULL.
This code gets fixed in 7/7.
In that case you can delete this entire chunk - VIR_ALLOC
ensures everything is initialized to 0, so the "= NULL" is
not doing anything
>
>>diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
>>index 57d8325..434f558 100644
>>--- a/src/xen/xend_internal.c
>>+++ b/src/xen/xend_internal.c
>>@@ -1161,6 +1161,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
>> ignore_value(virBitmapGetBit(cpuset, cpu, &used));
>> if (used) {
>> cpuInfo[n].id = cpu;
>>+ cpuInfo[n].siblings = NULL;
>
>As mentioned before, this should be initializing based on the nodeinfo.
>Here you've allowed socket_id + core_id to all initialize to 0 which
>is wrong.
Also here, the siblings are NULL so the new output isn't used at
all. I added the condition so that the new code could be avoided
until I prepare means to test the XEN support.
Again this initialization to NULL is redundant
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:42 a.m.
New subject: [libvirt] [PATCHv3] capabilities: Switch CPU data in NUMA topology to a struct
This will allow storing additional topology data in the NUMA topology
definition.
This patch changes the storage type and fixes fallout 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.
---
Removed initialization of unneeded data.
src/conf/capabilities.c | 30 +++++++++++++++++++-----------
src/conf/capabilities.h | 14 ++++++++++++--
src/nodeinfo.c | 20 ++++++++++----------
src/qemu/qemu_process.c | 2 +-
src/test/test_driver.c | 15 ++++++++++++---
src/xen/xend_internal.c | 20 ++++++++++----------
6 files changed, 64 insertions(+), 37 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 0d2512e..14d3498 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -65,12 +65,27 @@ virCapabilitiesNew(virArch hostarch,
return caps;
}
+void
+virCapabilitiesFreeHostNUMACellCPU(virCapsHostNUMACellCPUPtr cpu)
+{
+ if (!cpu)
+ return;
+
+ virBitmapFree(cpu->siblings);
+ cpu->siblings = NULL;
+}
+
static void
virCapabilitiesFreeHostNUMACell(virCapsHostNUMACellPtr cell)
{
+ size_t i;
+
if (cell == NULL)
return;
+ for (i = 0; i < cell->ncpus; i++)
+ virCapabilitiesFreeHostNUMACellCPU(cell->cpus + i);
+
VIR_FREE(cell->cpus);
VIR_FREE(cell);
}
@@ -236,7 +251,7 @@ virCapabilitiesAddHostMigrateTransport(virCapsPtr caps,
* @caps: capabilities to extend
* @num: ID number of NUMA cell
* @ncpus: number of CPUs in cell
- * @cpus: array of CPU ID numbers for cell
+ * @cpus: array of CPU definition structures, the pointer is stolen
*
* Registers a new NUMA cell for a host, passing in a
* array of CPU IDs belonging to the cell
@@ -245,7 +260,7 @@ int
virCapabilitiesAddHostNUMACell(virCapsPtr caps,
int num,
int ncpus,
- const int *cpus)
+ virCapsHostNUMACellCPUPtr cpus)
{
virCapsHostNUMACellPtr cell;
@@ -256,16 +271,9 @@ virCapabilitiesAddHostNUMACell(virCapsPtr caps,
if (VIR_ALLOC(cell) < 0)
return -1;
- if (VIR_ALLOC_N(cell->cpus, ncpus) < 0) {
- VIR_FREE(cell);
- return -1;
- }
- memcpy(cell->cpus,
- cpus,
- ncpus * sizeof(*cpus));
-
cell->ncpus = ncpus;
cell->num = num;
+ cell->cpus = cpus;
caps->host.numaCell[caps->host.nnumaCell++] = cell;
@@ -693,7 +701,7 @@ virCapabilitiesFormatNUMATopology(virBufferPtr xml,
virBufferAsprintf(xml, " <cpus num='%d'>\n",
cells[i]->ncpus);
for (j = 0; j < cells[i]->ncpus; j++)
virBufferAsprintf(xml, " <cpu
id='%d'/>\n",
- cells[i]->cpus[j]);
+ cells[i]->cpus[j].id);
virBufferAddLit(xml, " </cpus>\n");
virBufferAddLit(xml, " </cell>\n");
}
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index 19b99c6..2dd8ee1 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 {
+ unsigned int id;
+ unsigned int socket_id;
+ unsigned int core_id;
+ virBitmapPtr siblings;
+};
+
typedef struct _virCapsHostNUMACell virCapsHostNUMACell;
typedef virCapsHostNUMACell *virCapsHostNUMACellPtr;
struct _virCapsHostNUMACell {
int num;
int ncpus;
- int *cpus;
+ virCapsHostNUMACellCPUPtr cpus;
};
typedef struct _virCapsHostSecModel virCapsHostSecModel;
@@ -201,7 +210,7 @@ extern int
virCapabilitiesAddHostNUMACell(virCapsPtr caps,
int num,
int ncpus,
- const int *cpus);
+ virCapsHostNUMACellCPUPtr cpus);
extern int
@@ -250,6 +259,7 @@ virCapabilitiesSupportsGuestOSTypeArch(virCapsPtr caps,
const char *ostype,
virArch arch);
+void virCapabilitiesFreeHostNUMACellCPU(virCapsHostNUMACellCPUPtr cpu);
extern virArch
virCapabilitiesDefaultGuestArch(virCapsPtr caps,
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index a05159c..5febcfb 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1479,9 +1479,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;
@@ -1495,7 +1496,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",
@@ -1518,21 +1518,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;
}
ret = 0;
cleanup:
- VIR_FREE(cpus);
+ if (cpus) {
+ while (ncpus--)
+ virCapabilitiesFreeHostNUMACellCPU(cpus + ncpus);
+ VIR_FREE(cpus);
+ }
VIR_FREE(mask);
VIR_FREE(allonesmask);
return ret;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2f08215..509cc39 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2017,7 +2017,7 @@ qemuPrepareCpumap(virQEMUDriverPtr driver,
if (result) {
for (j = 0; j < cur_ncpus; j++)
ignore_value(virBitmapSetBit(cpumap,
-
driver->caps->host.numaCell[i]->cpus[j]));
+
driver->caps->host.numaCell[i]->cpus[j].id));
}
}
}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index a10a377..6909fa4 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -69,7 +69,7 @@ typedef struct _testDomainObjPrivate *testDomainObjPrivatePtr;
struct _testCell {
unsigned long mem;
int numCpus;
- int cpus[MAX_CPUS];
+ virCapsHostNUMACellCPU cpus[MAX_CPUS];
};
typedef struct _testCell testCell;
typedef struct _testCell *testCellPtr;
@@ -174,8 +174,17 @@ testBuildCapabilities(virConnectPtr conn) {
goto no_memory;
for (i = 0; i < privconn->numCells; i++) {
+ virCapsHostNUMACellCPUPtr cpu_cells;
+
+ if (VIR_ALLOC_N(cpu_cells, privconn->cells[i].numCpus) < 0)
+ goto no_memory;
+
+ memcpy(cpu_cells, privconn->cells[i].cpus,
+ sizeof(*cpu_cells) * privconn->cells[i].numCpus);
+
+
if (virCapabilitiesAddHostNUMACell(caps, i, privconn->cells[i].numCpus,
- privconn->cells[i].cpus) < 0)
+ cpu_cells) < 0)
goto no_memory;
}
@@ -549,7 +558,7 @@ static int testOpenDefault(virConnectPtr conn) {
privconn->cells[u].mem = (u + 1) * 2048 * 1024;
}
for (u = 0 ; u < 16 ; u++) {
- privconn->cells[u % 2].cpus[(u / 2)] = u;
+ privconn->cells[u % 2].cpus[(u / 2)].id = u;
}
if (!(privconn->caps = testBuildCapabilities(conn)))
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 00d1c9b..9330617 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1112,7 +1112,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
{
const char *nodeToCpu;
const char *cur;
- int *cpuNums = NULL;
+ virCapsHostNUMACellCPUPtr cpuInfo = NULL;
int cell, cpu, nb_cpus;
int n = 0;
int numCpus;
@@ -1124,9 +1124,6 @@ sexpr_to_xend_topology(const struct sexpr *root,
numCpus = sexpr_int(root, "node/nr_cpus");
- if (VIR_ALLOC_N(cpuNums, numCpus) < 0)
- goto memory_error;
-
cur = nodeToCpu;
while (*cur != 0) {
virBitmapPtr cpuset = NULL;
@@ -1155,31 +1152,34 @@ sexpr_to_xend_topology(const struct sexpr *root,
goto error;
}
+ if (VIR_ALLOC_N(cpuInfo, numCpus) < 0)
+ goto memory_error;
+
for (n = 0, cpu = 0; cpu < numCpus; cpu++) {
bool used;
ignore_value(virBitmapGetBit(cpuset, cpu, &used));
if (used)
- cpuNums[n++] = cpu;
+ cpuInfo[n++].id = cpu;
}
virBitmapFree(cpuset);
- if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, cpuNums) < 0)
+ if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, cpuInfo) < 0)
goto memory_error;
+ cpuInfo = NULL;
}
- VIR_FREE(cpuNums);
+
return 0;
parse_error:
virReportError(VIR_ERR_XEN_CALL, "%s", _("topology syntax
error"));
error:
- VIR_FREE(cpuNums);
+ VIR_FREE(cpuInfo);
return -1;
memory_error:
- VIR_FREE(cpuNums);
virReportOOMError();
- return -1;
+ goto error;
}
--
1.8.1.1
3:30 p.m.
New subject: [libvirt] [PATCHv2 7/7] test: Add support for thread and core information for the test driver
This patch adds demo processor topology information for the test driver.
---
src/test/test_driver.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 038f627..ddc4110 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -558,10 +558,16 @@ static int testOpenDefault(virConnectPtr conn) {
privconn->cells[u].mem = (u + 1) * 2048 * 1024;
}
for (u = 0 ; u < 16 ; u++) {
- privconn->cells[u % 2].cpus[(u / 2)].id = u;
- privconn->cells[u % 2].cpus[(u / 2)].socket_id = -1;
- privconn->cells[u % 2].cpus[(u / 2)].core_id = -1;
- privconn->cells[u % 2].cpus[(u / 2)].siblings = NULL;
+ virBitmapPtr siblings = virBitmapNew(16);
+ if (!siblings) {
+ virReportOOMError();
+ goto error;
+ }
+ ignore_value(virBitmapSetBit(siblings, u));
+ privconn->cells[u / 8].cpus[(u % 8)].id = u;
+ privconn->cells[u / 8].cpus[(u % 8)].socket_id = u / 8;
+ privconn->cells[u / 8].cpus[(u % 8)].core_id = u % 8;
+ privconn->cells[u / 8].cpus[(u % 8)].siblings = siblings;
}
if (!(privconn->caps = testBuildCapabilities(conn)))
--
1.8.1.1
4319
days inactive
4325
days old
20 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Martin Kletzander -
Peter Krempa