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

[Martin Kletzander]

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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140715/cf0fba41/attachment-0001.sig>