[libvirt] [PATCH v2 0/9] Implement Add/Del IOThreads
v1: http://www.redhat.com/archives/libvir-list/2015-March/msg01014.html v2 is mostly a completely rewrite of v1 - only smart parts were kept, mostly the "concepts" behind adding/removing a thread Patches 1&2: Couple of simple mods - mostly innocuous Patches 3-5 Since it was determined that just setting the IOThread count and keeping sequential IOThread ID's may not be a desired means to add/remove threads, I decided upon a mechanism which to a degree follows the tune/pin mechanism. Rework the existing IOThread logic to make use of a new domain XML element <iothreadids> which will list each of the <iothread>'s with attributes "id" (required) and "name" (optional). The "id" allows one to generate their own ID number/naming mechanism. If <iothreadids> is not found in the XML, then internally will generate the data structures that support it - IOThreadID's. Work the IOThreadID logic into the "live" iothreadpid lists (or now arrays) to manage the live IOThread data Patches 6-7 Add the API/plumbing for two new API's - virDomainAddIOThread and virDomainDelIOThread to complement the existing virDomainPinIOThread. The "Add" API will take two parameters - an iothread_id and an optional "name". The "Del" API will take just one parameter - the iothread_id Patch 8-9 Implement the qemu backend and virsh front end to manage IOThreads either live or just the config file. I did leave some // comments in Patch 8 as a way to ask a reviewer their thoughts in the add live error path whether 'extra' cleanup should be done or just ignored. Those would be cleaned up and removed for the final version. John Ferlan (9): Rename qemuCheckIothreads to qemuCheckIOThreads Convert virDomainPinIsDuplicate into bool return conf: Add new domain XML element 'iothreadids' qemu: Convert iothreadpids into an array of structures qemu: Use domain iothreadids to populate iothreadpids Implement virDomainAddIOThread and virDomainDelIOThread remote: Add support for AddIOThread and DelIOThread qemu: Add support to Add/Delete IOThreads virsh: Add iothreadadd and iothreaddel commands docs/formatdomain.html.in | 28 ++ docs/schemas/domaincommon.rng | 17 + include/libvirt/libvirt-domain.h | 7 + src/conf/domain_audit.c | 9 + src/conf/domain_audit.h | 6 + src/conf/domain_conf.c | 253 ++++++++++- src/conf/domain_conf.h | 29 +- src/driver-hypervisor.h | 13 + src/libvirt-domain.c | 132 ++++++ src/libvirt_private.syms | 7 + src/libvirt_public.syms | 6 + src/qemu/qemu_cgroup.c | 16 +- src/qemu/qemu_command.c | 108 ++++- src/qemu/qemu_command.h | 4 + src/qemu/qemu_domain.c | 55 ++- src/qemu/qemu_domain.h | 12 +- src/qemu/qemu_driver.c | 467 ++++++++++++++++++++- src/qemu/qemu_process.c | 32 +- src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 31 +- src/remote_protocol-structs | 13 + .../qemuxml2argv-iothreads-ids-partial.args | 10 + .../qemuxml2argv-iothreads-ids-partial.xml | 33 ++ .../qemuxml2argv-iothreads-ids.args | 8 + .../qemuxml2argv-iothreads-ids.xml | 33 ++ .../qemuxml2argv-iothreads-name.args | 17 + .../qemuxml2argv-iothreads-name.xml | 44 ++ tests/qemuxml2argvtest.c | 4 + tests/qemuxml2xmltest.c | 3 + tools/virsh-domain.c | 174 ++++++++ tools/virsh.pod | 32 ++ 31 files changed, 1546 insertions(+), 59 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.xml -- 2.1.0
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5103599..e7e0937 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3964,7 +3964,7 @@ qemuBuildDriveStr(virConnectPtr conn, static bool -qemuCheckIothreads(virDomainDefPtr def, +qemuCheckIOThreads(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainDiskDefPtr disk) { @@ -4024,7 +4024,7 @@ qemuBuildDriveDevStr(virDomainDefPtr def, } } - if (disk->iothread && !qemuCheckIothreads(def, qemuCaps, disk)) + if (disk->iothread && !qemuCheckIOThreads(def, qemuCaps, disk)) goto error; switch (disk->bus) { -- 2.1.0
On Fri, Apr 10, 2015 at 17:36:19 -0400, John Ferlan wrote:
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK, Peter
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/domain_conf.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 880b89c..1f5bf62 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17277,7 +17277,7 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def) /* Check if vcpupin with same id already exists. * Return 1 if exists, 0 if not. */ -int +bool virDomainPinIsDuplicate(virDomainPinDefPtr *def, int npin, int id) @@ -17285,14 +17285,14 @@ virDomainPinIsDuplicate(virDomainPinDefPtr *def, size_t i; if (!def || !npin) - return 0; + return false; for (i = 0; i < npin; i++) { if (def[i]->id == id) - return 1; + return true; } - return 0; + return false; } virDomainPinDefPtr diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 61082c8..95cbb9c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1934,9 +1934,9 @@ void virDomainPinDefArrayFree(virDomainPinDefPtr *def, int npin); virDomainPinDefPtr *virDomainPinDefCopy(virDomainPinDefPtr *src, int npin); -int virDomainPinIsDuplicate(virDomainPinDefPtr *def, - int npin, - int id); +bool virDomainPinIsDuplicate(virDomainPinDefPtr *def, + int npin, + int id); virDomainPinDefPtr virDomainPinFind(virDomainPinDefPtr *def, int npin, -- 2.1.0
On Fri, Apr 10, 2015 at 17:36:20 -0400, John Ferlan wrote:
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/domain_conf.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-)
ACK, by the way, the function is exported, but is used only in conf/domain_conf.c thus it could be converted to static. Peter
On 04/13/2015 06:33 AM, Peter Krempa wrote:
On Fri, Apr 10, 2015 at 17:36:20 -0400, John Ferlan wrote:
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/domain_conf.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-)
ACK, by the way, the function is exported, but is used only in conf/domain_conf.c thus it could be converted to static.
Peter
OK - I've separated these first two out, but since virDomainPinIsDuplicate is defined after it's used - I think it would be worthy of a separate patch "later" (I have it on my short list) to make it local and of course move it... Unless of course you want to do that in the series you have on list right now dealing with the PinParser... John
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@redhat.com> --- 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 @@ ... </domain> </pre> +<pre> +<domain> + ... + <iothreadids> + <iothread id="2" name="sysdisk"/> + <iothread id="4" name="userdisk"/> + <iothread id="6" name="datadisk"/> + <iothread id="8" name="datadisk"/> + </iothreadids> + ... +</domain> +</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/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 03fd541..d2f0898 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -675,6 +675,23 @@ </optional> <optional> + <element name="iothreadids"> + <zeroOrMore> + <element name="iothread"> + <attribute name="id"> + <ref name="unsignedInt"/> + </attribute> + <optional> + <attribute name="name"> + <ref name="genericName"/> + </attribute> + </optional> + </element> + </zeroOrMore> + </element> + </optional> + + <optional> <ref name="blkiotune"/> </optional> 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 @@ -2114,6 +2114,32 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin) } void +virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def) +{ + if (def) { + VIR_FREE(def->name); + VIR_FREE(def); + } +} + + +void +virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def, + int nids) +{ + size_t i; + + if (!def) + return; + + for (i = 0; i < nids; i++) + virDomainIOThreadIDDefFree(def[i]); + + VIR_FREE(def); +} + + +void virDomainPinDefFree(virDomainPinDefPtr def) { if (def) { @@ -2310,6 +2336,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 +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; @@ -2132,6 +2145,8 @@ struct _virDomainDef { virBitmapPtr cpumask; unsigned int iothreads; + size_t niothreadids; + virDomainIOThreadIDDefPtr *iothreadids; virDomainCputune cputune; @@ -2590,6 +2605,14 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src, int virDomainDefAddImplicitControllers(virDomainDefPtr def); +bool virDomainIOThreadIDIsDuplicate(virDomainDefPtr def, + unsigned int iothread_id); +virDomainIOThreadIDDefPtr +virDomainIOThreadIDFind(virDomainDefPtr def, unsigned int iothread_id); +int virDomainIOThreadIDAdd(virDomainDefPtr def, unsigned int iothread_id, + const char *name); +void virDomainIOThreadIDDel(virDomainDefPtr def, unsigned int iothread_id); + unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags); char *virDomainDefFormat(virDomainDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 67ab526..7e55aa0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -325,6 +325,12 @@ virDomainHubTypeToString; virDomainHypervTypeFromString; virDomainHypervTypeToString; virDomainInputDefFree; +virDomainIOThreadIDAdd; +virDomainIOThreadIDDefArrayFree; +virDomainIOThreadIDDefFree; +virDomainIOThreadIDDel; +virDomainIOThreadIDFind; +virDomainIOThreadIDIsDuplicate; virDomainLeaseDefFree; virDomainLeaseIndex; virDomainLeaseInsert; -- 2.1.0
On Fri, Apr 10, 2015 at 17:36:21 -0400, John Ferlan wrote:
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@redhat.com> --- 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 @@ ... </domain> </pre> +<pre> +<domain> + ... + <iothreadids> + <iothread id="2" name="sysdisk"/> + <iothread id="4" name="userdisk"/> + <iothread id="6" name="datadisk"/> + <iothread id="8" name="datadisk"/> + </iothreadids> + ... +</domain> +</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"));
'id' is an attribute, not an element.
+ 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);
This line can be dropped if ...
+ goto error; + } + def->defined = true; + + cleanup: + VIR_FREE(tmp); + ctxt->node = oldnode; + return def; + + error: + VIR_FREE(def);
... you use the freeing function you've added.
+ 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);
Or you can perhaps silently fix def->iothreads so that the user doesn't need to keep them in sync at least when increasing number of threads.
+ goto error; + } + + if (n && VIR_ALLOC_N(def->iothreadids, n) < 0) + goto error;
Here you'll need to allocate the array to MAX(n, def->iothreads) to unify the structures as I'll suggest below.
+ + 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;
How about "return !!virDomainIOThreadIDFind(def, iothread_id) rather than open coding it?
+} + +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)
VIR_STRDUP handles NULL just fine, no need to check @name.
+ goto error; + + if (!def->iothreadids) { + if (VIR_ALLOC_N(def->iothreadids, 1) < 0) + goto error; + def->niothreadids = 1; + def->iothreadids[0] = iddef;
The clause above is not necessary, code below handles NULL arrays just fine.
+ } 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]);
You've introduced a helper that frees the entries. You should use it here.
+ 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; + }
Dead code.
+ if (i < def->niothreadids) { + virBufferAddLit(buf, "<iothreadids>\n"); + virBufferAdjustIndent(buf, 2); + for (i = 0; i < def->niothreadids; i++) { + if (!def->iothreadids[i]->defined) + continue;
If all iothreadids are implicit from the post parse callback this will output an empty '<iothreadis>' element.
+ 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;
This structure along with the one you add in the next patch and the pinning structures that already exist make now three places where we store data regarding IO threads. I don't think we should do that. Keeping track of which data introduces messy code. I think it will be desirable that you move the data regarding pinning of iothreads into this structure along with thread ids from the monitor so that we can keep everything in one place.
+}; + +void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def); +void virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def, + int nids); + typedef struct _virDomainCputune virDomainCputune; typedef virDomainCputune *virDomainCputunePtr;
I think the direction this series is taking is okay. Peter
On 04/13/2015 08:13 AM, Peter Krempa wrote:
On Fri, Apr 10, 2015 at 17:36:21 -0400, John Ferlan wrote:
...
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;
This structure along with the one you add in the next patch and the pinning structures that already exist make now three places where we store data regarding IO threads. I don't think we should do that. Keeping track of which data introduces messy code.
I think it will be desirable that you move the data regarding pinning of iothreads into this structure along with thread ids from the monitor so that we can keep everything in one place.
While I don't disagree that having multiple places and data structures is suboptimal - it is no different than the existing vcpu code which has a [n]vcpupids list for the monitor/driver stored and separate cputune vcpupin list. What it doesn't have is a mechanism to have different vcpu id numbers - it forces sequential. There's also a lot to be said for keeping the cputune stuff together as much as there is for keeping the iothreadsid stuff together. The whole point behind not printing out <iothreadsid> <iothread ...> was if it doesn't exist, we can continue with the existing algorithm and no one is the wiser. As soon as someone goes to 'customize' by adding a non sequential id, then things are exposed. I could care less either way though. If the general feeling is print it regardless of whether it was found on input, then that's fine by me - makes it easier. Another "option" for the <name> (if it was to be kept) is that it becomes the alias for the thread. The current algorithm just generates the "iothread#" as a/the mechanism for the alias. An IOThread when "added" can use any name as long as it's unique. All that said - I'm fine with removing <name> - it was added mainly because I felt "id" would be lonely all by itself ;-)... Then moving the cputune.iothreadpin data into the internal workings for iothreadid's is fine - just have to account for existing configurations in some manner. Finally having the "live" information in the same data structure is fine - I separated it mainly because existing code has it separated. I didn't particularly like the existing code, but since no one has changed it for all the years it's been there, well I didn't want that added onto the "to do" list. Finally as for some of the extraneous comments - you may in some cases find them stating obvious facts, I've also seen that what some consider to be self documenting code isn't in fact self documenting unless you're the author or have become very familiar with the code due to working on patches in the space. I'll rework and repost tomorrow. Good to know this is moving in the right direction John FWIW: In patch 8 where I used // - yes I knew that (the .0 mentioned it), but I was trying to draw attention to it. The vcpu code doesn't do much in the way of error recovery and while I could just do the same, I figured I'd point it out rather than just ignore it. I knew the // would cause someone to balk.
+}; + +void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def); +void virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def, + int nids); + typedef struct _virDomainCputune virDomainCputune; typedef virDomainCputune *virDomainCputunePtr;
I think the direction this series is taking is okay.
Peter
Create qemuDomainIOThreadInfo which currently will just be the thread_id returned from IOThreadInfo, but will soon expand to handle the needs of live IOThread data for adding/deleting IOThread's Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_cgroup.c | 3 ++- src/qemu/qemu_domain.c | 19 ++++++++++++++----- src/qemu/qemu_domain.h | 10 ++++++++-- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_process.c | 7 ++++--- 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index e83342d..da07346 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1196,7 +1196,8 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm) goto cleanup; /* move the thread for iothread to sub dir */ - if (virCgroupAddTask(cgroup_iothread, priv->iothreadpids[i]) < 0) + if (virCgroupAddTask(cgroup_iothread, + priv->iothreadpids[i]->thread_id) < 0) goto cleanup; if (period || quota) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ae632c5..2b4a519 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -429,6 +429,8 @@ qemuDomainObjPrivateAlloc(void) static void qemuDomainObjPrivateFree(void *data) { + size_t i; + qemuDomainObjPrivatePtr priv = data; virObjectUnref(priv->qemuCaps); @@ -440,6 +442,8 @@ qemuDomainObjPrivateFree(void *data) virDomainChrSourceDefFree(priv->monConfig); qemuDomainObjFreeJob(priv); VIR_FREE(priv->vcpupids); + for (i = 0; i < priv->niothreadpids; i++) + VIR_FREE(priv->iothreadpids[i]); VIR_FREE(priv->iothreadpids); VIR_FREE(priv->lockState); VIR_FREE(priv->origname); @@ -507,7 +511,7 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) virBufferAdjustIndent(buf, 2); for (i = 0; i < priv->niothreadpids; i++) { virBufferAsprintf(buf, "<iothread pid='%d'/>\n", - priv->iothreadpids[i]); + priv->iothreadpids[i]->thread_id); } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</iothreads>\n"); @@ -638,16 +642,21 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data) goto error; if (n) { priv->niothreadpids = n; - if (VIR_REALLOC_N(priv->iothreadpids, priv->niothreadpids) < 0) + if (VIR_ALLOC_N(priv->iothreadpids, priv->niothreadpids) < 0) goto error; for (i = 0; i < n; i++) { + qemuDomainIOThreadInfoPtr info = NULL; char *pidstr = virXMLPropString(nodes[i], "pid"); - if (!pidstr) - goto error; + if (!pidstr || VIR_ALLOC(info) < 0) { + VIR_FREE(pidstr); + VIR_FREE(info); + goto error; + } + priv->iothreadpids[i] = info; if (virStrToLong_i(pidstr, NULL, 10, - &(priv->iothreadpids[i])) < 0) { + &info->thread_id) < 0) { VIR_FREE(pidstr); goto error; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 3225abb..2f75c70 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -136,6 +136,12 @@ struct qemuDomainJobObj { typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver, virDomainObjPtr vm); +typedef struct _qemuDomainIOThreadInfo qemuDomainIOThreadInfo; +typedef qemuDomainIOThreadInfo *qemuDomainIOThreadInfoPtr; +struct _qemuDomainIOThreadInfo { + int thread_id; +}; + typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate; typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr; struct _qemuDomainObjPrivate { @@ -158,8 +164,8 @@ struct _qemuDomainObjPrivate { int nvcpupids; int *vcpupids; - int niothreadpids; - int *iothreadpids; + size_t niothreadpids; + qemuDomainIOThreadInfoPtr *iothreadpids; virDomainPCIAddressSetPtr pciaddrs; virDomainCCWAddressSetPtr ccwaddrs; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f37a11e..1c575d0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6053,7 +6053,7 @@ qemuDomainPinIOThread(virDomainPtr dom, if (iothread_id > priv->niothreadpids) { virReportError(VIR_ERR_INVALID_ARG, - _("iothread value out of range %d > %d"), + _("iothread value out of range %d > %zu"), iothread_id, priv->niothreadpids); goto endjob; } @@ -6092,7 +6092,7 @@ qemuDomainPinIOThread(virDomainPtr dom, goto endjob; } } else { - if (virProcessSetAffinity(priv->iothreadpids[iothread_id - 1], + if (virProcessSetAffinity(priv->iothreadpids[iothread_id - 1]->thread_id, pcpumap) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("failed to set cpu affinity for IOThread %d"), diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 753afe8..939b8f5 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2272,7 +2272,7 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver, priv->niothreadpids = niothreads; for (i = 0; i < priv->niothreadpids; i++) - priv->iothreadpids[i] = iothreads[i]->thread_id; + priv->iothreadpids[i]->thread_id = iothreads[i]->thread_id; ret = 0; @@ -2469,7 +2469,8 @@ qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm) i + 1))) continue; - if (virProcessSetAffinity(priv->iothreadpids[i], pininfo->cpumask) < 0) + if (virProcessSetAffinity(priv->iothreadpids[i]->thread_id, + pininfo->cpumask) < 0) goto cleanup; } ret = 0; @@ -2520,7 +2521,7 @@ qemuProcessSetSchedulers(virDomainObjPtr vm) } for (i = 0; i < priv->niothreadpids; i++) { - if (qemuProcessSetSchedParams(i + 1, priv->iothreadpids[i], + if (qemuProcessSetSchedParams(i + 1, priv->iothreadpids[i]->thread_id, vm->def->cputune.niothreadsched, vm->def->cputune.iothreadsched) < 0) return -1; -- 2.1.0
On Fri, Apr 10, 2015 at 17:36:22 -0400, John Ferlan wrote:
Create qemuDomainIOThreadInfo which currently will just be the thread_id returned from IOThreadInfo, but will soon expand to handle the needs of live IOThread data for adding/deleting IOThread's
As I've said in previous patch the data should be part of the structure you'll be adding there. Peter
Rather than use the default numbering scheme of 1..number of iothreads defined for the domain, use the iothreadid's list for the iothread_id and possibly augmenting the alias using the iothreadsid name. This also requires adjusting the iothreadpids structure to include room for the iothread_id and name (if found in the alias, thus iothreadids entry). The iothreadpids will keep track of the live system, while iothreadids will be used for the configuration. Now that the iothreadpids list keeps track of the iothread_id's, these can be used in place of the many places where a for loop would "know" that the ID was "+ 1" from the array element. The new tests ensure usage of the <iothreadid> values for an exact number of iothreads, the usage of a smaller number of <iothreadid> values than iothreads that exist (and usage of the default numbering scheme), and the usage of the optional name value to provide the alias for the IOThread. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_cgroup.c | 13 ++- src/qemu/qemu_command.c | 104 ++++++++++++++++++--- src/qemu/qemu_command.h | 4 + src/qemu/qemu_domain.c | 40 +++++++- src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 35 ++++--- src/qemu/qemu_process.c | 29 +++++- .../qemuxml2argv-iothreads-ids-partial.args | 10 ++ .../qemuxml2argv-iothreads-ids-partial.xml | 33 +++++++ .../qemuxml2argv-iothreads-ids.args | 8 ++ .../qemuxml2argv-iothreads-ids.xml | 33 +++++++ .../qemuxml2argv-iothreads-name.args | 17 ++++ .../qemuxml2argv-iothreads-name.xml | 44 +++++++++ tests/qemuxml2argvtest.c | 4 + tests/qemuxml2xmltest.c | 3 + 15 files changed, 339 insertions(+), 40 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.xml diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index da07346..d74302a 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -803,7 +803,8 @@ qemuRestoreCgroupState(virDomainObjPtr vm) } for (i = 0; i < priv->niothreadpids; i++) { - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, i + 1, + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, + priv->iothreadpids[i]->iothread_id, false, &cgroup_temp) < 0 || virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 || virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0 || @@ -1188,10 +1189,8 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm) goto cleanup; for (i = 0; i < priv->niothreadpids; i++) { - /* IOThreads are numbered 1..n, although the array is 0..n-1, - * so we will account for that here - */ - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, i + 1, + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, + priv->iothreadpids[i]->iothread_id, true, &cgroup_iothread) < 0) goto cleanup; @@ -1222,8 +1221,8 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm) /* specific cpu mask */ for (j = 0; j < def->cputune.niothreadspin; j++) { - /* IOThreads are numbered/named 1..n */ - if (def->cputune.iothreadspin[j]->id == i + 1) { + if (def->cputune.iothreadspin[j]->id == + priv->iothreadpids[i]->iothread_id) { cpumask = def->cputune.iothreadspin[j]->cpumask; break; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e7e0937..68c85e2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -678,6 +678,57 @@ qemuOpenVhostNet(virDomainDefPtr def, } int +qemuDomainParseIOThreadAlias(char *alias, + unsigned int *iothread_id, + char **name) +{ + unsigned int idval; + char *idname = NULL; + + /* IOThread's alias is either "iothread#" or "name_iothread#", where + * name is a user definable prefix for the alias and the # is the + * iothreadids iothread_id provided in XML or generated during + * post parse processing + */ + if (STRPREFIX(alias, "iothread")) { + if (virStrToLong_ui(alias + strlen("iothread"), + NULL, 10, &idval) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to find iothread id for '%s'"), + alias); + return -1; + } + /* Default - no need to do anything with name */ + } else { + char *spot = strstr(alias, "_iothread"); + + if (!spot) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed IOThreads alias '%s'"), + alias); + return -1; + } + + /* Pick off the user defined name from the front */ + if (VIR_STRNDUP(idname, alias, spot - alias) < 0) + return -1; + + if (virStrToLong_ui(alias + strlen(idname) + strlen("_iothread"), + NULL, 10, &idval) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to find iothread id for '%s'"), + alias); + VIR_FREE(idname); + return -1; + } + } + + *iothread_id = idval; + *name = idname; + return 0; +} + +int qemuNetworkPrepareDevices(virDomainDefPtr def) { int ret = -1; @@ -3985,11 +4036,11 @@ qemuCheckIOThreads(virDomainDefPtr def, return false; } - /* Value larger than iothreads available? */ - if (disk->iothread > def->iothreads) { + /* Can we find the disk iothread in the iothreadid list? */ + if (!virDomainIOThreadIDFind(def, disk->iothread)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Disk iothread '%u' invalid only %u IOThreads"), - disk->iothread, def->iothreads); + _("Disk iothread '%u' not defined"), + disk->iothread); return false; } @@ -4005,6 +4056,8 @@ qemuBuildDriveDevStr(virDomainDefPtr def, { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); + virDomainIOThreadIDDefPtr iothrid = NULL; + char *disk_iothr_alias = NULL; int controllerModel; if (virDomainDiskDefDstDuplicates(def)) @@ -4194,10 +4247,27 @@ qemuBuildDriveDevStr(virDomainDefPtr def, } break; case VIR_DOMAIN_DISK_BUS_VIRTIO: + if (disk->iothread) { + if (!(iothrid = virDomainIOThreadIDFind(def, disk->iothread))) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot find iothread '%u' in iothreadids"), + disk->iothread); + goto error; + } + if (iothrid->name) { + if (virAsprintf(&disk_iothr_alias, "%s_iothread%u", + iothrid->name, iothrid->iothread_id) < 0) + goto error; + } else { + if (virAsprintf(&disk_iothr_alias, "iothread%u", + iothrid->iothread_id) < 0) + goto error; + } + } if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { virBufferAddLit(&opt, "virtio-blk-ccw"); - if (disk->iothread) - virBufferAsprintf(&opt, ",iothread=iothread%u", disk->iothread); + if (disk_iothr_alias) + virBufferAsprintf(&opt, ",iothread=%s", disk_iothr_alias); } else if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { virBufferAddLit(&opt, "virtio-blk-s390"); @@ -4206,9 +4276,11 @@ qemuBuildDriveDevStr(virDomainDefPtr def, virBufferAddLit(&opt, "virtio-blk-device"); } else { virBufferAddLit(&opt, "virtio-blk-pci"); - if (disk->iothread) - virBufferAsprintf(&opt, ",iothread=iothread%u", disk->iothread); + if (disk_iothr_alias) + virBufferAsprintf(&opt, ",iothread=%s", disk_iothr_alias); } + VIR_FREE(disk_iothr_alias); + qemuBuildIoEventFdStr(&opt, disk->ioeventfd, qemuCaps); if (disk->event_idx && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_EVENT_IDX)) { @@ -8794,14 +8866,20 @@ qemuBuildCommandLine(virConnectPtr conn, if (def->iothreads > 0 && virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { - /* Create named iothread objects starting with 1. These may be used + /* Create iothread objects using the defined iothreadids list + * and the defined id and name from the list. These may be used * by a disk definition which will associate to an iothread by - * supplying a value of 1 up to the number of iothreads available - * (since 0 would indicate to not use the feature). + * supplying a value of an id from the list */ - for (i = 1; i <= def->iothreads; i++) { + for (i = 0; i < def->niothreadids; i++) { virCommandAddArg(cmd, "-object"); - virCommandAddArgFormat(cmd, "iothread,id=iothread%zu", i); + if (def->iothreadids[i]->name) + virCommandAddArgFormat(cmd, "iothread,id=%s_iothread%u", + def->iothreadids[i]->name, + def->iothreadids[i]->iothread_id); + else + virCommandAddArgFormat(cmd, "iothread,id=iothread%u", + def->iothreadids[i]->iothread_id); } } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index a29db41..70be7ec 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -238,6 +238,10 @@ int qemuOpenVhostNet(virDomainDefPtr def, int *vhostfd, size_t *vhostfdSize); +int qemuDomainParseIOThreadAlias(char *alias, + unsigned int *iothread_id, + char **name); + int qemuNetworkPrepareDevices(virDomainDefPtr def); /* diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2b4a519..7244c2c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -442,8 +442,10 @@ qemuDomainObjPrivateFree(void *data) virDomainChrSourceDefFree(priv->monConfig); qemuDomainObjFreeJob(priv); VIR_FREE(priv->vcpupids); - for (i = 0; i < priv->niothreadpids; i++) + for (i = 0; i < priv->niothreadpids; i++) { + VIR_FREE(priv->iothreadpids[i]->name); VIR_FREE(priv->iothreadpids[i]); + } VIR_FREE(priv->iothreadpids); VIR_FREE(priv->lockState); VIR_FREE(priv->origname); @@ -510,8 +512,16 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) virBufferAddLit(buf, "<iothreads>\n"); virBufferAdjustIndent(buf, 2); for (i = 0; i < priv->niothreadpids; i++) { - virBufferAsprintf(buf, "<iothread pid='%d'/>\n", - priv->iothreadpids[i]->thread_id); + if (priv->iothreadpids[i]->name) + virBufferAsprintf(buf, + "<iothread pid='%d' iothreadid='%u' name='%s'/>\n", + priv->iothreadpids[i]->thread_id, + priv->iothreadpids[i]->iothread_id, + priv->iothreadpids[i]->name); + else + virBufferAsprintf(buf, "<iothread pid='%d' iothreadid='%u'/>\n", + priv->iothreadpids[i]->thread_id, + priv->iothreadpids[i]->iothread_id); } virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</iothreads>\n"); @@ -648,6 +658,7 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data) for (i = 0; i < n; i++) { qemuDomainIOThreadInfoPtr info = NULL; char *pidstr = virXMLPropString(nodes[i], "pid"); + char *idstr; if (!pidstr || VIR_ALLOC(info) < 0) { VIR_FREE(pidstr); @@ -661,6 +672,29 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data) goto error; } VIR_FREE(pidstr); + + /* Prior to 1.2.15 - iothread_id's were generated by starting + * at 1 and incrementing to the number of iothread's. So follow + * that here for any running domain that has iothreads configured + * so that the domain doesn't disappear. As of 1.2.15, the write + * of the running domain will include the iothreadid, so it + * should be found. The 'name' is optional and also is only + * available as of 1.2.15 - the only way it'll be present is if + * idstr is present and something was defined; otherwise, it'll + * be NULL which indicates usage of only the default algorithm. + */ + idstr = virXMLPropString(nodes[i], "iothreadid"); + if (!idstr) { + info->iothread_id = i + 1; + } else { + if (virStrToLong_ui(idstr, NULL, 10, + &info->iothread_id) < 0) { + VIR_FREE(idstr); + goto error; + } + info->name = virXMLPropString(nodes[i], "name"); + } + VIR_FREE(idstr); } VIR_FREE(nodes); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2f75c70..515d73b 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -140,6 +140,8 @@ typedef struct _qemuDomainIOThreadInfo qemuDomainIOThreadInfo; typedef qemuDomainIOThreadInfo *qemuDomainIOThreadInfoPtr; struct _qemuDomainIOThreadInfo { int thread_id; + unsigned int iothread_id; + char *name; }; typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1c575d0..d99f886 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5827,14 +5827,20 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver, goto endjob; for (i = 0; i < niothreads; i++) { + unsigned int iothread_id; + char *name = NULL; virBitmapPtr map = NULL; - if (VIR_ALLOC(info_ret[i]) < 0) + if (qemuDomainParseIOThreadAlias(iothreads[i]->name, + &iothread_id, &name) < 0) goto endjob; - if (virStrToLong_ui(iothreads[i]->name + strlen("iothread"), NULL, 10, - &info_ret[i]->iothread_id) < 0) + if (VIR_ALLOC(info_ret[i]) < 0) { + VIR_FREE(name); goto endjob; + } + info_ret[i]->iothread_id = iothread_id; + VIR_FREE(name); if (virProcessGetAffinity(iothreads[i]->thread_id, &map, hostcpus) < 0) goto endjob; @@ -5889,19 +5895,19 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef, if (VIR_ALLOC_N(info_ret, targetDef->iothreads) < 0) goto cleanup; - for (i = 0; i < targetDef->iothreads; i++) { + for (i = 0; i < targetDef->niothreadids; i++) { virDomainPinDefPtr pininfo; if (VIR_ALLOC(info_ret[i]) < 0) goto cleanup; - /* IOThreads being counting at 1 */ - info_ret[i]->iothread_id = i + 1; + /* IOThread ID's are taken from the iothreadids list */ + info_ret[i]->iothread_id = targetDef->iothreadids[i]->iothread_id; /* Initialize the cpumap */ pininfo = virDomainPinFind(targetDef->cputune.iothreadspin, targetDef->cputune.niothreadspin, - i + 1); + targetDef->iothreadids[i]->iothread_id); if (!pininfo) { if (targetDef->cpumask) { cpumask = targetDef->cpumask; @@ -5988,6 +5994,7 @@ qemuDomainPinIOThread(virDomainPtr dom, unsigned int flags) { int ret = -1; + size_t i; virQEMUDriverPtr driver = dom->conn->privateData; virQEMUDriverConfigPtr cfg = NULL; virDomainObjPtr vm; @@ -6051,10 +6058,13 @@ qemuDomainPinIOThread(virDomainPtr dom, goto endjob; } - if (iothread_id > priv->niothreadpids) { + for (i = 0; i < priv->niothreadpids; i++) { + if (iothread_id == priv->iothreadpids[i]->iothread_id) + break; + } + if (i == priv->niothreadpids) { virReportError(VIR_ERR_INVALID_ARG, - _("iothread value out of range %d > %zu"), - iothread_id, priv->niothreadpids); + _("iothread value %d not found"), iothread_id); goto endjob; } @@ -6092,7 +6102,7 @@ qemuDomainPinIOThread(virDomainPtr dom, goto endjob; } } else { - if (virProcessSetAffinity(priv->iothreadpids[iothread_id - 1]->thread_id, + if (virProcessSetAffinity(priv->iothreadpids[i]->thread_id, pcpumap) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("failed to set cpu affinity for IOThread %d"), @@ -10039,7 +10049,8 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, } for (i = 0; i < priv->niothreadpids; i++) { - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, i + 1, + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, + priv->iothreadpids[i]->iothread_id, false, &cgroup_temp) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 939b8f5..a17ce70 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2271,8 +2271,24 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver, goto cleanup; priv->niothreadpids = niothreads; - for (i = 0; i < priv->niothreadpids; i++) - priv->iothreadpids[i]->thread_id = iothreads[i]->thread_id; + for (i = 0; i < priv->niothreadpids; i++) { + unsigned int iothread_id; + char *name = NULL; + qemuDomainIOThreadInfoPtr info; + + if (qemuDomainParseIOThreadAlias(iothreads[i]->name, + &iothread_id, &name) < 0) + goto cleanup; + + if (VIR_ALLOC(info) < 0) { + VIR_FREE(name); + goto cleanup; + } + priv->iothreadpids[i] = info; + info->thread_id = iothreads[i]->thread_id; + info->iothread_id = iothread_id; + info->name = name; + } ret = 0; @@ -2462,11 +2478,11 @@ qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm) return -1; } - for (i = 0; i < def->iothreads; i++) { + for (i = 0; i < priv->niothreadpids; i++) { /* set affinity only for existing vcpus */ if (!(pininfo = virDomainPinFind(def->cputune.iothreadspin, def->cputune.niothreadspin, - i + 1))) + priv->iothreadpids[i]->iothread_id))) continue; if (virProcessSetAffinity(priv->iothreadpids[i]->thread_id, @@ -2521,7 +2537,8 @@ qemuProcessSetSchedulers(virDomainObjPtr vm) } for (i = 0; i < priv->niothreadpids; i++) { - if (qemuProcessSetSchedParams(i + 1, priv->iothreadpids[i]->thread_id, + if (qemuProcessSetSchedParams(priv->iothreadpids[i]->iothread_id, + priv->iothreadpids[i]->thread_id, vm->def->cputune.niothreadsched, vm->def->cputune.iothreadsched) < 0) return -1; @@ -5306,6 +5323,8 @@ void qemuProcessStop(virQEMUDriverPtr driver, virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason); VIR_FREE(priv->vcpupids); priv->nvcpupids = 0; + for (i = 0; i < priv->niothreadpids; i++) + VIR_FREE(priv->iothreadpids[i]); VIR_FREE(priv->iothreadpids); priv->niothreadpids = 0; virObjectUnref(priv->qemuCaps); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args new file mode 100644 index 0000000..444cd17 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args @@ -0,0 +1,10 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M \ +pc -m 214 -smp 2 \ +-object iothread,id=iothread5 \ +-object iothread,id=iothread6 \ +-object iothread,id=iothread1 \ +-object iothread,id=iothread2 \ +-nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml new file mode 100644 index 0000000..c631677 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <iothreads>4</iothreads> + <iothreadids> + <iothread id='5'/> + <iothread id='6'/> + </iothreadids> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args new file mode 100644 index 0000000..68998f6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M \ +pc -m 214 -smp 2 \ +-object iothread,id=iothread2 \ +-object iothread,id=iothread4 \ +-nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ +/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml new file mode 100644 index 0000000..d70e74b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <iothreads>2</iothreads> + <iothreadids> + <iothread id='2'/> + <iothread id='4'/> + </iothreadids> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.args b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.args new file mode 100644 index 0000000..824b7ac --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.args @@ -0,0 +1,17 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M \ +pc -m 214 -smp 2 \ +-object iothread,id=test1disk_iothread1 \ +-object iothread,id=test2disk_iothread2 \ +-nographic -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ +-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-drive file=/var/lib/libvirt/images/iothrtest1.img,if=none,\ +id=drive-virtio-disk1 \ +-device virtio-blk-pci,iothread=test1disk_iothread1,bus=pci.0,addr=0x4,\ +drive=drive-virtio-disk1,id=virtio-disk1 \ +-drive file=/var/lib/libvirt/images/iothrtest2.img,if=none,\ +id=drive-virtio-disk2 \ +-device virtio-blk-pci,iothread=test2disk_iothread2,bus=pci.0,addr=0x3,\ +drive=drive-virtio-disk2,id=virtio-disk2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.xml b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.xml new file mode 100644 index 0000000..85b502f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.xml @@ -0,0 +1,44 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <iothreads>2</iothreads> + <iothreadids> + <iothread id='1' name='test1disk'/> + <iothread id='2' name='test2disk'/> + </iothreadids> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='raw' iothread='1'/> + <source file='/var/lib/libvirt/images/iothrtest1.img'/> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='raw' iothread='2'/> + <source file='/var/lib/libvirt/images/iothrtest2.img'/> + <target dev='vdc' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c02555d..1508d59 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1224,6 +1224,10 @@ mymain(void) DO_TEST("smp", QEMU_CAPS_SMP_TOPOLOGY); DO_TEST("iothreads", QEMU_CAPS_OBJECT_IOTHREAD); + DO_TEST("iothreads-ids", QEMU_CAPS_OBJECT_IOTHREAD); + DO_TEST("iothreads-ids-partial", QEMU_CAPS_OBJECT_IOTHREAD); + DO_TEST("iothreads-name", QEMU_CAPS_OBJECT_IOTHREAD, QEMU_CAPS_DEVICE, + QEMU_CAPS_DRIVE); DO_TEST("iothreads-disk", QEMU_CAPS_OBJECT_IOTHREAD, QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE); DO_TEST("iothreads-disk-virtio-ccw", QEMU_CAPS_OBJECT_IOTHREAD, QEMU_CAPS_DEVICE, diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 817e408..a9529fd 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -492,6 +492,9 @@ mymain(void) DO_TEST("smp"); DO_TEST("iothreads"); + DO_TEST("iothreads-ids"); + DO_TEST("iothreads-ids-partial"); + DO_TEST("iothreads-name"); DO_TEST_DIFFERENT("cputune-iothreads"); DO_TEST("iothreads-disk"); DO_TEST("iothreads-disk-virtio-ccw"); -- 2.1.0
On Fri, Apr 10, 2015 at 17:36:23 -0400, John Ferlan wrote:
Rather than use the default numbering scheme of 1..number of iothreads defined for the domain, use the iothreadid's list for the iothread_id and possibly augmenting the alias using the iothreadsid name.
This also requires adjusting the iothreadpids structure to include room for the iothread_id and name (if found in the alias, thus iothreadids entry).
The iothreadpids will keep track of the live system, while iothreadids will be used for the configuration.
Now that the iothreadpids list keeps track of the iothread_id's, these can be used in place of the many places where a for loop would "know" that the ID was "+ 1" from the array element.
The new tests ensure usage of the <iothreadid> values for an exact number of iothreads, the usage of a smaller number of <iothreadid> values than iothreads that exist (and usage of the default numbering scheme), and the usage of the optional name value to provide the alias for the IOThread.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_cgroup.c | 13 ++- src/qemu/qemu_command.c | 104 ++++++++++++++++++--- src/qemu/qemu_command.h | 4 + src/qemu/qemu_domain.c | 40 +++++++- src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 35 ++++--- src/qemu/qemu_process.c | 29 +++++- .../qemuxml2argv-iothreads-ids-partial.args | 10 ++ .../qemuxml2argv-iothreads-ids-partial.xml | 33 +++++++ .../qemuxml2argv-iothreads-ids.args | 8 ++ .../qemuxml2argv-iothreads-ids.xml | 33 +++++++ .../qemuxml2argv-iothreads-name.args | 17 ++++ .../qemuxml2argv-iothreads-name.xml | 44 +++++++++ tests/qemuxml2argvtest.c | 4 + tests/qemuxml2xmltest.c | 3 + 15 files changed, 339 insertions(+), 40 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-name.xml
...
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e7e0937..68c85e2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -678,6 +678,57 @@ qemuOpenVhostNet(virDomainDefPtr def, }
int +qemuDomainParseIOThreadAlias(char *alias, + unsigned int *iothread_id, + char **name) +{ + unsigned int idval; + char *idname = NULL; + + /* IOThread's alias is either "iothread#" or "name_iothread#", where
I don't really think we should put any user-configurable stuff into the alias. We can keep the name internally and use it for lookup but using it in the alias can be tricky. If it would be part of the alias, the name definitely should not start with the user configurable part.
+ * name is a user definable prefix for the alias and the # is the + * iothreadids iothread_id provided in XML or generated during + * post parse processing + */ + if (STRPREFIX(alias, "iothread")) { + if (virStrToLong_ui(alias + strlen("iothread"), + NULL, 10, &idval) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to find iothread id for '%s'"), + alias); + return -1; + } + /* Default - no need to do anything with name */ + } else { + char *spot = strstr(alias, "_iothread"); + + if (!spot) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed IOThreads alias '%s'"), + alias); + return -1; + } + + /* Pick off the user defined name from the front */ + if (VIR_STRNDUP(idname, alias, spot - alias) < 0) + return -1; + + if (virStrToLong_ui(alias + strlen(idname) + strlen("_iothread"), + NULL, 10, &idval) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to find iothread id for '%s'"), + alias); + VIR_FREE(idname); + return -1; + } + } + + *iothread_id = idval; + *name = idname; + return 0; +} + +int qemuNetworkPrepareDevices(virDomainDefPtr def) { int ret = -1; @@ -3985,11 +4036,11 @@ qemuCheckIOThreads(virDomainDefPtr def, return false; }
- /* Value larger than iothreads available? */ - if (disk->iothread > def->iothreads) { + /* Can we find the disk iothread in the iothreadid list? */
Comment is not necessary.
+ if (!virDomainIOThreadIDFind(def, disk->iothread)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Disk iothread '%u' invalid only %u IOThreads"), - disk->iothread, def->iothreads); + _("Disk iothread '%u' not defined"), + disk->iothread); return false; }
@@ -4005,6 +4056,8 @@ qemuBuildDriveDevStr(virDomainDefPtr def, { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); + virDomainIOThreadIDDefPtr iothrid = NULL; + char *disk_iothr_alias = NULL; int controllerModel;
if (virDomainDiskDefDstDuplicates(def)) @@ -4194,10 +4247,27 @@ qemuBuildDriveDevStr(virDomainDefPtr def, } break; case VIR_DOMAIN_DISK_BUS_VIRTIO: + if (disk->iothread) { + if (!(iothrid = virDomainIOThreadIDFind(def, disk->iothread))) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot find iothread '%u' in iothreadids"), + disk->iothread); + goto error; + } + if (iothrid->name) { + if (virAsprintf(&disk_iothr_alias, "%s_iothread%u", + iothrid->name, iothrid->iothread_id) < 0)
As said, we should not put the strings into qemu... or definitely not at the beginning.
+ goto error; + } else { + if (virAsprintf(&disk_iothr_alias, "iothread%u", + iothrid->iothread_id) < 0) + goto error; + } + } if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { virBufferAddLit(&opt, "virtio-blk-ccw"); - if (disk->iothread) - virBufferAsprintf(&opt, ",iothread=iothread%u", disk->iothread); + if (disk_iothr_alias) + virBufferAsprintf(&opt, ",iothread=%s", disk_iothr_alias); } else if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { virBufferAddLit(&opt, "virtio-blk-s390");
Peter
Add libvirt API's to manage adding and deleting IOThreads to/from the domain Signed-off-by: John Ferlan <jferlan@redhat.com> --- include/libvirt/libvirt-domain.h | 7 +++ src/driver-hypervisor.h | 13 ++++ src/libvirt-domain.c | 132 +++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ 4 files changed, 158 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7be4219..472258c 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1615,6 +1615,13 @@ int virDomainPinIOThread(virDomainPtr domain, unsigned char *cpumap, int maplen, unsigned int flags); +int virDomainAddIOThread(virDomainPtr domain, + unsigned int iothread_id, + const char *name, + unsigned int flags); +int virDomainDelIOThread(virDomainPtr domain, + unsigned int iothread_id, + unsigned int flags); /** * VIR_USE_CPU: diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 1b92460..283562f 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -393,6 +393,17 @@ typedef int unsigned int flags); typedef int +(*virDrvDomainAddIOThread)(virDomainPtr domain, + unsigned int iothread_id, + const char *name, + unsigned int flags); + +typedef int +(*virDrvDomainDelIOThread)(virDomainPtr domain, + unsigned int iothread_id, + unsigned int flags); + +typedef int (*virDrvDomainGetSecurityLabel)(virDomainPtr domain, virSecurityLabelPtr seclabel); @@ -1273,6 +1284,8 @@ struct _virHypervisorDriver { virDrvDomainGetMaxVcpus domainGetMaxVcpus; virDrvDomainGetIOThreadInfo domainGetIOThreadInfo; virDrvDomainPinIOThread domainPinIOThread; + virDrvDomainAddIOThread domainAddIOThread; + virDrvDomainDelIOThread domainDelIOThread; virDrvDomainGetSecurityLabel domainGetSecurityLabel; virDrvDomainGetSecurityLabelList domainGetSecurityLabelList; virDrvNodeGetSecurityModel nodeGetSecurityModel; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 0acfd13..ffd50b3 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -8020,6 +8020,138 @@ virDomainPinIOThread(virDomainPtr domain, /** + * virDomainAddIOThread: + * @domain: a domain object + * @iothread_id: the specific IOThread ID value to add + * @name: optional additional naming string (NUL terminated) + * @flags: bitwise-OR of virDomainModificationImpact + * + * Dynamically add an IOThread to the domain. If @iothread_id is a positive + * non-zero value, then attempt to add the specific IOThread ID and error + * out if the iothread id already exists. If the @name is NULL, then only + * the default naming scheme is used. Any name containing "iothread" will + * be rejected. + * + * Note that this call can fail if the underlying virtualization hypervisor + * does not support it or if growing the number is arbitrarily limited. + * This function may require privileged access to the hypervisor. + * + * @flags may include VIR_DOMAIN_AFFECT_LIVE to affect a running + * domain (which may fail if domain is not active), or + * VIR_DOMAIN_AFFECT_CONFIG to affect the next boot via the XML + * description of the domain. Both flags may be set. + * If neither flag is specified (that is, @flags is VIR_DOMAIN_AFFECT_CURRENT), + * then an inactive domain modifies persistent setup, while an active domain + * is hypervisor-dependent on whether just live or both live and persistent + * state is changed. + * + * Not all hypervisors can support all flag combinations. + * + * Returns 0 in case of success, -1 in case of failure. + */ +int +virDomainAddIOThread(virDomainPtr domain, + unsigned int iothread_id, + const char *name, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "iothread_id=%u, name=%p flags=%x", + iothread_id, name, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckReadOnlyGoto(domain->conn->flags, error); + + if ((unsigned short) iothread_id != iothread_id) { + virReportError(VIR_ERR_OVERFLOW, _("input too large: %u"), iothread_id); + goto error; + } + conn = domain->conn; + + if (conn->driver->domainAddIOThread) { + int ret; + ret = conn->driver->domainAddIOThread(domain, iothread_id, name, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** + * virDomainDelIOThread: + * @domain: a domain object + * @iothread_id: the specific IOThread ID value to delete + * @flags: bitwise-OR of virDomainModificationImpact + * + * Dynamically delete an IOThread from the domain. The @iothread_id to be + * deleted must not have a resource associated with it and can be any of + * the currently valid IOThread ID's. + * + * Note that this call can fail if the underlying virtualization hypervisor + * does not support it or if reducing the number is arbitrarily limited. + * This function may require privileged access to the hypervisor. + * + * @flags may include VIR_DOMAIN_AFFECT_LIVE to affect a running + * domain (which may fail if domain is not active), or + * VIR_DOMAIN_AFFECT_CONFIG to affect the next boot via the XML + * description of the domain. Both flags may be set. + * If neither flag is specified (that is, @flags is VIR_DOMAIN_AFFECT_CURRENT), + * then an inactive domain modifies persistent setup, while an active domain + * is hypervisor-dependent on whether just live or both live and persistent + * state is changed. + * + * Not all hypervisors can support all flag combinations. + * + * Returns 0 in case of success, -1 in case of failure. + */ +int +virDomainDelIOThread(virDomainPtr domain, + unsigned int iothread_id, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "iothread_id=%u, flags=%x", iothread_id, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckReadOnlyGoto(domain->conn->flags, error); + virCheckNonZeroArgGoto(iothread_id, error); + + if ((unsigned short) iothread_id != iothread_id) { + virReportError(VIR_ERR_OVERFLOW, _("input too large: %u"), iothread_id); + goto error; + } + conn = domain->conn; + + if (conn->driver->domainDelIOThread) { + int ret; + ret = conn->driver->domainDelIOThread(domain, iothread_id, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virDomainGetSecurityLabel: * @domain: a domain object * @seclabel: pointer to a virSecurityLabel structure diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 28347c6..ef3d2f0 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -704,4 +704,10 @@ LIBVIRT_1.2.14 { virDomainInterfaceFree; } LIBVIRT_1.2.12; +LIBVIRT_1.2.15 { + global: + virDomainAddIOThread; + virDomainDelIOThread; +} LIBVIRT_1.2.14; + # .... define new API here using predicted next version number .... -- 2.1.0
On Fri, Apr 10, 2015 at 17:36:24 -0400, John Ferlan wrote:
Add libvirt API's to manage adding and deleting IOThreads to/from the domain
Signed-off-by: John Ferlan <jferlan@redhat.com> --- include/libvirt/libvirt-domain.h | 7 +++ src/driver-hypervisor.h | 13 ++++ src/libvirt-domain.c | 132 +++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ 4 files changed, 158 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7be4219..472258c 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1615,6 +1615,13 @@ int virDomainPinIOThread(virDomainPtr domain, unsigned char *cpumap, int maplen, unsigned int flags); +int virDomainAddIOThread(virDomainPtr domain, + unsigned int iothread_id, + const char *name, + unsigned int flags); +int virDomainDelIOThread(virDomainPtr domain, + unsigned int iothread_id, + unsigned int flags);
/** * VIR_USE_CPU: diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 1b92460..283562f 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -393,6 +393,17 @@ typedef int unsigned int flags);
typedef int +(*virDrvDomainAddIOThread)(virDomainPtr domain, + unsigned int iothread_id, + const char *name, + unsigned int flags); + +typedef int +(*virDrvDomainDelIOThread)(virDomainPtr domain, + unsigned int iothread_id, + unsigned int flags); + +typedef int (*virDrvDomainGetSecurityLabel)(virDomainPtr domain, virSecurityLabelPtr seclabel);
@@ -1273,6 +1284,8 @@ struct _virHypervisorDriver { virDrvDomainGetMaxVcpus domainGetMaxVcpus; virDrvDomainGetIOThreadInfo domainGetIOThreadInfo; virDrvDomainPinIOThread domainPinIOThread; + virDrvDomainAddIOThread domainAddIOThread; + virDrvDomainDelIOThread domainDelIOThread; virDrvDomainGetSecurityLabel domainGetSecurityLabel; virDrvDomainGetSecurityLabelList domainGetSecurityLabelList; virDrvNodeGetSecurityModel nodeGetSecurityModel; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 0acfd13..ffd50b3 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -8020,6 +8020,138 @@ virDomainPinIOThread(virDomainPtr domain,
/** + * virDomainAddIOThread: + * @domain: a domain object + * @iothread_id: the specific IOThread ID value to add + * @name: optional additional naming string (NUL terminated) + * @flags: bitwise-OR of virDomainModificationImpact + * + * Dynamically add an IOThread to the domain. If @iothread_id is a positive + * non-zero value, then attempt to add the specific IOThread ID and error + * out if the iothread id already exists. If the @name is NULL, then only + * the default naming scheme is used. Any name containing "iothread" will + * be rejected. + * + * Note that this call can fail if the underlying virtualization hypervisor + * does not support it or if growing the number is arbitrarily limited. + * This function may require privileged access to the hypervisor.
It requires, not may require.
+ * + * @flags may include VIR_DOMAIN_AFFECT_LIVE to affect a running + * domain (which may fail if domain is not active), or + * VIR_DOMAIN_AFFECT_CONFIG to affect the next boot via the XML + * description of the domain. Both flags may be set. + * If neither flag is specified (that is, @flags is VIR_DOMAIN_AFFECT_CURRENT), + * then an inactive domain modifies persistent setup, while an active domain + * is hypervisor-dependent on whether just live or both live and persistent + * state is changed.
I'd opt for a more sane explanation, where CURRENT with active VM means the live definiton is modified.
+ * + * Not all hypervisors can support all flag combinations.
There are no flags this could potentially apply to yet.
+ * + * Returns 0 in case of success, -1 in case of failure. + */ +int +virDomainAddIOThread(virDomainPtr domain, + unsigned int iothread_id, + const char *name, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "iothread_id=%u, name=%p flags=%x", + iothread_id, name, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckReadOnlyGoto(domain->conn->flags, error); + + if ((unsigned short) iothread_id != iothread_id) { + virReportError(VIR_ERR_OVERFLOW, _("input too large: %u"), iothread_id); + goto error;
Just store it as a full unsigned integer and kill this check.
+ } + conn = domain->conn; + + if (conn->driver->domainAddIOThread) { + int ret; + ret = conn->driver->domainAddIOThread(domain, iothread_id, name, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** + * virDomainDelIOThread: + * @domain: a domain object + * @iothread_id: the specific IOThread ID value to delete + * @flags: bitwise-OR of virDomainModificationImpact + * + * Dynamically delete an IOThread from the domain. The @iothread_id to be + * deleted must not have a resource associated with it and can be any of + * the currently valid IOThread ID's. + * + * Note that this call can fail if the underlying virtualization hypervisor + * does not support it or if reducing the number is arbitrarily limited. + * This function may require privileged access to the hypervisor.
This function requires privileged access.
+ * + * @flags may include VIR_DOMAIN_AFFECT_LIVE to affect a running + * domain (which may fail if domain is not active), or + * VIR_DOMAIN_AFFECT_CONFIG to affect the next boot via the XML + * description of the domain. Both flags may be set. + * If neither flag is specified (that is, @flags is VIR_DOMAIN_AFFECT_CURRENT), + * then an inactive domain modifies persistent setup, while an active domain + * is hypervisor-dependent on whether just live or both live and persistent + * state is changed. + * + * Not all hypervisors can support all flag combinations.
... see above.
+ * + * Returns 0 in case of success, -1 in case of failure. + */ +int +virDomainDelIOThread(virDomainPtr domain, + unsigned int iothread_id, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "iothread_id=%u, flags=%x", iothread_id, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckReadOnlyGoto(domain->conn->flags, error); + virCheckNonZeroArgGoto(iothread_id, error); + + if ((unsigned short) iothread_id != iothread_id) { + virReportError(VIR_ERR_OVERFLOW, _("input too large: %u"), iothread_id); + goto error;
See above.
+ } + conn = domain->conn; + + if (conn->driver->domainDelIOThread) { + int ret; + ret = conn->driver->domainDelIOThread(domain, iothread_id, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virDomainGetSecurityLabel: * @domain: a domain object * @seclabel: pointer to a virSecurityLabel structure
Peter
Add remote support for the add/delete IOThread API's Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/remote/remote_driver.c | 2 ++ src/remote/remote_protocol.x | 31 ++++++++++++++++++++++++++++++- src/remote_protocol-structs | 13 +++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b275d86..b2b5a13 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8210,6 +8210,8 @@ static virHypervisorDriver hypervisor_driver = { .domainGetMaxVcpus = remoteDomainGetMaxVcpus, /* 0.3.0 */ .domainGetIOThreadInfo = remoteDomainGetIOThreadInfo, /* 1.2.14 */ .domainPinIOThread = remoteDomainPinIOThread, /* 1.2.14 */ + .domainAddIOThread = remoteDomainAddIOThread, /* 1.2.15 */ + .domainDelIOThread = remoteDomainDelIOThread, /* 1.2.15 */ .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */ .domainGetSecurityLabelList = remoteDomainGetSecurityLabelList, /* 0.10.0 */ .nodeGetSecurityModel = remoteNodeGetSecurityModel, /* 0.6.1 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index d90e6b5..4b9e815 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1212,6 +1212,19 @@ struct remote_domain_pin_iothread_args { unsigned int flags; }; +struct remote_domain_add_iothread_args { + remote_nonnull_domain dom; + unsigned int iothread_id; + remote_string name; + unsigned int flags; +}; + +struct remote_domain_del_iothread_args { + remote_nonnull_domain dom; + unsigned int iothread_id; + unsigned int flags; +}; + struct remote_domain_get_security_label_args { remote_nonnull_domain dom; }; @@ -5643,5 +5656,21 @@ enum remote_procedure { * @generate: none * @acl: domain:read */ - REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353 + REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353, + + /** + * @generate:both + * @acl: domain:write + * @acl: domain:save:!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE + * @acl: domain:save:VIR_DOMAIN_AFFECT_CONFIG + */ + REMOTE_PROC_DOMAIN_ADD_IOTHREAD = 354, + + /** + * @generate:both + * @acl: domain:write + * @acl: domain:save:!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE + * @acl: domain:save:VIR_DOMAIN_AFFECT_CONFIG + */ + REMOTE_PROC_DOMAIN_DEL_IOTHREAD = 355 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e614f77..b4e4756 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -834,6 +834,17 @@ struct remote_domain_pin_iothread_args { } cpumap; u_int flags; }; +struct remote_domain_add_iothread_args { + remote_nonnull_domain dom; + u_int iothread_id; + remote_string name; + u_int flags; +}; +struct remote_domain_del_iothread_args { + remote_nonnull_domain dom; + u_int iothread_id; + u_int flags; +}; struct remote_domain_get_security_label_args { remote_nonnull_domain dom; }; @@ -3017,4 +3028,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_IOTHREAD_INFO = 351, REMOTE_PROC_DOMAIN_PIN_IOTHREAD = 352, REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353, + REMOTE_PROC_DOMAIN_ADD_IOTHREAD = 354, + REMOTE_PROC_DOMAIN_DEL_IOTHREAD = 355, }; -- 2.1.0
On Fri, Apr 10, 2015 at 17:36:25 -0400, John Ferlan wrote:
Add remote support for the add/delete IOThread API's
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/remote/remote_driver.c | 2 ++ src/remote/remote_protocol.x | 31 ++++++++++++++++++++++++++++++- src/remote_protocol-structs | 13 +++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-)
Looks good to me. I'd leave this one open until v3 for other people to chime in. Peter
Add qemuDomainAddIOThread and qemuDomainDelIOThread in order to add or remove an IOThread to/from the host either for live or config optoins The implementation for the 'live' option will use the iothreadpids list in order to make decision, while the 'config' option will use the iothreadids list. Additionally, for deletion each may have to adjust the iothreadpin list. IOThreads are implemented by qmp objects, the code makes use of the existing qemuMonitorAddObject or qemuMonitorDelObject APIs. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_audit.c | 9 + src/conf/domain_audit.h | 6 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 432 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 448 insertions(+) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 159ebf5..92fcdd3 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -790,6 +790,15 @@ virDomainAuditVcpu(virDomainObjPtr vm, return virDomainAuditResource(vm, "vcpu", oldvcpu, newvcpu, reason, success); } +void +virDomainAuditIOThread(virDomainObjPtr vm, + unsigned int oldiothread, unsigned int newiothread, + const char *reason, bool success) +{ + return virDomainAuditResource(vm, "iothread", oldiothread, newiothread, + reason, success); +} + static void virDomainAuditLifecycle(virDomainObjPtr vm, const char *op, const char *reason, bool success) diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 4c1ef90..97dadca 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -102,6 +102,12 @@ void virDomainAuditVcpu(virDomainObjPtr vm, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); +void virDomainAuditIOThread(virDomainObjPtr vm, + unsigned int oldiothread, + unsigned int newiothread, + const char *reason, + bool success) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); void virDomainAuditSecurityLabel(virDomainObjPtr vm, bool success) ATTRIBUTE_NONNULL(1); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7e55aa0..e4629b1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -129,6 +129,7 @@ virDomainAuditDisk; virDomainAuditFS; virDomainAuditHostdev; virDomainAuditInit; +virDomainAuditIOThread; virDomainAuditMemory; virDomainAuditNet; virDomainAuditNetDevice; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d99f886..5b0784f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6186,6 +6186,436 @@ qemuDomainPinIOThread(virDomainPtr dom, return ret; } +static int +qemuDomainHotplugIOThread(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int iothread_id, + const char *name, + bool add) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + char *alias = NULL; + size_t i, idx; + int rc = -1; + int ret = -1; + unsigned int orig_niothreads = vm->def->iothreads; + unsigned int exp_niothreads = vm->def->iothreads; + int new_niothreads = 0; + qemuMonitorIOThreadInfoPtr *new_iothreads = NULL; + virCgroupPtr cgroup_iothread = NULL; + char *mem_mask = NULL; + + /* Let's see if we can find this iothread_id in our iothreadpids list + * For add finding the same iothread_id will cause a failure since we + * cannot add the same iothread_id twice. + * For del finding our iothread_id is good since we cannot delete + * something that doesn't exist + */ + for (idx = 0; idx < priv->niothreadpids; idx++) { + if (iothread_id == priv->iothreadpids[idx]->iothread_id) + break; + } + + if (add) { + if (idx < priv->niothreadpids) { + virReportError(VIR_ERR_INVALID_ARG, + _("an IOThread is already using iothread_id " + "'%u' in iothreadpids"), + iothread_id); + goto cleanup; + } + } else { + if (idx == priv->niothreadpids) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot find IOThread '%u' in iothreadpids"), + iothread_id); + goto cleanup; + } + + /* The qemuDomainDelThread doesn't pass (or need to pass) the + * name. So we'll grab it here so that we can formulate the + * correct alias for qemuMonitorDelObject to find this object. + */ + name = priv->iothreadpids[idx]->name; + } + + /* Generate alias */ + if (name) { + if (virAsprintf(&alias, "%s_iothread%u", name, iothread_id) < 0) + return -1; + } else { + if (virAsprintf(&alias, "iothread%u", iothread_id) < 0) + return -1; + } + + qemuDomainObjEnterMonitor(driver, vm); + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change IOThreads for an inactive domain")); + goto exit_monitor; + } + + if (add) { + rc = qemuMonitorAddObject(priv->mon, "iothread", alias, NULL); + exp_niothreads++; + } else { + rc = qemuMonitorDelObject(priv->mon, alias); + exp_niothreads--; + } + + if (rc < 0) + goto exit_monitor; + + /* After hotplugging the IOThreads we need to re-detect the + * IOThreads thread_id's, adjust the cgroups, thread affinity, + * and the priv->iothreadpids list. + */ + if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon, + &new_iothreads)) < 0) { + virResetLastError(); + goto exit_monitor; + } + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + + /* ohhh something went wrong */ + if (new_niothreads != exp_niothreads) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("got wrong number of IOThread ids from QEMU monitor. " + "got %d, wanted %d"), + new_niothreads, exp_niothreads); + vm->def->iothreads = new_niothreads; + goto cleanup; + } + vm->def->iothreads = exp_niothreads; + + if (virDomainNumatuneGetMode(vm->def->numa, -1) == + VIR_DOMAIN_NUMATUNE_MEM_STRICT && + virDomainNumatuneMaybeFormatNodeset(vm->def->numa, + priv->autoNodeset, + &mem_mask, -1) < 0) + goto cleanup; + + + if (add) { + qemuDomainIOThreadInfoPtr info; + unsigned int idval = 0; + char *idname = NULL; + int thread_id; + + /* + * If we've successfully added an IOThread, find out where we added it + * in the new IOThread list, so we can add it to our iothreadpids list + */ + for (i = 0; i < new_niothreads; i++) { + + if (qemuDomainParseIOThreadAlias(new_iothreads[i]->name, + &idval, &idname) < 0) + goto cleanup; + + if (iothread_id == idval) + break; + + VIR_FREE(idname); + } + + /* something else went wrong */ + if (idval != iothread_id) { + VIR_FREE(idname); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find new IOThread '%u' in QEMU monitor."), + iothread_id); + goto cleanup; + } + thread_id = new_iothreads[i]->thread_id; + + if (VIR_ALLOC(info) < 0) { + VIR_FREE(idname); + goto cleanup; + } + info->thread_id = thread_id; + info->iothread_id = iothread_id; + info->name = idname; + if (VIR_APPEND_ELEMENT(priv->iothreadpids, + priv->niothreadpids, info) < 0) { + VIR_FREE(info->name); + VIR_FREE(info); + goto cleanup; + } + + /* Add IOThread to cgroup if present */ + if (priv->cgroup) { + cgroup_iothread = + qemuDomainAddCgroupForThread(priv->cgroup, + VIR_CGROUP_THREAD_IOTHREAD, + iothread_id, mem_mask, thread_id); + if (!cgroup_iothread) + // Do we remove from iothreadpids? + goto cleanup; + } + + /* Inherit def->cpuset */ + if (vm->def->cpumask) { + if (qemuDomainHotplugAddPin(vm->def->cpumask, iothread_id, + &vm->def->cputune.iothreadspin, + &vm->def->cputune.niothreadspin) < 0) + + // Do we remove from iothreadpids && scratch the cgroup? + goto cleanup; + + if (qemuDomainHotplugPinThread(vm->def->cpumask, iothread_id, + thread_id, cgroup_iothread) < 0) + // Do we remove from iothreadpids, iothreadspin, and cgroup + goto cleanup; + + if (qemuProcessSetSchedParams(iothread_id, thread_id, + vm->def->cputune.niothreadsched, + vm->def->cputune.iothreadsched) < 0) + goto cleanup; + } + } else { + + /* Remove our iothread_id from the list of iothreadpid's */ + if (VIR_DELETE_ELEMENT(priv->iothreadpids, idx, + priv->niothreadpids) < 0) + goto cleanup; + + /* Remove the cgroup links */ + if (qemuDomainDelCgroupForThread(priv->cgroup, + VIR_CGROUP_THREAD_IOTHREAD, + iothread_id) < 0) + goto cleanup; + + /* Free pin setting */ + virDomainPinDel(&vm->def->cputune.iothreadspin, + &vm->def->cputune.niothreadspin, + iothread_id); + } + + ret = 0; + + cleanup: + if (new_iothreads) { + for (i = 0; i < new_niothreads; i++) + qemuMonitorIOThreadInfoFree(new_iothreads[i]); + VIR_FREE(new_iothreads); + } + VIR_FREE(mem_mask); + virDomainAuditIOThread(vm, orig_niothreads, new_niothreads, + "update", rc == 0); + if (cgroup_iothread) + virCgroupFree(&cgroup_iothread); + VIR_FREE(alias); + return ret; + + exit_monitor: + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + goto cleanup; +} + +static int +qemuDomainChgIOThread(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int iothread_id, + const char *name, + bool add, + unsigned int flags) +{ + virQEMUDriverConfigPtr cfg = NULL; + virCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv; + virCgroupPtr cgroup_temp = NULL; + virBitmapPtr all_nodes = NULL; + char *all_nodes_str = NULL; + char *mem_mask = NULL; + virDomainDefPtr persistentDef; + size_t i; + int ret = -1; + + if ((unsigned short) iothread_id != iothread_id) { + virReportError(VIR_ERR_INVALID_ARG, + _("argument out of range: %d"), iothread_id); + return -1; + } + + + cfg = virQEMUDriverGetConfig(driver); + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + + if (!add) { + /* If there is a disk using the IOThread to be removed, then fail. */ + for (i = 0; i < vm->def->ndisks; i++) { + if (vm->def->disks[i]->iothread == iothread_id) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot remove IOThread %u since it " + "is being used by disk path '%s'"), + iothread_id, + NULLSTR(vm->def->disks[i]->src->path)); + goto cleanup; + } + } + } + + priv = vm->privateData; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, + false, &cgroup_temp) < 0) + goto endjob; + + if (!(all_nodes = virNumaGetHostNodeset())) + goto endjob; + + if (!(all_nodes_str = virBitmapFormat(all_nodes))) + goto endjob; + + if (virCgroupGetCpusetMems(cgroup_temp, &mem_mask) < 0 || + virCgroupSetCpusetMems(cgroup_temp, all_nodes_str) < 0) + goto endjob; + } + + if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, + &persistentDef) < 0) + goto endjob; + + /* For a live change - let's make sure the binary supports this */ + if (flags & VIR_DOMAIN_AFFECT_LIVE && + !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not supported with this binary")); + goto endjob; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + + if (qemuDomainHotplugIOThread(driver, vm, iothread_id, name, add) < 0) + goto endjob; + + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + goto endjob; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + virDomainIOThreadIDDefPtr iddef; + + iddef = virDomainIOThreadIDFind(persistentDef, iothread_id); + if (add) { + /* Already there? Then error since we cannot have the same + * iothread_id listed twice + */ + if (iddef) { + virReportError(VIR_ERR_INVALID_ARG, + _("an IOThread is already using iothread_id " + "'%u' in persistent iothreadids"), + iothread_id); + goto endjob; + } + + if (virDomainIOThreadIDAdd(persistentDef, iothread_id, name) < 0) + goto endjob; + + /* Nothing to do in iothreadspin list (that's a separate command) */ + + persistentDef->iothreads++; + } else { + if (!iddef) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot find IOThread '%u' in persistent " + "iothreadids"), + iothread_id); + goto cleanup; + } + + virDomainIOThreadIDDel(persistentDef, iothread_id); + virDomainPinDel(&persistentDef->cputune.iothreadspin, + &persistentDef->cputune.niothreadspin, + iothread_id); + persistentDef->iothreads--; + } + + if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0) + goto endjob; + } + + ret = 0; + + endjob: + if (mem_mask) { + virErrorPtr err = virSaveLastError(); + virCgroupSetCpusetMems(cgroup_temp, mem_mask); + virSetError(err); + virFreeError(err); + } + qemuDomainObjEndJob(driver, vm); + + cleanup: + VIR_FREE(mem_mask); + VIR_FREE(all_nodes_str); + virBitmapFree(all_nodes); + virCgroupFree(&cgroup_temp); + virObjectUnref(caps); + virObjectUnref(cfg); + return ret; +} + +static int +qemuDomainAddIOThread(virDomainPtr dom, + unsigned int iothread_id, + const char *name, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainAddIOThreadEnsureACL(dom->conn, vm->def, flags) < 0) + goto cleanup; + + ret = qemuDomainChgIOThread(driver, vm, iothread_id, name, true, flags); + + cleanup: + qemuDomObjEndAPI(&vm); + return ret; +} + + +static int +qemuDomainDelIOThread(virDomainPtr dom, + unsigned int iothread_id, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainDelIOThreadEnsureACL(dom->conn, vm->def, flags) < 0) + goto cleanup; + ret = qemuDomainChgIOThread(driver, vm, iothread_id, NULL, false, flags); + + cleanup: + qemuDomObjEndAPI(&vm); + return ret; +} + static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr seclabel) { virQEMUDriverPtr driver = dom->conn->privateData; @@ -19863,6 +20293,8 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetMaxVcpus = qemuDomainGetMaxVcpus, /* 0.4.4 */ .domainGetIOThreadInfo = qemuDomainGetIOThreadInfo, /* 1.2.14 */ .domainPinIOThread = qemuDomainPinIOThread, /* 1.2.14 */ + .domainAddIOThread = qemuDomainAddIOThread, /* 1.2.15 */ + .domainDelIOThread = qemuDomainDelIOThread, /* 1.2.15 */ .domainGetSecurityLabel = qemuDomainGetSecurityLabel, /* 0.6.1 */ .domainGetSecurityLabelList = qemuDomainGetSecurityLabelList, /* 0.10.0 */ .nodeGetSecurityModel = qemuNodeGetSecurityModel, /* 0.6.1 */ -- 2.1.0
On Fri, Apr 10, 2015 at 17:36:26 -0400, John Ferlan wrote:
Add qemuDomainAddIOThread and qemuDomainDelIOThread in order to add or remove an IOThread to/from the host either for live or config optoins
The implementation for the 'live' option will use the iothreadpids list in order to make decision, while the 'config' option will use the iothreadids list. Additionally, for deletion each may have to adjust the iothreadpin list.
IOThreads are implemented by qmp objects, the code makes use of the existing qemuMonitorAddObject or qemuMonitorDelObject APIs.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_audit.c | 9 + src/conf/domain_audit.h | 6 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 432 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 448 insertions(+)
...
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d99f886..5b0784f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6186,6 +6186,436 @@ qemuDomainPinIOThread(virDomainPtr dom, return ret; }
+static int +qemuDomainHotplugIOThread(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int iothread_id, + const char *name, + bool add) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + char *alias = NULL; + size_t i, idx; + int rc = -1; + int ret = -1; + unsigned int orig_niothreads = vm->def->iothreads; + unsigned int exp_niothreads = vm->def->iothreads; + int new_niothreads = 0; + qemuMonitorIOThreadInfoPtr *new_iothreads = NULL; + virCgroupPtr cgroup_iothread = NULL; + char *mem_mask = NULL; + + /* Let's see if we can find this iothread_id in our iothreadpids list + * For add finding the same iothread_id will cause a failure since we + * cannot add the same iothread_id twice. + * For del finding our iothread_id is good since we cannot delete + * something that doesn't exist + */
The comment states obvious facts.
+ for (idx = 0; idx < priv->niothreadpids; idx++) { + if (iothread_id == priv->iothreadpids[idx]->iothread_id) + break; + } + + if (add) { + if (idx < priv->niothreadpids) { + virReportError(VIR_ERR_INVALID_ARG, + _("an IOThread is already using iothread_id " + "'%u' in iothreadpids"), + iothread_id); + goto cleanup; + } + } else { + if (idx == priv->niothreadpids) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot find IOThread '%u' in iothreadpids"), + iothread_id); + goto cleanup; + } + + /* The qemuDomainDelThread doesn't pass (or need to pass) the + * name. So we'll grab it here so that we can formulate the + * correct alias for qemuMonitorDelObject to find this object. + */
With the changes I've suggested this won't be necessary
+ name = priv->iothreadpids[idx]->name; + } + + /* Generate alias */ + if (name) { + if (virAsprintf(&alias, "%s_iothread%u", name, iothread_id) < 0) + return -1; + } else { + if (virAsprintf(&alias, "iothread%u", iothread_id) < 0) + return -1; + }
Neither this.
+ + qemuDomainObjEnterMonitor(driver, vm); + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change IOThreads for an inactive domain")); + goto exit_monitor; + }
This is a bit too late to check if the domain is active.
+ + if (add) { + rc = qemuMonitorAddObject(priv->mon, "iothread", alias, NULL); + exp_niothreads++; + } else { + rc = qemuMonitorDelObject(priv->mon, alias); + exp_niothreads--; + } + + if (rc < 0) + goto exit_monitor; + + /* After hotplugging the IOThreads we need to re-detect the + * IOThreads thread_id's, adjust the cgroups, thread affinity, + * and the priv->iothreadpids list. + */ + if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon, + &new_iothreads)) < 0) { + virResetLastError(); + goto exit_monitor;
In this case you'd report an empty error as exit_monitor leads to returning -1 without reporting any new one.
+ } + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + + /* ohhh something went wrong */
Obvious.
+ if (new_niothreads != exp_niothreads) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("got wrong number of IOThread ids from QEMU monitor. " + "got %d, wanted %d"), + new_niothreads, exp_niothreads); + vm->def->iothreads = new_niothreads; + goto cleanup; + } + vm->def->iothreads = exp_niothreads; + + if (virDomainNumatuneGetMode(vm->def->numa, -1) == + VIR_DOMAIN_NUMATUNE_MEM_STRICT && + virDomainNumatuneMaybeFormatNodeset(vm->def->numa, + priv->autoNodeset, + &mem_mask, -1) < 0) + goto cleanup;
We really should store the above data somewhere. I've seen the above construct a few times in different places lately.
+ + + if (add) { + qemuDomainIOThreadInfoPtr info; + unsigned int idval = 0; + char *idname = NULL; + int thread_id; + + /* + * If we've successfully added an IOThread, find out where we added it + * in the new IOThread list, so we can add it to our iothreadpids list + */ + for (i = 0; i < new_niothreads; i++) { + + if (qemuDomainParseIOThreadAlias(new_iothreads[i]->name, + &idval, &idname) < 0) + goto cleanup;
You already know the data as you've formated it a few lines above.
+ + if (iothread_id == idval) + break; + + VIR_FREE(idname); + } + + /* something else went wrong */
...
+ if (idval != iothread_id) { + VIR_FREE(idname); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find new IOThread '%u' in QEMU monitor."), + iothread_id); + goto cleanup; + } + thread_id = new_iothreads[i]->thread_id; + + if (VIR_ALLOC(info) < 0) { + VIR_FREE(idname); + goto cleanup; + } + info->thread_id = thread_id; + info->iothread_id = iothread_id; + info->name = idname; + if (VIR_APPEND_ELEMENT(priv->iothreadpids, + priv->niothreadpids, info) < 0) { + VIR_FREE(info->name); + VIR_FREE(info); + goto cleanup; + } + + /* Add IOThread to cgroup if present */ + if (priv->cgroup) { + cgroup_iothread = + qemuDomainAddCgroupForThread(priv->cgroup, + VIR_CGROUP_THREAD_IOTHREAD, + iothread_id, mem_mask, thread_id); + if (!cgroup_iothread) + // Do we remove from iothreadpids?
The coding guidelines state that the old style comments should be used rather than //
+ goto cleanup; + } + + /* Inherit def->cpuset */ + if (vm->def->cpumask) { + if (qemuDomainHotplugAddPin(vm->def->cpumask, iothread_id, + &vm->def->cputune.iothreadspin, + &vm->def->cputune.niothreadspin) < 0) + + // Do we remove from iothreadpids && scratch the cgroup?
...
+ goto cleanup; + + if (qemuDomainHotplugPinThread(vm->def->cpumask, iothread_id, + thread_id, cgroup_iothread) < 0) + // Do we remove from iothreadpids, iothreadspin, and cgroup
...
+ goto cleanup; + + if (qemuProcessSetSchedParams(iothread_id, thread_id, + vm->def->cputune.niothreadsched, + vm->def->cputune.iothreadsched) < 0) + goto cleanup; + } + } else { + + /* Remove our iothread_id from the list of iothreadpid's */ + if (VIR_DELETE_ELEMENT(priv->iothreadpids, idx, + priv->niothreadpids) < 0) + goto cleanup; + + /* Remove the cgroup links */ + if (qemuDomainDelCgroupForThread(priv->cgroup, + VIR_CGROUP_THREAD_IOTHREAD, + iothread_id) < 0) + goto cleanup; + + /* Free pin setting */ + virDomainPinDel(&vm->def->cputune.iothreadspin, + &vm->def->cputune.niothreadspin, + iothread_id);
I think it would be preferable to split this function into two, one for adding and one for removing.
+ } + + ret = 0; + + cleanup: + if (new_iothreads) { + for (i = 0; i < new_niothreads; i++) + qemuMonitorIOThreadInfoFree(new_iothreads[i]); + VIR_FREE(new_iothreads); + } + VIR_FREE(mem_mask); + virDomainAuditIOThread(vm, orig_niothreads, new_niothreads, + "update", rc == 0); + if (cgroup_iothread) + virCgroupFree(&cgroup_iothread); + VIR_FREE(alias); + return ret; + + exit_monitor: + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + goto cleanup; +} + +static int +qemuDomainChgIOThread(virQEMUDriverPtr driver, + virDomainObjPtr vm, + unsigned int iothread_id, + const char *name, + bool add, + unsigned int flags) +{ + virQEMUDriverConfigPtr cfg = NULL; + virCapsPtr caps = NULL; + qemuDomainObjPrivatePtr priv; + virCgroupPtr cgroup_temp = NULL; + virBitmapPtr all_nodes = NULL; + char *all_nodes_str = NULL; + char *mem_mask = NULL; + virDomainDefPtr persistentDef; + size_t i; + int ret = -1; + + if ((unsigned short) iothread_id != iothread_id) { + virReportError(VIR_ERR_INVALID_ARG, + _("argument out of range: %d"), iothread_id);
Again, please store it as an int and kill this code.
+ return -1; + } + + + cfg = virQEMUDriverGetConfig(driver); + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + + if (!add) { + /* If there is a disk using the IOThread to be removed, then fail. */ + for (i = 0; i < vm->def->ndisks; i++) { + if (vm->def->disks[i]->iothread == iothread_id) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot remove IOThread %u since it " + "is being used by disk path '%s'"), + iothread_id, + NULLSTR(vm->def->disks[i]->src->path)); + goto cleanup; + } + } + } + + priv = vm->privateData; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) {
This should be after virDomainLiveConfigHelperMethod since it updates @flags.
+ if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, + false, &cgroup_temp) < 0) + goto endjob; + + if (!(all_nodes = virNumaGetHostNodeset())) + goto endjob; + + if (!(all_nodes_str = virBitmapFormat(all_nodes))) + goto endjob; + + if (virCgroupGetCpusetMems(cgroup_temp, &mem_mask) < 0 || + virCgroupSetCpusetMems(cgroup_temp, all_nodes_str) < 0) + goto endjob; + } + + if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, + &persistentDef) < 0) + goto endjob; + + /* For a live change - let's make sure the binary supports this */ + if (flags & VIR_DOMAIN_AFFECT_LIVE && + !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOThreads not supported with this binary")); + goto endjob;
The inactive config will fail if qemu doesn't support iothreads, won't it?
+ } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + + if (qemuDomainHotplugIOThread(driver, vm, iothread_id, name, add) < 0) + goto endjob; + + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + goto endjob; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
Having the config section appear before the LIVE section will make it less prone to return failure but the thread being actually added.
+ virDomainIOThreadIDDefPtr iddef; + + iddef = virDomainIOThreadIDFind(persistentDef, iothread_id); + if (add) { + /* Already there? Then error since we cannot have the same + * iothread_id listed twice + */ + if (iddef) { + virReportError(VIR_ERR_INVALID_ARG, + _("an IOThread is already using iothread_id " + "'%u' in persistent iothreadids"), + iothread_id); + goto endjob; + } + + if (virDomainIOThreadIDAdd(persistentDef, iothread_id, name) < 0) + goto endjob; + + /* Nothing to do in iothreadspin list (that's a separate command) */ + + persistentDef->iothreads++; + } else { + if (!iddef) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot find IOThread '%u' in persistent " + "iothreadids"), + iothread_id); + goto cleanup; + } + + virDomainIOThreadIDDel(persistentDef, iothread_id); + virDomainPinDel(&persistentDef->cputune.iothreadspin, + &persistentDef->cputune.niothreadspin, + iothread_id); + persistentDef->iothreads--; + } + + if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0) + goto endjob; + } + + ret = 0; + + endjob: + if (mem_mask) { + virErrorPtr err = virSaveLastError(); + virCgroupSetCpusetMems(cgroup_temp, mem_mask); + virSetError(err); + virFreeError(err); + } + qemuDomainObjEndJob(driver, vm); + + cleanup: + VIR_FREE(mem_mask); + VIR_FREE(all_nodes_str); + virBitmapFree(all_nodes); + virCgroupFree(&cgroup_temp); + virObjectUnref(caps); + virObjectUnref(cfg); + return ret; +} +
Peter
https://bugzilla.redhat.com/show_bug.cgi?id=1161617 Add command to allow adding and removing IOThreads from the domain including the configuration and live domain. $ virsh iothreadadd --help NAME iothreadadd - add an IOThread to the guest domain SYNOPSIS iothreadadd <domain> <id> [--name <string>] [--config] [--live] [--current] DESCRIPTION Add an IOThread to the guest domain. OPTIONS [--domain] <string> domain name, id or uuid [--id] <number> iothread for the new IOThread --name <string> a name for the new IOThread --config affect next boot --live affect running domain --current affect current domain $ virsh iothreaddel --help NAME iothreaddel - delete an IOThread from the guest domain SYNOPSIS iothreaddel <domain> <id> [--config] [--live] [--current] DESCRIPTION Delete an IOThread from the guest domain. OPTIONS [--domain] <string> domain name, id or uuid [--id] <number> iothread_id for the IOThread to delete --config affect next boot --live affect running domain --current affect current domain Assuming a running $dom with multiple IOThreads assigned and that that the $dom has disks assigned to IOThread 1 and IOThread 2: $ virsh iothreadinfo $dom IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1 $ virsh iothreadadd $dom 1 error: invalid argument: an IOThread is already using iothread_id '1' in iothreadpids $ virsh iothreadadd $dom 1 --config error: invalid argument: an IOThread is already using iothread_id '1' in persistent iothreadids $ virsh iothreadadd $dom 4 $ virsh iothreadinfo $dom IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1 4 0-3 $ virsh iothreadinfo $dom --config IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1 $ virsh iothreadadd $dom 4 --config $ virsh iothreadinfo $dom --config IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1 4 0-3 $ virsh iothreadadd $dom 5 userdisk $ virsh qemu-monitor-command $dom '{"execute":"query-iothreads"}' {"return":[{"thread-id":17889,"id":"iothread1"},{"thread-id":17890,"id":"iothread2"},{"thread-id":17892,"id":"iothread3"},{"thread-id":17893,"id":"iothread4"},{"thread-id":18108,"id":"userdisk_iothread5"}],"id":"libvirt-104"} $ virsh iothreaddel $dom 5 userdisk Assuming the same original configuration $ virsh iothreaddel $dom 1 error: invalid argument: cannot remove IOThread 1 since it is being used by disk path '/home/vm-images/f18' $ virsh iothreaddel $dom 3 $ virsh iothreadinfo $dom IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 $ virsh iothreadinfo $dom --config IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1 Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh-domain.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 32 ++++++++++ 2 files changed, 206 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 928360c..37836ce 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6977,6 +6977,168 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd) } /* + * "iothreadadd" command + */ +static const vshCmdInfo info_iothreadadd[] = { + {.name = "help", + .data = N_("add an IOThread to the guest domain") + }, + {.name = "desc", + .data = N_("Add an IOThread to the guest domain.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_iothreadadd[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "id", + .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ, + .help = N_("iothread for the new IOThread") + }, + {.name = "name", + .type = VSH_OT_STRING, + .help = N_("a name for the new IOThread") + }, + {.name = "config", + .type = VSH_OT_BOOL, + .help = N_("affect next boot") + }, + {.name = "live", + .type = VSH_OT_BOOL, + .help = N_("affect running domain") + }, + {.name = "current", + .type = VSH_OT_BOOL, + .help = N_("affect current domain") + }, + {.name = NULL} +}; + +static bool +cmdIOThreadAdd(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + int iothread_id = 0; + const char *name_option = NULL; + bool ret = false; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptInt(cmd, "id", &iothread_id) < 0 || iothread_id <= 0) { + vshError(ctl, _("Invalid IOThread id value: '%d'"), iothread_id); + goto cleanup; + } + + if (vshCommandOptStringReq(ctl, cmd, "name", &name_option) < 0) { + vshError(ctl, "%s", _("Unable to parse 'name' parameter")); + goto cleanup; + } + + if (virDomainAddIOThread(dom, iothread_id, name_option, flags) < 0) + goto cleanup; + + ret = true; + + cleanup: + virDomainFree(dom); + return ret; +} + +/* + * "iothreaddel" command + */ +static const vshCmdInfo info_iothreaddel[] = { + {.name = "help", + .data = N_("delete an IOThread from the guest domain") + }, + {.name = "desc", + .data = N_("Delete an IOThread from the guest domain.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_iothreaddel[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "id", + .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ, + .help = N_("iothread_id for the IOThread to delete") + }, + {.name = "config", + .type = VSH_OT_BOOL, + .help = N_("affect next boot") + }, + {.name = "live", + .type = VSH_OT_BOOL, + .help = N_("affect running domain") + }, + {.name = "current", + .type = VSH_OT_BOOL, + .help = N_("affect current domain") + }, + {.name = NULL} +}; + +static bool +cmdIOThreadDel(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + int iothread_id = 0; + bool ret = false; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptInt(cmd, "id", &iothread_id) < 0 || iothread_id <= 0) { + vshError(ctl, _("Invalid IOThread id value: '%d'"), iothread_id); + goto cleanup; + } + + if (virDomainDelIOThread(dom, iothread_id, flags) < 0) + goto cleanup; + + ret = true; + + cleanup: + virDomainFree(dom); + return ret; +} + +/* * "cpu-compare" command */ static const vshCmdInfo info_cpu_compare[] = { @@ -12906,6 +13068,18 @@ const vshCmdDef domManagementCmds[] = { .info = info_iothreadpin, .flags = 0 }, + {.name = "iothreadadd", + .handler = cmdIOThreadAdd, + .opts = opts_iothreadadd, + .info = info_iothreadadd, + .flags = 0 + }, + {.name = "iothreaddel", + .handler = cmdIOThreadDel, + .opts = opts_iothreaddel, + .info = info_iothreaddel, + .flags = 0 + }, {.name = "send-key", .handler = cmdSendKey, .opts = opts_send_key, diff --git a/tools/virsh.pod b/tools/virsh.pod index d642a69..6ea57f1 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1416,6 +1416,38 @@ If no flag is specified, behavior is different depending on hypervisor. B<Note>: The expression is sequentially evaluated, so "0-15,^8" is identical to "9-14,0-7,15" but not identical to "^8,0-15". +=item B<iothreadadd> I<domain> I<iothread_id> [I<name>] +[[I<--config>] [I<--live>] | [I<--current>]] + +Add a new IOThread to the domain using the specified I<iothread_id>. +Optionally a I<name> may be provided to further describe the IOThread. +If the I<iothread_id> already exists, the command will fail. The +I<iothread_id> must be greater than zero. + +If I<--live> is specified, affect a running guest. If the guest is not +running an error is returned. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified or I<--live> and I<--config> are not specified, +affect the current guest state. +Both I<--live> and I<--config> flags may be given, but if not flag is +specified, behavior is different depending on hypervisor. + +=item B<iothreaddel> I<domain> I<iothread_id> +[[I<--config>] [I<--live>] | [I<--current>]] + +Delete an IOThread from the domain using the specified I<iothread_id>. +If an IOThread is currently assigned to a disk resource such as via the +B<attach-disk> command, then the attempt to remove the IOThread will fail. +If the I<iothread_id> does not exist an error will occur. + +If I<--live> is specified, affect a running guest. If the guest is not +running an error is returned. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified or I<--live> and I<--config> are not specified, +affect the current guest state. +Both I<--live> and I<--config> flags may be given, but if not flag is +specified, behavior is different depending on hypervisor. + =item B<managedsave> I<domain> [I<--bypass-cache>] [{I<--running> | I<--paused>}] [I<--verbose>] -- 2.1.0
On Fri, Apr 10, 2015 at 17:36:27 -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1161617
Add command to allow adding and removing IOThreads from the domain including the configuration and live domain.
$ virsh iothreadadd --help NAME iothreadadd - add an IOThread to the guest domain
SYNOPSIS iothreadadd <domain> <id> [--name <string>] [--config] [--live] [--current]
DESCRIPTION Add an IOThread to the guest domain.
OPTIONS [--domain] <string> domain name, id or uuid [--id] <number> iothread for the new IOThread --name <string> a name for the new IOThread --config affect next boot --live affect running domain --current affect current domain
$ virsh iothreaddel --help NAME iothreaddel - delete an IOThread from the guest domain
SYNOPSIS iothreaddel <domain> <id> [--config] [--live] [--current]
DESCRIPTION Delete an IOThread from the guest domain.
OPTIONS [--domain] <string> domain name, id or uuid [--id] <number> iothread_id for the IOThread to delete --config affect next boot --live affect running domain --current affect current domain
Assuming a running $dom with multiple IOThreads assigned and that that the $dom has disks assigned to IOThread 1 and IOThread 2:
$ virsh iothreadinfo $dom IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1
$ virsh iothreadadd $dom 1 error: invalid argument: an IOThread is already using iothread_id '1' in iothreadpids
$ virsh iothreadadd $dom 1 --config error: invalid argument: an IOThread is already using iothread_id '1' in persistent iothreadids
$ virsh iothreadadd $dom 4 $ virsh iothreadinfo $dom IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1 4 0-3
$ virsh iothreadinfo $dom --config IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1
$ virsh iothreadadd $dom 4 --config $ virsh iothreadinfo $dom --config IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1 4 0-3
$ virsh iothreadadd $dom 5 userdisk $ virsh qemu-monitor-command $dom '{"execute":"query-iothreads"}' {"return":[{"thread-id":17889,"id":"iothread1"},{"thread-id":17890,"id":"iothread2"},{"thread-id":17892,"id":"iothread3"},{"thread-id":17893,"id":"iothread4"},{"thread-id":18108,"id":"userdisk_iothread5"}],"id":"libvirt-104"}
$ virsh iothreaddel $dom 5 userdisk
Assuming the same original configuration
$ virsh iothreaddel $dom 1 error: invalid argument: cannot remove IOThread 1 since it is being used by disk path '/home/vm-images/f18'
$ virsh iothreaddel $dom 3
$ virsh iothreadinfo $dom IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3
$ virsh iothreadinfo $dom --config IOThread ID CPU Affinity --------------------------------------------------- 1 2 2 3 3 0-1
Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh-domain.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 32 ++++++++++ 2 files changed, 206 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 928360c..37836ce 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6977,6 +6977,168 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd) }
/* + * "iothreadadd" command + */ +static const vshCmdInfo info_iothreadadd[] = { + {.name = "help", + .data = N_("add an IOThread to the guest domain") + }, + {.name = "desc", + .data = N_("Add an IOThread to the guest domain.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_iothreadadd[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "id", + .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ, + .help = N_("iothread for the new IOThread") + }, + {.name = "name", + .type = VSH_OT_STRING, + .help = N_("a name for the new IOThread") + }, + {.name = "config", + .type = VSH_OT_BOOL, + .help = N_("affect next boot") + }, + {.name = "live", + .type = VSH_OT_BOOL, + .help = N_("affect running domain") + }, + {.name = "current", + .type = VSH_OT_BOOL, + .help = N_("affect current domain") + }, + {.name = NULL} +}; + +static bool +cmdIOThreadAdd(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + int iothread_id = 0; + const char *name_option = NULL; + bool ret = false; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptInt(cmd, "id", &iothread_id) < 0 || iothread_id <= 0) { + vshError(ctl, _("Invalid IOThread id value: '%d'"), iothread_id); + goto cleanup; + } + + if (vshCommandOptStringReq(ctl, cmd, "name", &name_option) < 0) {
vshCommandOptStringReq should already report a sane error.
+ vshError(ctl, "%s", _("Unable to parse 'name' parameter")); + goto cleanup; + } + + if (virDomainAddIOThread(dom, iothread_id, name_option, flags) < 0) + goto cleanup; + + ret = true; + + cleanup: + virDomainFree(dom); + return ret; +} + +/* + * "iothreaddel" command + */ +static const vshCmdInfo info_iothreaddel[] = { + {.name = "help", + .data = N_("delete an IOThread from the guest domain") + }, + {.name = "desc", + .data = N_("Delete an IOThread from the guest domain.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_iothreaddel[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") + }, + {.name = "id", + .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ, + .help = N_("iothread_id for the IOThread to delete") + }, + {.name = "config", + .type = VSH_OT_BOOL, + .help = N_("affect next boot") + }, + {.name = "live", + .type = VSH_OT_BOOL, + .help = N_("affect running domain") + }, + {.name = "current", + .type = VSH_OT_BOOL, + .help = N_("affect current domain") + }, + {.name = NULL} +};
Looks good. Peter
participants (2)
-
John Ferlan -
Peter Krempa