Re: [libvirt] [PATCH v2 3/9] 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 iothreads element will have 'n' <iothread> children elements which will have attributes "id" and "name". The "id" will allow for definition of any "valid" (eg > 0) iothread_id value. The "name" attribute will allow for adding a name to the alias generated for the IOThread. The name cannot contain "iothread" since that's part of the default IOThread naming scheme already in use. 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. the input XML 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 | 28 +++++ docs/schemas/domaincommon.rng | 17 +++ src/conf/domain_conf.c | 245 +++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 23 ++++ src/libvirt_private.syms | 6 ++ 5 files changed, 317 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7ceb1fa..3224c20 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" name="sysdisk"/&gt; + &lt;iothread id="4" name="userdisk"/&gt; + &lt;iothread id="6" name="datadisk"/&gt; + &lt;iothread id="8" name="datadisk"/&gt; + &lt;/iothreadids&gt; + ... +&lt;/domain&gt; +</pre> <dl> <dt><code>iothreads</code></dt> @@ -530,7 +542,23 @@ 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 and + the optional <code>name</code> attribute is a user defined name that + may be used to name the IOThread for the hypervisor. The id 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 id. + <span class="since">Since 1.2.15</span> + </dd> </dl> <h3><a name="elementsCPUTuning">CPU Tuning</a></h3>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1f5bf62..844caf6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c

@@ -3304,6 +3332,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 (!virDomainIOThreadIDIsDuplicate(def, iothread_id)) { + if (virDomainIOThreadIDAdd(def, iothread_id, NULL) < 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 " @@ -13192,6 +13232,65 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt, return idmap; } +/* Parse the XML definition for an IOThread ID + * + * Format is : + * + * <iothreads>4</iothreads> + * <iothreadids> + * <iothread id='1' name='string'/> + * <iothread id='3' name='string'/> + * <iothread id='5' name='string'/> + * <iothread id='7' name='string'/> + * </iothreadids> + */ +static virDomainIOThreadIDDefPtr +virDomainIOThreadIDDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + virDomainIOThreadIDDefPtr def; + xmlNodePtr oldnode = ctxt->node; + char *tmp = NULL; + + if (VIR_ALLOC(def) < 0) + return NULL; + + ctxt->node = node; + + if (!(tmp = virXPathString("string(./@id)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing <id> element in IOThread ID"));

+ goto error; + } + if (virStrToLong_uip(tmp, NULL, 10, &def->iothread_id) < 0 || + def->iothread_id == 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid iothread 'id' value '%s'"), tmp); + goto error; + } + + def->name = virXMLPropString(node, "name"); + if (def->name && strstr(def->name, "iothread")) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid iothread 'name' value '%s' - cannot " + "contain 'iothread'"), + def->name); + VIR_FREE(def->name);

+ goto error; + } + def->defined = true; + + cleanup: + VIR_FREE(tmp); + ctxt->node = oldnode; + return def; + + error: + VIR_FREE(def);

+ goto cleanup; +} + + /* Parse the XML definition for a vcpupin or emulatorpin. * * vcpupin has the form of @@ -13899,6 +13998,37 @@ 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) { + virReportError(VIR_ERR_XML_ERROR, + _("number of iothreadid iothread elements '%u' is " + "greater than the number of iothreads '%u'"), + n, def->iothreads);

+ goto error; + } + + if (n && VIR_ALLOC_N(def->iothreadids, n) < 0) + goto error;

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

+} + +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, + const char *name) +{ + virDomainIOThreadIDDefPtr iddef = NULL; + + if (virDomainIOThreadIDIsDuplicate(def, iothread_id)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot duplicate iothread_id '%u' in iothreadids"), + iothread_id); + return -1; + } + + if (VIR_ALLOC(iddef) < 0) + goto error; + + iddef->iothread_id = iothread_id; + if (name && VIR_STRDUP(iddef->name, name) < 0)

+ goto error; + + if (!def->iothreadids) { + if (VIR_ALLOC_N(def->iothreadids, 1) < 0) + goto error; + def->niothreadids = 1; + def->iothreadids[0] = iddef;

+ } else { + if (VIR_APPEND_ELEMENT(def->iothreadids, def->niothreadids, iddef) < 0) + goto error; + } + + return 0; + + error: + virDomainIOThreadIDDefFree(iddef); + 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]->name); + VIR_FREE(def->iothreadids[n]);

+ VIR_DELETE_ELEMENT(def->iothreadids, n, def->niothreadids); + return; + } + } +} + /* Check if vcpupin with same id already exists. * Return 1 if exists, 0 if not. */ bool @@ -20618,8 +20836,31 @@ 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'", + def->iothreadids[i]->iothread_id); + if (def->iothreadids[i]->name) + virBufferAsprintf(buf, " name='%s'", + def->iothreadids[i]->name); + virBufferAddLit(buf, "/>\n"); + } + 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 95cbb9c..03a0ecd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2041,6 +2041,19 @@ 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; + char *name;

+}; + +void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def); +void virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def, + int nids); + typedef struct _virDomainCputune virDomainCputune; typedef virDomainCputune *virDomainCputunePtr;