[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