[libvirt] [PATCH v5 05/10] conf: Adjust the iothreadsched expectations

[John Ferlan]

On 04/27/2015 10:28 AM, Peter Krempa wrote:
> On Fri, Apr 24, 2015 at 12:05:57 -0400, John Ferlan wrote:
>> With iothreadid's allowing any 'id' value for an iothread_id, the
>> iothreadsched code needs a slight adjustment to allow for "any"
>> unsigned int value in order to create the bitmap of ids that will
>> have scheduler adjustments. Adjusted the doc description as well.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  docs/formatdomain.html.in                                | 16 ++++++++++++----
>>  src/conf/domain_conf.c                                   |  2 +-
>>  .../qemuxml2argv-cputune-iothreadsched-toomuch.xml       |  3 ++-
>>  3 files changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 7af6bd7..0767a2a 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -691,10 +691,18 @@
>>          type (values <code>batch</code>, <code>idle</code>, <code>fifo</code>,
>>          <code>rr</code>) for particular vCPU/IOThread threads (based on
>>          <code>vcpus</code> and <code>iothreads</code>, leaving out
>> -        <code>vcpus</code>/<code>iothreads</code> sets the default).  For
>> -        real-time schedulers (<code>fifo</code>, <code>rr</code>), priority must
>> -        be specified as well (and is ignored for non-real-time ones). The value
>> -        range for the priority depends on the host kernel (usually 1-99).
>> +        <code>vcpus</code>/<code>iothreads</code> sets the default). Valid
>> +        <code>vcpus</code> values start at 0 through one less than the
>> +        number of vCPU's defined for the domain. Valid <code>iothreads</code>
>> +        values are described in the <code>iothreadids</code>
>> +        <a href="#elementsIOThreadsAllocation"><code>description</code></a>.
>> +        If no <code>iothreadids</code> are defined, then libvirt numbers
>> +        IOThreads from 1 to the number of <code>iothreads</code> available
>> +        for the domain. For real-time schedulers (<code>fifo</code>,
>> +        <code>rr</code>), priority must real-time schedulers
>> +        (<code>fifo</code>, <code>rr</code>), priority must be specified as
>> +        well (and is ignored for non-real-time ones). The value range
>> +        for the priority depends on the host kernel (usually 1-99).
>>          <span class="since">Since 1.2.13</span>
>>        </dd>
>>  
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 129908d..9d4c916 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -14348,7 +14348,7 @@ virDomainDefParseXML(xmlDocPtr xml,
>>  
>>          for (i = 0; i < def->cputune.niothreadsched; i++) {
>>              if (virDomainThreadSchedParse(nodes[i],
>> -                                          1, def->iothreads,
>> +                                          1, UINT_MAX,
>>                                            "iothreads",
>>                                            &def->cputune.iothreadsched[i]) < 0)
>>                  goto error;
> 
> I think this patch should also add code that checks that the provided
> scheduler info is provided only for valid iothread IDs.
> 

Yuck...  I know you eschew inline diffs, but it's just easier if nothing
else just to make progress:

index b6a8129..7da94bb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14348,12 +14348,24 @@ virDomainDefParseXML(xmlDocPtr xml,
         def->cputune.niothreadsched = n;
 
         for (i = 0; i < def->cputune.niothreadsched; i++) {
+            ssize_t pos = -1;
+
             if (virDomainThreadSchedParse(nodes[i],
                                           1, UINT_MAX,
                                           "iothreads",
                                           &def->cputune.iothreadsched[i]) < 0)
                 goto error;
 
+            while ((pos = virBitmapNextSetBit(def->cputune.iothreadsched[i].ids,
+                                              pos)) > -1) {
+                if (!virDomainIOThreadIDFind(def, pos)) {
+                    virReportError(VIR_ERR_XML_DETAIL, "%s",
+                                   _("iothreadsched attribute 'iothreads' "
+                                     "uses undefined iothread ids"));
+                    goto error;
+                }
+            }
+
             for (j = 0; j < i; j++) {
                 if (virBitmapOverlaps(def->cputune.iothreadsched[i].ids,
                                       def->cputune.iothreadsched[j].ids)) {