8:23 a.m.
On Fri, Jul 11, 2014 at 05:11:05PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, 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.
>
> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
> ---
> docs/formatdomain.html.in | 11 +++---
> docs/schemas/domaincommon.rng | 5 +++
> src/conf/cpu_conf.c | 39 +++++++++++++++++++---
> 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, 139 insertions(+), 18 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..ad87b7c 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
[...]
> @@ -1041,8 +1041,11 @@
> Each <code>cell</code> element specifies a NUMA cell or a NUMA
node.
> <code>cpus</code> specifies the CPU or range of CPUs that are
part of
> the node. <code>memory</code> 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.
> + (i.e. blocks of 1024 bytes). All cells should have
<code>id</code>
> + 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 <code>id</code>
attribute
> + is not recommended as it may result in unwanted behaviour.
I'd note here, that the @id attribute is since 1.2.7
I wasn't sure this is needed for attributes, but it cannot hurt,
right? The new hunk would now look like this:
@@ -1039,10 +1039,15 @@
<p>
Each <code>cell</code> element specifies a NUMA cell or a NUMA node.
- <code>cpus</code> specifies the CPU or range of CPUs that are part of
- the node. <code>memory</code> 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.
+ <code>cpus</code> specifies the CPU or range of CPUs that are
+ part of the node. <code>memory</code> specifies the node memory
+ in kibibytes (i.e. blocks of 1024 bytes).
+ <span class="since">Since 1.2.7</span> all cells should
+ have <code>id</code> attribute in case referring to some cell is
+ necessary in the code, otherwise the cells are
+ assigned <code>id</code>s in the increasing order starting from
+ 0. Mixing cells with and without the <code>id</code> attribute
+ is not recommended as it may result in unwanted behaviour.
</p>
<p>
--
Is that OK?
[...]
> @@ -438,17 +437,46 @@ 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);
> + VIR_FREE(tmp);
> + if (ret == -1) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Invalid 'id' attribute in NUMA
cell"));
> + goto error;
> + }
> + }
If there's a typo in the @id, I think this can make users lives easier:
You mean like a typo in an integer, right? :-) If the user cannot find
a typo that causes our code not to parse it as an integer; then I
guess such user's biggest issue isn't the typo :-) But I squashed in
your suggestion.
Martin