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@xxxxxxxxxx> --- 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 schedulers/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.
Attachment:
pgpiQNcWzb3LZ.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list