Re: [libvirt] [PATCH v3 1/6] conf: Add new domain XML element 'iothreadids'

Adding a new XML element 'iothreadids' in order to allow defining specific IOThread ID's rather than relying on the algorithm to assign IOThread ID's starting at 1 and incrementing to iothreads count. This will allow future patches to be able to add new IOThreads by a specific iothread_id and of course delete any exisiting IOThread. Each iothreadids element will have 'n' <iothread> children elements which will have attribute "id". The "id" will allow for definition of any "valid" (eg > 0) iothread_id value. On input, if any <iothreadids> <iothread>'s are provided, they will be marked so that we only print out what we read in. On input, if no <iothreadids> are provided, the PostParse code will self generate a list of ID's starting at 1 and going to the number of iothreads defined for the domain (just like the current algorithm numbering scheme). A future patch will rework the existing algorithm to make use of the iothreadids list. On output, only print out the <iothreadids> if they were read in. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/formatdomain.html.in | 30 +++++++ docs/schemas/domaincommon.rng | 12 +++ src/conf/domain_conf.c | 190 +++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 15 ++++ src/libvirt_private.syms | 3 + 5 files changed, 248 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e921749..518f7c5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -521,6 +521,18 @@ ... &lt;/domain&gt; </pre> +<pre> +&lt;domain&gt; + ... + &lt;iothreadids&gt; + &lt;iothread id="2"/&gt; + &lt;iothread id="4"/&gt; + &lt;iothread id="6"/&gt; + &lt;iothread id="8"/&gt; + &lt;/iothreadids&gt; + ... +&lt;/domain&gt; +</pre> <dl> <dt><code>iothreads</code></dt> @@ -530,7 +542,25 @@ virtio-blk-pci and virtio-blk-ccw target storage devices. There should be only 1 or 2 IOThreads per host CPU. There may be more than one supported device assigned to each IOThread. + <span class="since">Since 1.2.8</span> </dd> + <dt><code>iothreadids</code></dt> + <dd> + The optional <code>iothreadids</code> element provides the capability + to specifically define the IOThread ID's for the domain. By default, + IOThread ID's are sequentially numbered starting from 1 through the + number of <code>iothreads</code> defined for the domain. The + <code>id</code> attribute is used to define the IOThread ID. The + <code>id</code> attribute must be a positive integer greater than 0. + If there are less <code>iothreadids</code> defined than + <code>iothreads</code> defined for the domain, then libvirt will + sequentially fill <code>iothreadids</code> starting at 1 avoiding + any predefined <code>id</code>. If there are more + <code>iothreadids</code> defined than <code>iothreads</code> + defined for the domain, then the <code>iothreads</code> value + will be adjusted accordingly. + <span class="since">Since 1.2.15</span> + </dd> </dl> <h3><a name="elementsCPUTuning">CPU Tuning</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 03fd541..4bd32bd 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -675,6 +675,18 @@ </optional> <optional> + <element name="iothreadids"> + <zeroOrMore> + <element name="iothread"> + <attribute name="id"> + <ref name="unsignedInt"/> + </attribute> + </element> + </zeroOrMore> + </element> + </optional> + + <optional> <ref name="blkiotune"/> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4d7e3c9..e86b7e2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2113,6 +2113,23 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin) return NULL; } + +static void +virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def, + int nids) +{ + size_t i; + + if (!def) + return; + + for (i = 0; i < nids; i++) + VIR_FREE(def[i]); + + VIR_FREE(def); +} + + void virDomainPinDefFree(virDomainPinDefPtr def) { @@ -2310,6 +2327,8 @@ void virDomainDefFree(virDomainDefPtr def) virCPUDefFree(def->cpu); + virDomainIOThreadIDDefArrayFree(def->iothreadids, def->niothreadids); + virDomainPinDefArrayFree(def->cputune.vcpupin, def->cputune.nvcpupin); virDomainPinDefFree(def->cputune.emulatorpin); @@ -3304,6 +3323,18 @@ virDomainDefPostParseInternal(virDomainDefPtr def, return -1; } + /* Fully populate the IOThread ID list */ + if (def->iothreads && def->iothreads != def->niothreadids) { + unsigned int iothread_id = 1; + while (def->niothreadids != def->iothreads) { + if (!virDomainIOThreadIDFind(def, iothread_id)) { + if (virDomainIOThreadIDAdd(def, iothread_id) < 0)

+ return -1; + } + iothread_id++; + } + } + if (virDomainDefGetMemoryInitial(def) == 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Memory size must be specified via <memory> or in the " @@ -13173,6 +13204,56 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt, return idmap; } +/* Parse the XML definition for an IOThread ID + * + * Format is : + * + * <iothreads>4</iothreads> + * <iothreadids> + * <iothread id='1'/> + * <iothread id='3'/> + * <iothread id='5'/> + * <iothread id='7'/> + * </iothreadids> + */ +static virDomainIOThreadIDDefPtr +virDomainIOThreadIDDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + virDomainIOThreadIDDefPtr iothrid; + xmlNodePtr oldnode = ctxt->node; + char *tmp = NULL; + + if (VIR_ALLOC(iothrid) < 0) + return NULL; + + ctxt->node = node; + + if (!(tmp = virXPathString("string(./@id)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing 'id' attribute in <iothread> element")); + goto error; + } + if (virStrToLong_uip(tmp, NULL, 10, &iothrid->iothread_id) < 0 || + iothrid->iothread_id == 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid iothread 'id' value '%s'"), tmp); + goto error; + } + + iothrid->defined = true; + + cleanup: + VIR_FREE(tmp); + ctxt->node = oldnode; + return iothrid; + + error: + VIR_FREE(iothrid); + goto cleanup; +} + + /* Parse the XML definition for a vcpupin * * vcpupin has the form of @@ -13948,6 +14029,32 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(tmp); + /* Extract any iothread id's defined */ + if ((n = virXPathNodeSet("./iothreadids/iothread", ctxt, &nodes)) < 0) + goto error; + + if (n > def->iothreads) + def->iothreads = n; + + if (n && VIR_ALLOC_N(def->iothreadids, MAX(n, def->iothreads)) < 0) + goto error;

+ + for (i = 0; i < n; i++) { + virDomainIOThreadIDDefPtr iothrid = NULL; + if (!(iothrid = virDomainIOThreadIDDefParseXML(nodes[i], ctxt))) + goto error; + + if (virDomainIOThreadIDFind(def, iothrid->iothread_id)) { + virReportError(VIR_ERR_XML_ERROR, + _("duplicate iothread id '%u' found"), + iothrid->iothread_id); + VIR_FREE(iothrid); + goto error; + } + def->iothreadids[def->niothreadids++] = iothrid; + } + VIR_FREE(nodes); + /* Extract cpu tunables. */ if ((n = virXPathULong("string(./cputune/shares[1])", ctxt, &def->cputune.shares)) < -1) { @@ -17324,6 +17431,66 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def) return 0; } +virDomainIOThreadIDDefPtr +virDomainIOThreadIDFind(virDomainDefPtr def, + unsigned int iothread_id) +{ + size_t i; + + if (!def->iothreadids || !def->niothreadids) + return NULL; + + for (i = 0; i < def->niothreadids; i++) { + if (iothread_id == def->iothreadids[i]->iothread_id) + return def->iothreadids[i]; + } + + return NULL; +} + +int +virDomainIOThreadIDAdd(virDomainDefPtr def, + unsigned int iothread_id)

+{ + virDomainIOThreadIDDefPtr iothrid = NULL; + + if (virDomainIOThreadIDFind(def, iothread_id)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot duplicate iothread_id '%u' in iothreadids"), + iothread_id); + return -1; + } + + if (VIR_ALLOC(iothrid) < 0) + goto error; + + iothrid->iothread_id = iothread_id; + + if (VIR_APPEND_ELEMENT(def->iothreadids, def->niothreadids, iothrid) < 0) + goto error; + + return 0; + + error: + VIR_FREE(iothrid); + return -1; +} + +void +virDomainIOThreadIDDel(virDomainDefPtr def, + unsigned int iothread_id) +{ + int n; + + for (n = 0; n < def->niothreadids; n++) { + if (def->iothreadids[n]->iothread_id == iothread_id) { + VIR_FREE(def->iothreadids[n]);

+ VIR_DELETE_ELEMENT(def->iothreadids, n, def->niothreadids); + return; + } + } +} + /* Check if vcpupin with same id already exists. */ bool virDomainPinIsDuplicate(virDomainPinDefPtr *def, @@ -20666,8 +20833,27 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, " current='%u'", def->vcpus); virBufferAsprintf(buf, ">%u</vcpu>\n", def->maxvcpus); - if (def->iothreads > 0) - virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n", def->iothreads); + if (def->iothreads > 0) { + virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n", + def->iothreads); + /* If we parsed the iothreadids, then write out those that we parsed */

+ for (i = 0; i < def->niothreadids; i++) { + if (def->iothreadids[i]->defined) + break; + } + if (i < def->niothreadids) { + virBufferAddLit(buf, "<iothreadids>\n"); + virBufferAdjustIndent(buf, 2); + for (i = 0; i < def->niothreadids; i++) { + if (!def->iothreadids[i]->defined) + continue; + virBufferAsprintf(buf, "<iothread id='%u'/>\n", + def->iothreadids[i]->iothread_id); + } + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</iothreadids>\n"); + } + } if (def->cputune.sharesSpecified || (def->cputune.nvcpupin && !virDomainIsAllVcpupinInherited(def)) || diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e6fa3c9..9e7bdf9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2041,6 +2041,14 @@ struct _virDomainHugePage { unsigned long long size; /* hugepage size in KiB */ }; +typedef struct _virDomainIOThreadIDDef virDomainIOThreadIDDef; +typedef virDomainIOThreadIDDef *virDomainIOThreadIDDefPtr; + +struct _virDomainIOThreadIDDef { + bool defined;

+ unsigned int iothread_id; +}; + typedef struct _virDomainCputune virDomainCputune; typedef virDomainCputune *virDomainCputunePtr; @@ -2132,6 +2140,8 @@ struct _virDomainDef { virBitmapPtr cpumask; unsigned int iothreads; + size_t niothreadids; + virDomainIOThreadIDDefPtr *iothreadids; virDomainCputune cputune; @@ -2590,6 +2600,11 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src, int virDomainDefAddImplicitControllers(virDomainDefPtr def); +virDomainIOThreadIDDefPtr +virDomainIOThreadIDFind(virDomainDefPtr def, unsigned int iothread_id);

+int virDomainIOThreadIDAdd(virDomainDefPtr def, unsigned int iothread_id); +void virDomainIOThreadIDDel(virDomainDefPtr def, unsigned int iothread_id); + unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags); char *virDomainDefFormat(virDomainDefPtr def,