Re: [libvirt] [PATCH v3 04/16] conf, schema: add 'id' field for cells

2014-07-16 Thread Martin Kletzander

On Wed, Jul 16, 2014 at 12:14:35PM -0600, Eric Blake wrote:

On 07/16/2014 08:42 AM, Martin Kletzander wrote:

In XML format, by definition, order of fields should not matter, so
order of parsing the elements doesn't affect the end result.  When
specifying guest NUMA cells, we depend only on the order of the 'cell'
elements.  With this patch all older domain XMLs are parsed as before,
but with the 'id' attribute they are parsed and formatted according to
that field.  This will be useful when we have tuning settings for
particular guest NUMA node.


It's not always true that order doesn't matter - for the longest time,
the order of  under  determined boot order.  But I agree
that the flexibility of using an id to allow arbitrary ordering can be
nicer than enforcing constant ordering.



I was under the impression that it's determined by the buses the disks
are connected to.  Anyway even if that matters, I'm not sure we
guarantee that and if we do, it should be fixed from my POV.  But
that's a discussion for another day.



Signed-off-by: Martin Kletzander 
---
 docs/formatdomain.html.in  | 17 +
 docs/schemas/domaincommon.rng  |  5 +++


Looks okay to me on the schema side.



Thanks for checking it.


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 04/16] conf, schema: add 'id' field for cells

2014-07-16 Thread Eric Blake
On 07/16/2014 08:42 AM, Martin Kletzander wrote:
> In XML format, by definition, order of fields should not matter, so
> order of parsing the elements doesn't affect the end result.  When
> specifying guest NUMA cells, we depend only on the order of the 'cell'
> elements.  With this patch all older domain XMLs are parsed as before,
> but with the 'id' attribute they are parsed and formatted according to
> that field.  This will be useful when we have tuning settings for
> particular guest NUMA node.

It's not always true that order doesn't matter - for the longest time,
the order of  under  determined boot order.  But I agree
that the flexibility of using an id to allow arbitrary ordering can be
nicer than enforcing constant ordering.

> 
> Signed-off-by: Martin Kletzander 
> ---
>  docs/formatdomain.html.in  | 17 +
>  docs/schemas/domaincommon.rng  |  5 +++

Looks okay to me on the schema side.


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 04/16] conf, schema: add 'id' field for cells

2014-07-16 Thread Martin Kletzander
In XML format, by definition, order of fields should not matter, so
order of parsing the elements doesn't affect the end result.  When
specifying guest NUMA cells, we depend only on the order of the 'cell'
elements.  With this patch all older domain XMLs are parsed as before,
but with the 'id' attribute they are parsed and formatted according to
that field.  This will be useful when we have tuning settings for
particular guest NUMA node.

Signed-off-by: Martin Kletzander 
---
 docs/formatdomain.html.in  | 17 +
 docs/schemas/domaincommon.rng  |  5 +++
 src/conf/cpu_conf.c| 41 +++---
 src/conf/cpu_conf.h|  3 +-
 src/qemu/qemu_command.c|  2 +-
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml  |  6 ++--
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml  |  6 ++--
 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml  | 25 +
 tests/qemuxml2argvtest.c   |  1 +
 .../qemuxml2xmlout-cpu-numa1.xml   | 28 +++
 .../qemuxml2xmlout-cpu-numa2.xml   | 28 +++
 tests/qemuxml2xmltest.c|  3 ++
 12 files changed, 145 insertions(+), 20 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b69da4c..9f1082b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1030,8 +1030,8 @@
   
 ...
 
-  
-  
+  
+  
 
 ...
   
@@ -1039,10 +1039,15 @@

 
   Each cell element specifies a NUMA cell or a NUMA node.
-  cpus specifies the CPU or range of CPUs that are part of
-  the node. memory specifies the node memory in kibibytes
-  (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid
-  or nodeid in the increasing order starting from 0.
+  cpus specifies the CPU or range of CPUs that are
+  part of the node. memory specifies the node memory
+  in kibibytes (i.e. blocks of 1024 bytes).
+  Since 1.2.7 all cells should
+  have id attribute in case referring to some cell is
+  necessary in the code, otherwise the cells are
+  assigned ids in the increasing order starting from
+  0.  Mixing cells with and without the id attribute
+  is not recommended as it may result in unwanted behaviour.
 

 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7be028d..155a33e 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3902,6 +3902,11 @@

   
 
+  
+
+  
+
+  
   
 
   
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 811893d..5003cf1 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -152,7 +152,6 @@ virCPUDefCopy(const virCPUDef *cpu)
 copy->ncells_max = copy->ncells = cpu->ncells;

 for (i = 0; i < cpu->ncells; i++) {
-copy->cells[i].cellid = cpu->cells[i].cellid;
 copy->cells[i].mem = cpu->cells[i].mem;

 copy->cells[i].cpumask = virBitmapNewCopy(cpu->cells[i].cpumask);
@@ -438,17 +437,48 @@ virCPUDefParseXML(xmlNodePtr node,
 for (i = 0; i < n; i++) {
 char *cpus, *memory;
 int ret, ncpus = 0;
+unsigned int cur_cell;
+char *tmp = NULL;
+
+tmp = virXMLPropString(nodes[i], "id");
+if (!tmp) {
+cur_cell = i;
+} else {
+ret  = virStrToLong_ui(tmp, NULL, 10, &cur_cell);
+if (ret == -1) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Invalid 'id' attribute in NUMA cell: 
%s"),
+   tmp);
+VIR_FREE(tmp);
+goto error;
+}
+VIR_FREE(tmp);
+}
+
+if (cur_cell >= n) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Exactly one 'cell' element per guest "
+ "NUMA cell allowed, non-contiguous ranges or "
+ "ranges not starting from 0 are not 
allowed"));
+goto error;
+}
+
+if (def->cells[cur_cell].cpustr) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Duplicate NUMA cell info for cell id '%u'"),
+   cur_cell);
+goto err