[libvirt] [PATCH v2 2/3] docs, schema, conf: Add support for setting scheduler parameters of guest threads

Martin Kletzander mkletzan at redhat.com
Wed Feb 11 16:13:16 UTC 2015


On Wed, Feb 11, 2015 at 03:04:34PM +0100, Michal Privoznik wrote:
>On 10.02.2015 16:35, Martin Kletzander wrote:
>> In order for QEMU vCPU (and other) threads to run with RT scheduler,
>> libvirt needs to take care of that so QEMU doesn't have to run privileged.
>>
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1178986
>>
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>>  docs/formatdomain.html.in                          |  16 ++
>>  docs/schemas/domaincommon.rng                      |  44 +++++
>>  src/conf/domain_conf.c                             | 183 ++++++++++++++++++++-
>>  src/conf/domain_conf.h                             |  24 +++
>>  .../qemuxml2argv-cputune-iothreadsched-toomuch.xml |  38 +++++
>>  .../qemuxml2argv-cputune-iothreadsched.xml         |  39 +++++
>>  .../qemuxml2argv-cputune-vcpusched-overlap.xml     |  38 +++++
>>  tests/qemuxml2argvtest.c                           |   2 +
>>  .../qemuxml2xmlout-cputune-iothreadsched.xml       |  39 +++++
>>  tests/qemuxml2xmltest.c                            |   1 +
>>  10 files changed, 423 insertions(+), 1 deletion(-)
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched-toomuch.xml
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched.xml
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-vcpusched-overlap.xml
>>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-iothreadsched.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index d8144ea..3381dfe 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -550,6 +550,8 @@
>>      <quota>-1</quota>
>>      <emulator_period>1000000</emulator_period>
>>      <emulator_quota>-1</emulator_quota>
>> +    <vcpusched vcpus='0-4,^3' scheduler='fifo' priority='1'/>
>> +    <iothreadsched iothreads='2' scheduler='batch'/>
>>    </cputune>
>>    ...
>>  </domain>
>> @@ -652,6 +654,20 @@
>>          <span class="since">Only QEMU driver support since 0.10.0</span>
>>        </dd>
>>
>> +      <dt><code>vcpusched</code> and <code>iothreadsched</code></dt>
>> +      <dd>
>> +        The optional <code>vcpusched</code> elements specifie the scheduler
>
>s/specifie/specifies/
>
>And maybe s/scheduler/scheduler type/?
>

I went with "scheduler type".

[...]
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index d467dce..98766dc 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -815,10 +815,54 @@
>>              </attribute>
>>            </element>
>>          </zeroOrMore>
>> +        <zeroOrMore>
>> +          <element name="vcpusched">
>> +            <optional>
>> +              <attribute name="vcpus">
>> +                <ref name='cpuset'/>
>> +              </attribute>
>> +            </optional>
>> +            <ref name="schedparam"/>
>> +          </element>
>> +        </zeroOrMore>
>> +        <zeroOrMore>
>> +          <element name="iothreadsched">
>> +            <optional>
>> +              <attribute name="iothreads">
>> +                <ref name='cpuset'/>
>> +              </attribute>
>> +            </optional>
>> +            <ref name="schedparam"/>
>> +          </element>
>> +        </zeroOrMore>
>>        </interleave>
>>      </element>
>>    </define>
>>
>> +  <define name="schedparam">
>> +    <choice>
>> +      <group>
>> +        <attribute name="scheduler">
>> +          <choice>
>> +            <value>batch</value>
>> +            <value>idle</value>
>> +          </choice>
>> +        </attribute>
>> +      </group>
>> +      <group>
>> +        <attribute name="scheduler">
>> +          <choice>
>> +            <value>fifo</value>
>> +            <value>rr</value>
>> +          </choice>
>> +        </attribute>
>> +        <attribute name="priority">
>> +          <ref name="unsignedShort"/>
>> +        </attribute>
>> +      </group>
>> +    </choice>
>> +  </define>
>> +
>
>Have we returned to the rule where each new XML extension requires ACK
>from at least one of Daniels?
>

Not that I know of.  Not that I know that there was such a rule.

[...]
>> @@ -13139,6 +13218,77 @@ virDomainDefParseXML(xmlDocPtr xml,
>>      }
>>      VIR_FREE(nodes);
>>
>> +    if ((n = virXPathNodeSet("./cputune/vcpusched", ctxt, &nodes)) < 0) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                       _("cannot extract vcpusched nodes"));
>> +        goto error;
>> +    }
>> +    if (n) {
>> +        if (n > def->maxvcpus) {
>> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                           _("too many vcpusched nodes in cputune"));
>> +            goto error;
>> +        }
>> +
>> +        if (VIR_ALLOC_N(def->cputune.vcpusched, n) < 0)
>> +            goto error;
>> +        def->cputune.nvcpusched = n;
>> +
>> +        for (i = 0; i < def->cputune.nvcpusched; i++) {
>> +            if (virDomainThreadSchedParse(nodes[i],
>> +                                          0, def->maxvcpus - 1,
>> +                                          "vcpus",
>> +                                          &def->cputune.vcpusched[i]) < 0)
>> +                goto error;
>> +
>> +            for (j = 0; j < i; j++) {
>> +                if (virBitmapOverlaps(def->cputune.vcpusched[i].ids,
>> +                                      def->cputune.vcpusched[j].ids)) {
>> +                    virReportError(VIR_ERR_XML_DETAIL, "%s",
>> +                                   _("vcpusched attributes 'vcpus' "
>> +                                     "must not overlap"));
>> +                    goto error;
>> +                }
>> +            }
>> +        }
>> +    }
>> +    VIR_FREE(nodes);
>> +
>> +    if ((n = virXPathNodeSet("./cputune/iothreadsched", ctxt, &nodes)) < 0) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                       _("cannot extract iothreadsched nodes"));
>> +        goto error;
>> +    }
>> +    if (n) {
>> +        if (n > def->iothreads) {
>> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                           _("too many iothreadsched nodes in cputune"));
>> +            goto error;
>> +        }
>> +
>> +        if (VIR_ALLOC_N(def->cputune.iothreadsched, n) < 0)
>> +            goto error;
>> +        def->cputune.niothreadsched = n;
>> +
>> +        for (i = 0; i < def->cputune.niothreadsched; i++) {
>> +            if (virDomainThreadSchedParse(nodes[i],
>> +                                          1, def->iothreads,
>> +                                          "iothreads",
>> +                                          &def->cputune.iothreadsched[i]) < 0)
>> +                goto error;
>> +
>> +            for (j = 0; j < i; j++) {
>> +                if (virBitmapOverlaps(def->cputune.iothreadsched[i].ids,
>> +                                      def->cputune.iothreadsched[j].ids)) {
>> +                    virReportError(VIR_ERR_XML_DETAIL, "%s",
>> +                                   _("iothreadsched attributes 'iothreads' "
>> +                                     "must not overlap"));
>> +                    goto error;
>> +                }
>> +            }
>> +        }
>> +    }
>> +    VIR_FREE(nodes);
>
>Once parsed, do we also need a check in virDomainDefCheckABIStability()?
>Or can the sched parameters change on migration? I guess they can, but
>seems like you dig more into this area than me recently.
>

No, this is something QEMU has no idea about and it's something users
can change themselves (when having the right privilege, of course).

[...]
>> @@ -2844,6 +2867,7 @@ VIR_ENUM_DECL(virDomainRNGModel)
>>  VIR_ENUM_DECL(virDomainRNGBackend)
>>  VIR_ENUM_DECL(virDomainTPMModel)
>>  VIR_ENUM_DECL(virDomainTPMBackend)
>> +VIR_ENUM_DECL(virDomainThreadSched)
>
>Yet another ENUM_DECL without corresponding libvirt_private.syms change.
>

Ouch!

>>  /* from libvirt.h */
>>  VIR_ENUM_DECL(virDomainState)
>>  VIR_ENUM_DECL(virDomainNostateReason)
>
>ACK with all of that fixed.
>
>Michal

Thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150211/1f2a1fff/attachment-0001.sig>


More information about the libvir-list mailing list