8:18 p.m.
v2:
http://www.redhat.com/archives/libvir-list/2015-April/msg00426.html
Differences:
Patch 1 (old patch 3):
- virDomainIOThreadIDDefParseXML:
- Fix error message for <id> (was element, change to attribute for
<iothread> element)
- Silently bump the 'def->iothreads' count if there are a more
<iothreadid> elements found than def->iothreads defined rather than
causing an error
- virDomainIOThreadIDAdd
- Just use VIR_APPEND_ELEMENT - no need for empty array check
- Remove virDomainIOThreadIDIsDuplicate
- Since it was doing essentially the same as virDomainIOThreadIDFind
- Replace callers of IDIsDuplicate with the IDFind
- Remove 'name' from _virDomainIOThreadIDDef causes ripple effect...
- formatdomain.html.in and docs/schemas/domaincommon.rng
- No need for virDomainIOThreadIDDefFree
- Change of parameters to virDomainIOThreadIDAdd
- Remove parse in virDomainIOThreadIDDefParseXML
- Remove format in virDomainDefFormatInternal
- Remove virDomainIOThreadIDDefFree
- Unnecessary since 'name' is removed
Patch 2 (merged old patch 4 & 5)
- Add 'thread_id' to virDomainIOThreadIDDef and remove from
_qemuDomainObjPrivate. Adjust qemuDomainObjPrivateFree,
qemuDomainObjPrivateXMLFormat, and qemuDomainObjPrivateXMLParse
to remove the the iothreadpids usage. Rely on saved domain XML
for the 'live' data.
- Use vm->def->niothreadids instead of priv->niothreadpids
- Remove a couple of (now) unnecessary checks where priv->[n]iothreadpids
was referenced. Now we now that at XMLParse we've created the
[vm->]def->iothreadids with enough elements to satisfy the maximum
of def->iothreads and def->niothreadids, so no need to check if it
exists like we had to for priv->iothreadpids
Patch 3 (old patch 6)
- Remove 'name' from virDomainAddIOThread
- Fix comments in virDomainIOThreadAdd and virDomainIOThreadDel
- Remove if ((unsigned short) iothread_id != iothread_id) check
Patch 4 (old patch 7)
- Remove 'name' from remote_domain_add_iothread_args and
remote_domain_add_iothread_args
Patch 5 (old patch 8)
- Split qemuDomainHotplugIOThread into qemuDomainHotplugAddIOThread and
qemuDomainHotplugDelIOThread
- Removed extraneous comments
- Moved active domain check to after job starts and only for LIVE
- Removed the virResetLastError if qemuMonitorGetIOThreads fails
- Didn't touch the Numatune change - that can be resolved separately
- For the Add case, there was a comment about already having the data
which while true, we don't know where in the returned qemu iothreads
list the new IOThread was added, so we have to find it in the list so
we can get the thread_id so save once we create the element in the
iothreadids list.
- Moved where the virDomainLiveConfigHelperMethod was called
- For an inactive config - we can add iothreads and iothreadids, when/if
someone starts the guest, then they'll get the bad news if the right
emulator wasn't available.
- Handling LIVE before CONFIG just seemed to be the norm for commands
that I looked at, so I left it that way.
Patch 6 (old patch 9)
- Remove "name" (avoided the one comment from the review)
John Ferlan (6):
conf: Add new domain XML element 'iothreadids'
qemu: Use domain iothreadids to IOThread's 'thread_id'
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 | 30 ++
docs/schemas/domaincommon.rng | 12 +
include/libvirt/libvirt-domain.h | 6 +
src/conf/domain_audit.c | 9 +
src/conf/domain_audit.h | 6 +
src/conf/domain_conf.c | 190 ++++++++-
src/conf/domain_conf.h | 16 +
src/driver-hypervisor.h | 12 +
src/libvirt-domain.c | 118 ++++++
src/libvirt_private.syms | 4 +
src/libvirt_public.syms | 6 +
src/qemu/qemu_cgroup.c | 22 +-
src/qemu/qemu_command.c | 38 +-
src/qemu/qemu_command.h | 3 +
src/qemu/qemu_domain.c | 36 --
src/qemu/qemu_domain.h | 3 -
src/qemu/qemu_driver.c | 466 ++++++++++++++++++++-
src/qemu/qemu_process.c | 37 +-
src/remote/remote_driver.c | 2 +
src/remote/remote_protocol.x | 30 +-
src/remote_protocol-structs | 12 +
.../qemuxml2argv-iothreads-ids-partial.args | 10 +
.../qemuxml2argv-iothreads-ids-partial.xml | 33 ++
.../qemuxml2argv-iothreads-ids.args | 8 +
.../qemuxml2argv-iothreads-ids.xml | 33 ++
tests/qemuxml2argvtest.c | 2 +
tests/qemuxml2xmltest.c | 2 +
tools/virsh-domain.c | 164 ++++++++
tools/virsh.pod | 31 ++
29 files changed, 1239 insertions(+), 102 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
--
2.1.0
8:18 p.m.
New subject: [libvirt] [PATCH v3 1/6] conf: Add new domain XML element 'iothreadids'
Adding a new XML element 'iothreadids' in order to allow defining
specific IOThread ID's rather than relying on the algorithm to assign
IOThread ID's starting at 1 and incrementing to iothreads count.
This will allow future patches to be able to add new IOThreads by
a specific iothread_id and of course delete any exisiting IOThread.
Each iothreadids element will have 'n' <iothread> children elements
which will have attribute "id". The "id" will allow for definition
of any "valid" (eg > 0) iothread_id value.
On input, if any <iothreadids> <iothread>'s are provided, they will
be marked so that we only print out what we read in.
On input, if no <iothreadids> are provided, the PostParse code will
self generate a list of ID's starting at 1 and going to the number
of iothreads defined for the domain (just like the current algorithm
numbering scheme). A future patch will rework the existing algorithm
to make use of the iothreadids list.
On output, only print out the <iothreadids> if they were read in.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/formatdomain.html.in | 30 +++++++
docs/schemas/domaincommon.rng | 12 +++
src/conf/domain_conf.c | 190 +++++++++++++++++++++++++++++++++++++++++-
src/conf/domain_conf.h | 15 ++++
src/libvirt_private.syms | 3 +
5 files changed, 248 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e921749..518f7c5 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -521,6 +521,18 @@
...
</domain>
</pre>
+<pre>
+<domain>
+ ...
+ <iothreadids>
+ <iothread id="2"/>
+ <iothread id="4"/>
+ <iothread id="6"/>
+ <iothread id="8"/>
+ </iothreadids>
+ ...
+</domain>
+</pre>
<dl>
<dt><code>iothreads</code></dt>
@@ -530,7 +542,25 @@
virtio-blk-pci and virtio-blk-ccw target storage devices. There
should be only 1 or 2 IOThreads per host CPU. There may be more
than one supported device assigned to each IOThread.
+ <span class="since">Since 1.2.8</span>
</dd>
+ <dt><code>iothreadids</code></dt>
+ <dd>
+ The optional <code>iothreadids</code> element provides the
capability
+ to specifically define the IOThread ID's for the domain. By default,
+ IOThread ID's are sequentially numbered starting from 1 through the
+ number of <code>iothreads</code> defined for the domain. The
+ <code>id</code> attribute is used to define the IOThread ID. The
+ <code>id</code> attribute must be a positive integer greater than 0.
+ If there are less <code>iothreadids</code> defined than
+ <code>iothreads</code> defined for the domain, then libvirt will
+ sequentially fill <code>iothreadids</code> starting at 1 avoiding
+ any predefined <code>id</code>. If there are more
+ <code>iothreadids</code> defined than
<code>iothreads</code>
+ defined for the domain, then the <code>iothreads</code> value
+ will be adjusted accordingly.
+ <span class="since">Since 1.2.15</span>
+ </dd>
</dl>
<h3><a name="elementsCPUTuning">CPU
Tuning</a></h3>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 03fd541..4bd32bd 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -675,6 +675,18 @@
</optional>
<optional>
+ <element name="iothreadids">
+ <zeroOrMore>
+ <element name="iothread">
+ <attribute name="id">
+ <ref name="unsignedInt"/>
+ </attribute>
+ </element>
+ </zeroOrMore>
+ </element>
+ </optional>
+
+ <optional>
<ref name="blkiotune"/>
</optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4d7e3c9..e86b7e2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2113,6 +2113,23 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin)
return NULL;
}
+
+static void
+virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def,
+ int nids)
+{
+ size_t i;
+
+ if (!def)
+ return;
+
+ for (i = 0; i < nids; i++)
+ VIR_FREE(def[i]);
+
+ VIR_FREE(def);
+}
+
+
void
virDomainPinDefFree(virDomainPinDefPtr def)
{
@@ -2310,6 +2327,8 @@ void virDomainDefFree(virDomainDefPtr def)
virCPUDefFree(def->cpu);
+ virDomainIOThreadIDDefArrayFree(def->iothreadids, def->niothreadids);
+
virDomainPinDefArrayFree(def->cputune.vcpupin, def->cputune.nvcpupin);
virDomainPinDefFree(def->cputune.emulatorpin);
@@ -3304,6 +3323,18 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
return -1;
}
+ /* Fully populate the IOThread ID list */
+ if (def->iothreads && def->iothreads != def->niothreadids) {
+ unsigned int iothread_id = 1;
+ while (def->niothreadids != def->iothreads) {
+ if (!virDomainIOThreadIDFind(def, iothread_id)) {
+ if (virDomainIOThreadIDAdd(def, iothread_id) < 0)
+ return -1;
+ }
+ iothread_id++;
+ }
+ }
+
if (virDomainDefGetMemoryInitial(def) == 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Memory size must be specified via <memory> or in the
"
@@ -13173,6 +13204,56 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt,
return idmap;
}
+/* Parse the XML definition for an IOThread ID
+ *
+ * Format is :
+ *
+ * <iothreads>4</iothreads>
+ * <iothreadids>
+ * <iothread id='1'/>
+ * <iothread id='3'/>
+ * <iothread id='5'/>
+ * <iothread id='7'/>
+ * </iothreadids>
+ */
+static virDomainIOThreadIDDefPtr
+virDomainIOThreadIDDefParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt)
+{
+ virDomainIOThreadIDDefPtr iothrid;
+ xmlNodePtr oldnode = ctxt->node;
+ char *tmp = NULL;
+
+ if (VIR_ALLOC(iothrid) < 0)
+ return NULL;
+
+ ctxt->node = node;
+
+ if (!(tmp = virXPathString("string(./@id)", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Missing 'id' attribute in <iothread>
element"));
+ goto error;
+ }
+ if (virStrToLong_uip(tmp, NULL, 10, &iothrid->iothread_id) < 0 ||
+ iothrid->iothread_id == 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("invalid iothread 'id' value '%s'"),
tmp);
+ goto error;
+ }
+
+ iothrid->defined = true;
+
+ cleanup:
+ VIR_FREE(tmp);
+ ctxt->node = oldnode;
+ return iothrid;
+
+ error:
+ VIR_FREE(iothrid);
+ goto cleanup;
+}
+
+
/* Parse the XML definition for a vcpupin
*
* vcpupin has the form of
@@ -13948,6 +14029,32 @@ virDomainDefParseXML(xmlDocPtr xml,
}
VIR_FREE(tmp);
+ /* Extract any iothread id's defined */
+ if ((n = virXPathNodeSet("./iothreadids/iothread", ctxt, &nodes)) <
0)
+ goto error;
+
+ if (n > def->iothreads)
+ def->iothreads = n;
+
+ if (n && VIR_ALLOC_N(def->iothreadids, MAX(n, def->iothreads)) < 0)
+ goto error;
+
+ for (i = 0; i < n; i++) {
+ virDomainIOThreadIDDefPtr iothrid = NULL;
+ if (!(iothrid = virDomainIOThreadIDDefParseXML(nodes[i], ctxt)))
+ goto error;
+
+ if (virDomainIOThreadIDFind(def, iothrid->iothread_id)) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("duplicate iothread id '%u' found"),
+ iothrid->iothread_id);
+ VIR_FREE(iothrid);
+ goto error;
+ }
+ def->iothreadids[def->niothreadids++] = iothrid;
+ }
+ VIR_FREE(nodes);
+
/* Extract cpu tunables. */
if ((n = virXPathULong("string(./cputune/shares[1])", ctxt,
&def->cputune.shares)) < -1) {
@@ -17324,6 +17431,66 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def)
return 0;
}
+virDomainIOThreadIDDefPtr
+virDomainIOThreadIDFind(virDomainDefPtr def,
+ unsigned int iothread_id)
+{
+ size_t i;
+
+ if (!def->iothreadids || !def->niothreadids)
+ return NULL;
+
+ for (i = 0; i < def->niothreadids; i++) {
+ if (iothread_id == def->iothreadids[i]->iothread_id)
+ return def->iothreadids[i];
+ }
+
+ return NULL;
+}
+
+int
+virDomainIOThreadIDAdd(virDomainDefPtr def,
+ unsigned int iothread_id)
+{
+ virDomainIOThreadIDDefPtr iothrid = NULL;
+
+ if (virDomainIOThreadIDFind(def, iothread_id)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot duplicate iothread_id '%u' in
iothreadids"),
+ iothread_id);
+ return -1;
+ }
+
+ if (VIR_ALLOC(iothrid) < 0)
+ goto error;
+
+ iothrid->iothread_id = iothread_id;
+
+ if (VIR_APPEND_ELEMENT(def->iothreadids, def->niothreadids, iothrid) < 0)
+ goto error;
+
+ return 0;
+
+ error:
+ VIR_FREE(iothrid);
+ return -1;
+}
+
+void
+virDomainIOThreadIDDel(virDomainDefPtr def,
+ unsigned int iothread_id)
+{
+ int n;
+
+ for (n = 0; n < def->niothreadids; n++) {
+ if (def->iothreadids[n]->iothread_id == iothread_id) {
+ VIR_FREE(def->iothreadids[n]);
+ VIR_DELETE_ELEMENT(def->iothreadids, n, def->niothreadids);
+ return;
+ }
+ }
+}
+
/* Check if vcpupin with same id already exists. */
bool
virDomainPinIsDuplicate(virDomainPinDefPtr *def,
@@ -20666,8 +20833,27 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAsprintf(buf, " current='%u'", def->vcpus);
virBufferAsprintf(buf, ">%u</vcpu>\n", def->maxvcpus);
- if (def->iothreads > 0)
- virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n",
def->iothreads);
+ if (def->iothreads > 0) {
+ virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n",
+ def->iothreads);
+ /* If we parsed the iothreadids, then write out those that we parsed */
+ for (i = 0; i < def->niothreadids; i++) {
+ if (def->iothreadids[i]->defined)
+ break;
+ }
+ if (i < def->niothreadids) {
+ virBufferAddLit(buf, "<iothreadids>\n");
+ virBufferAdjustIndent(buf, 2);
+ for (i = 0; i < def->niothreadids; i++) {
+ if (!def->iothreadids[i]->defined)
+ continue;
+ virBufferAsprintf(buf, "<iothread id='%u'/>\n",
+ def->iothreadids[i]->iothread_id);
+ }
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</iothreadids>\n");
+ }
+ }
if (def->cputune.sharesSpecified ||
(def->cputune.nvcpupin && !virDomainIsAllVcpupinInherited(def)) ||
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e6fa3c9..9e7bdf9 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2041,6 +2041,14 @@ struct _virDomainHugePage {
unsigned long long size; /* hugepage size in KiB */
};
+typedef struct _virDomainIOThreadIDDef virDomainIOThreadIDDef;
+typedef virDomainIOThreadIDDef *virDomainIOThreadIDDefPtr;
+
+struct _virDomainIOThreadIDDef {
+ bool defined;
+ unsigned int iothread_id;
+};
+
typedef struct _virDomainCputune virDomainCputune;
typedef virDomainCputune *virDomainCputunePtr;
@@ -2132,6 +2140,8 @@ struct _virDomainDef {
virBitmapPtr cpumask;
unsigned int iothreads;
+ size_t niothreadids;
+ virDomainIOThreadIDDefPtr *iothreadids;
virDomainCputune cputune;
@@ -2590,6 +2600,11 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src,
int virDomainDefAddImplicitControllers(virDomainDefPtr def);
+virDomainIOThreadIDDefPtr
+virDomainIOThreadIDFind(virDomainDefPtr def, unsigned int iothread_id);
+int virDomainIOThreadIDAdd(virDomainDefPtr def, unsigned int iothread_id);
+void virDomainIOThreadIDDel(virDomainDefPtr def, unsigned int iothread_id);
+
unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags);
char *virDomainDefFormat(virDomainDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7166283..12fd9e9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -326,6 +326,9 @@ virDomainHubTypeToString;
virDomainHypervTypeFromString;
virDomainHypervTypeToString;
virDomainInputDefFree;
+virDomainIOThreadIDAdd;
+virDomainIOThreadIDDel;
+virDomainIOThreadIDFind;
virDomainLeaseDefFree;
virDomainLeaseIndex;
virDomainLeaseInsert;
--
2.1.0
9:08 a.m.
New subject: [libvirt] [PATCH v3 1/6] conf: Add new domain XML element 'iothreadids'
On Tue, Apr 14, 2015 at 21:18: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 iothreadids element will have 'n' <iothread> children elements
which will have attribute "id". The "id" will allow for definition
of any "valid" (eg > 0) iothread_id value.
On input, if any <iothreadids> <iothread>'s are provided, they will
be marked so that we only print out what we read in.
On input, if no <iothreadids> are provided, the PostParse code will
self generate a list of ID's starting at 1 and going to the number
of iothreads defined for the domain (just like the current algorithm
numbering scheme). A future patch will rework the existing algorithm
to make use of the iothreadids list.
On output, only print out the <iothreadids> if they were read in.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/formatdomain.html.in | 30 +++++++
docs/schemas/domaincommon.rng | 12 +++
src/conf/domain_conf.c | 190 +++++++++++++++++++++++++++++++++++++++++-
src/conf/domain_conf.h | 15 ++++
src/libvirt_private.syms | 3 +
5 files changed, 248 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e921749..518f7c5 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -521,6 +521,18 @@
...
</domain>
</pre>
+<pre>
+<domain>
+ ...
+ <iothreadids>
+ <iothread id="2"/>
+ <iothread id="4"/>
+ <iothread id="6"/>
+ <iothread id="8"/>
+ </iothreadids>
+ ...
+</domain>
+</pre>
<dl>
<dt><code>iothreads</code></dt>
@@ -530,7 +542,25 @@
virtio-blk-pci and virtio-blk-ccw target storage devices. There
should be only 1 or 2 IOThreads per host CPU. There may be more
than one supported device assigned to each IOThread.
+ <span class="since">Since 1.2.8</span>
</dd>
+ <dt><code>iothreadids</code></dt>
+ <dd>
+ The optional <code>iothreadids</code> element provides the
capability
+ to specifically define the IOThread ID's for the domain. By default,
+ IOThread ID's are sequentially numbered starting from 1 through the
+ number of <code>iothreads</code> defined for the domain. The
+ <code>id</code> attribute is used to define the IOThread ID. The
+ <code>id</code> attribute must be a positive integer greater than
0.
+ If there are less <code>iothreadids</code> defined than
+ <code>iothreads</code> defined for the domain, then libvirt will
+ sequentially fill <code>iothreadids</code> starting at 1 avoiding
+ any predefined <code>id</code>. If there are more
+ <code>iothreadids</code> defined than
<code>iothreads</code>
+ defined for the domain, then the <code>iothreads</code> value
+ will be adjusted accordingly.
+ <span class="since">Since 1.2.15</span>
+ </dd>
</dl>
<h3><a name="elementsCPUTuning">CPU
Tuning</a></h3>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 03fd541..4bd32bd 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -675,6 +675,18 @@
</optional>
<optional>
+ <element name="iothreadids">
+ <zeroOrMore>
+ <element name="iothread">
+ <attribute name="id">
+ <ref name="unsignedInt"/>
+ </attribute>
+ </element>
+ </zeroOrMore>
+ </element>
+ </optional>
+
+ <optional>
<ref name="blkiotune"/>
</optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4d7e3c9..e86b7e2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2113,6 +2113,23 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin)
return NULL;
}
+
+static void
+virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def,
+ int nids)
+{
+ size_t i;
+
+ if (!def)
+ return;
+
+ for (i = 0; i < nids; i++)
+ VIR_FREE(def[i]);
+
+ VIR_FREE(def);
+}
+
+
void
virDomainPinDefFree(virDomainPinDefPtr def)
{
@@ -2310,6 +2327,8 @@ void virDomainDefFree(virDomainDefPtr def)
virCPUDefFree(def->cpu);
+ virDomainIOThreadIDDefArrayFree(def->iothreadids, def->niothreadids);
+
virDomainPinDefArrayFree(def->cputune.vcpupin, def->cputune.nvcpupin);
virDomainPinDefFree(def->cputune.emulatorpin);
@@ -3304,6 +3323,18 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
return -1;
}
+ /* Fully populate the IOThread ID list */
+ if (def->iothreads && def->iothreads != def->niothreadids) {
+ unsigned int iothread_id = 1;
+ while (def->niothreadids != def->iothreads) {
+ if (!virDomainIOThreadIDFind(def, iothread_id)) {
+ if (virDomainIOThreadIDAdd(def, iothread_id) < 0)
This code is _adding_ fields for every iothread that was not specified
in <iothreadids> but is implied by <iothreads>, but before that you've
allocated all the structures in [1].
+ return -1;
+ }
+ iothread_id++;
+ }
+ }
+
if (virDomainDefGetMemoryInitial(def) == 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Memory size must be specified via <memory> or in
the "
@@ -13173,6 +13204,56 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt,
return idmap;
}
+/* Parse the XML definition for an IOThread ID
+ *
+ * Format is :
+ *
+ * <iothreads>4</iothreads>
+ * <iothreadids>
+ * <iothread id='1'/>
+ * <iothread id='3'/>
+ * <iothread id='5'/>
+ * <iothread id='7'/>
+ * </iothreadids>
+ */
+static virDomainIOThreadIDDefPtr
+virDomainIOThreadIDDefParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt)
+{
+ virDomainIOThreadIDDefPtr iothrid;
+ xmlNodePtr oldnode = ctxt->node;
+ char *tmp = NULL;
+
+ if (VIR_ALLOC(iothrid) < 0)
+ return NULL;
+
+ ctxt->node = node;
+
+ if (!(tmp = virXPathString("string(./@id)", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Missing 'id' attribute in <iothread>
element"));
+ goto error;
+ }
+ if (virStrToLong_uip(tmp, NULL, 10, &iothrid->iothread_id) < 0 ||
+ iothrid->iothread_id == 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("invalid iothread 'id' value '%s'"),
tmp);
+ goto error;
+ }
+
+ iothrid->defined = true;
+
+ cleanup:
+ VIR_FREE(tmp);
+ ctxt->node = oldnode;
+ return iothrid;
+
+ error:
+ VIR_FREE(iothrid);
+ goto cleanup;
+}
+
+
/* Parse the XML definition for a vcpupin
*
* vcpupin has the form of
@@ -13948,6 +14029,32 @@ virDomainDefParseXML(xmlDocPtr xml,
}
VIR_FREE(tmp);
+ /* Extract any iothread id's defined */
+ if ((n = virXPathNodeSet("./iothreadids/iothread", ctxt, &nodes)) <
0)
+ goto error;
+
+ if (n > def->iothreads)
+ def->iothreads = n;
+
+ if (n && VIR_ALLOC_N(def->iothreadids, MAX(n, def->iothreads)) <
0)
+ goto error;
[1]
+
+ for (i = 0; i < n; i++) {
+ virDomainIOThreadIDDefPtr iothrid = NULL;
+ if (!(iothrid = virDomainIOThreadIDDefParseXML(nodes[i], ctxt)))
+ goto error;
+
+ if (virDomainIOThreadIDFind(def, iothrid->iothread_id)) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("duplicate iothread id '%u' found"),
+ iothrid->iothread_id);
+ VIR_FREE(iothrid);
+ goto error;
+ }
+ def->iothreadids[def->niothreadids++] = iothrid;
+ }
+ VIR_FREE(nodes);
+
/* Extract cpu tunables. */
if ((n = virXPathULong("string(./cputune/shares[1])", ctxt,
&def->cputune.shares)) < -1) {
@@ -17324,6 +17431,66 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def)
return 0;
}
+virDomainIOThreadIDDefPtr
+virDomainIOThreadIDFind(virDomainDefPtr def,
+ unsigned int iothread_id)
+{
+ size_t i;
+
+ if (!def->iothreadids || !def->niothreadids)
+ return NULL;
+
+ for (i = 0; i < def->niothreadids; i++) {
+ if (iothread_id == def->iothreadids[i]->iothread_id)
+ return def->iothreadids[i];
+ }
+
+ return NULL;
+}
+
+int
+virDomainIOThreadIDAdd(virDomainDefPtr def,
+ unsigned int iothread_id)
If this function returned the pointer to the added structure you'd be
able to use it later to modify other aspects of it.
+{
+ virDomainIOThreadIDDefPtr iothrid = NULL;
+
+ if (virDomainIOThreadIDFind(def, iothread_id)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot duplicate iothread_id '%u' in
iothreadids"),
+ iothread_id);
+ return -1;
+ }
+
+ if (VIR_ALLOC(iothrid) < 0)
+ goto error;
+
+ iothrid->iothread_id = iothread_id;
+
+ if (VIR_APPEND_ELEMENT(def->iothreadids, def->niothreadids, iothrid) < 0)
+ goto error;
+
+ return 0;
+
+ error:
+ VIR_FREE(iothrid);
+ return -1;
+}
+
+void
+virDomainIOThreadIDDel(virDomainDefPtr def,
+ unsigned int iothread_id)
+{
+ int n;
+
+ for (n = 0; n < def->niothreadids; n++) {
+ if (def->iothreadids[n]->iothread_id == iothread_id) {
+ VIR_FREE(def->iothreadids[n]);
This could use a common freeing function, so that once you add more data
you won't have to tweak it.
+ VIR_DELETE_ELEMENT(def->iothreadids, n,
def->niothreadids);
+ return;
+ }
+ }
+}
+
/* Check if vcpupin with same id already exists. */
bool
virDomainPinIsDuplicate(virDomainPinDefPtr *def,
@@ -20666,8 +20833,27 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAsprintf(buf, " current='%u'", def->vcpus);
virBufferAsprintf(buf, ">%u</vcpu>\n", def->maxvcpus);
- if (def->iothreads > 0)
- virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n",
def->iothreads);
+ if (def->iothreads > 0) {
+ virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n",
+ def->iothreads);
+ /* If we parsed the iothreadids, then write out those that we parsed */
Seems rather obvious.
+ for (i = 0; i < def->niothreadids; i++) {
+ if (def->iothreadids[i]->defined)
+ break;
+ }
+ if (i < def->niothreadids) {
+ virBufferAddLit(buf, "<iothreadids>\n");
+ virBufferAdjustIndent(buf, 2);
+ for (i = 0; i < def->niothreadids; i++) {
+ if (!def->iothreadids[i]->defined)
+ continue;
+ virBufferAsprintf(buf, "<iothread id='%u'/>\n",
+ def->iothreadids[i]->iothread_id);
+ }
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</iothreadids>\n");
+ }
+ }
if (def->cputune.sharesSpecified ||
(def->cputune.nvcpupin && !virDomainIsAllVcpupinInherited(def)) ||
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e6fa3c9..9e7bdf9 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2041,6 +2041,14 @@ struct _virDomainHugePage {
unsigned long long size; /* hugepage size in KiB */
};
+typedef struct _virDomainIOThreadIDDef virDomainIOThreadIDDef;
+typedef virDomainIOThreadIDDef *virDomainIOThreadIDDefPtr;
+
+struct _virDomainIOThreadIDDef {
+ bool defined;
You may want to invert this field, since the case where you add the
thread structs due to the fact that <iohreads> was more than the count
of entries is less common than other places that add the struct.
+ unsigned int iothread_id;
+};
+
typedef struct _virDomainCputune virDomainCputune;
typedef virDomainCputune *virDomainCputunePtr;
@@ -2132,6 +2140,8 @@ struct _virDomainDef {
virBitmapPtr cpumask;
unsigned int iothreads;
+ size_t niothreadids;
+ virDomainIOThreadIDDefPtr *iothreadids;
virDomainCputune cputune;
@@ -2590,6 +2600,11 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src,
int virDomainDefAddImplicitControllers(virDomainDefPtr def);
+virDomainIOThreadIDDefPtr
+virDomainIOThreadIDFind(virDomainDefPtr def, unsigned int iothread_id);
Usually the return type is on the same line and the argument list is
broken if it doesn't fit.
+int virDomainIOThreadIDAdd(virDomainDefPtr def, unsigned int
iothread_id);
+void virDomainIOThreadIDDel(virDomainDefPtr def, unsigned int iothread_id);
+
unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags);
char *virDomainDefFormat(virDomainDefPtr def,
11:37 a.m.
New subject: [libvirt] [PATCH v3 1/6] conf: Add new domain XML element 'iothreadids'
On 04/21/2015 10:08 AM, Peter Krempa wrote:
On Tue, Apr 14, 2015 at 21:18: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 iothreadids element will have 'n' <iothread> children elements
> which will have attribute "id". The "id" will allow for
definition
> of any "valid" (eg > 0) iothread_id value.
>
> On input, if any <iothreadids> <iothread>'s are provided, they will
> be marked so that we only print out what we read in.
>
> On input, if no <iothreadids> are provided, the PostParse code will
> self generate a list of ID's starting at 1 and going to the number
> of iothreads defined for the domain (just like the current algorithm
> numbering scheme). A future patch will rework the existing algorithm
> to make use of the iothreadids list.
>
> On output, only print out the <iothreadids> if they were read in.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> docs/formatdomain.html.in | 30 +++++++
> docs/schemas/domaincommon.rng | 12 +++
> src/conf/domain_conf.c | 190 +++++++++++++++++++++++++++++++++++++++++-
> src/conf/domain_conf.h | 15 ++++
> src/libvirt_private.syms | 3 +
> 5 files changed, 248 insertions(+), 2 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index e921749..518f7c5 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -521,6 +521,18 @@
> ...
> </domain>
> </pre>
> +<pre>
> +<domain>
> + ...
> + <iothreadids>
> + <iothread id="2"/>
> + <iothread id="4"/>
> + <iothread id="6"/>
> + <iothread id="8"/>
> + </iothreadids>
> + ...
> +</domain>
> +</pre>
>
> <dl>
> <dt><code>iothreads</code></dt>
> @@ -530,7 +542,25 @@
> virtio-blk-pci and virtio-blk-ccw target storage devices. There
> should be only 1 or 2 IOThreads per host CPU. There may be more
> than one supported device assigned to each IOThread.
> + <span class="since">Since 1.2.8</span>
> </dd>
> + <dt><code>iothreadids</code></dt>
> + <dd>
> + The optional <code>iothreadids</code> element provides the
capability
> + to specifically define the IOThread ID's for the domain. By default,
> + IOThread ID's are sequentially numbered starting from 1 through the
> + number of <code>iothreads</code> defined for the domain. The
> + <code>id</code> attribute is used to define the IOThread ID.
The
> + <code>id</code> attribute must be a positive integer greater
than 0.
> + If there are less <code>iothreadids</code> defined than
> + <code>iothreads</code> defined for the domain, then libvirt
will
> + sequentially fill <code>iothreadids</code> starting at 1
avoiding
> + any predefined <code>id</code>. If there are more
> + <code>iothreadids</code> defined than
<code>iothreads</code>
> + defined for the domain, then the <code>iothreads</code> value
> + will be adjusted accordingly.
> + <span class="since">Since 1.2.15</span>
> + </dd>
> </dl>
>
> <h3><a name="elementsCPUTuning">CPU
Tuning</a></h3>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 03fd541..4bd32bd 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -675,6 +675,18 @@
> </optional>
>
> <optional>
> + <element name="iothreadids">
> + <zeroOrMore>
> + <element name="iothread">
> + <attribute name="id">
> + <ref name="unsignedInt"/>
> + </attribute>
> + </element>
> + </zeroOrMore>
> + </element>
> + </optional>
> +
> + <optional>
> <ref name="blkiotune"/>
> </optional>
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 4d7e3c9..e86b7e2 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2113,6 +2113,23 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin)
> return NULL;
> }
>
> +
> +static void
> +virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def,
> + int nids)
> +{
> + size_t i;
> +
> + if (!def)
> + return;
> +
> + for (i = 0; i < nids; i++)
> + VIR_FREE(def[i]);
> +
> + VIR_FREE(def);
> +}
> +
> +
> void
> virDomainPinDefFree(virDomainPinDefPtr def)
> {
> @@ -2310,6 +2327,8 @@ void virDomainDefFree(virDomainDefPtr def)
>
> virCPUDefFree(def->cpu);
>
> + virDomainIOThreadIDDefArrayFree(def->iothreadids, def->niothreadids);
> +
> virDomainPinDefArrayFree(def->cputune.vcpupin, def->cputune.nvcpupin);
>
> virDomainPinDefFree(def->cputune.emulatorpin);
> @@ -3304,6 +3323,18 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
> return -1;
> }
>
> + /* Fully populate the IOThread ID list */
> + if (def->iothreads && def->iothreads != def->niothreadids) {
> + unsigned int iothread_id = 1;
> + while (def->niothreadids != def->iothreads) {
> + if (!virDomainIOThreadIDFind(def, iothread_id)) {
> + if (virDomainIOThreadIDAdd(def, iothread_id) < 0)
This code is _adding_ fields for every iothread that was not specified
in <iothreadids> but is implied by <iothreads>, but before that you've
allocated all the structures in [1].
Hmm... correct - of course the allocation in [1] was suggested by
previous code review.
> + return -1;
> + }
> + iothread_id++;
> + }
> + }
> +
> if (virDomainDefGetMemoryInitial(def) == 0) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> _("Memory size must be specified via <memory> or
in the "
> @@ -13173,6 +13204,56 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt,
> return idmap;
> }
>
> +/* Parse the XML definition for an IOThread ID
> + *
> + * Format is :
> + *
> + * <iothreads>4</iothreads>
> + * <iothreadids>
> + * <iothread id='1'/>
> + * <iothread id='3'/>
> + * <iothread id='5'/>
> + * <iothread id='7'/>
> + * </iothreadids>
> + */
> +static virDomainIOThreadIDDefPtr
> +virDomainIOThreadIDDefParseXML(xmlNodePtr node,
> + xmlXPathContextPtr ctxt)
> +{
> + virDomainIOThreadIDDefPtr iothrid;
> + xmlNodePtr oldnode = ctxt->node;
> + char *tmp = NULL;
> +
> + if (VIR_ALLOC(iothrid) < 0)
> + return NULL;
> +
> + ctxt->node = node;
> +
> + if (!(tmp = virXPathString("string(./@id)", ctxt))) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Missing 'id' attribute in <iothread>
element"));
> + goto error;
> + }
> + if (virStrToLong_uip(tmp, NULL, 10, &iothrid->iothread_id) < 0 ||
> + iothrid->iothread_id == 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("invalid iothread 'id' value
'%s'"), tmp);
> + goto error;
> + }
> +
> + iothrid->defined = true;
> +
> + cleanup:
> + VIR_FREE(tmp);
> + ctxt->node = oldnode;
> + return iothrid;
> +
> + error:
> + VIR_FREE(iothrid);
> + goto cleanup;
> +}
> +
> +
> /* Parse the XML definition for a vcpupin
> *
> * vcpupin has the form of
> @@ -13948,6 +14029,32 @@ virDomainDefParseXML(xmlDocPtr xml,
> }
> VIR_FREE(tmp);
>
> + /* Extract any iothread id's defined */
> + if ((n = virXPathNodeSet("./iothreadids/iothread", ctxt, &nodes))
< 0)
> + goto error;
> +
> + if (n > def->iothreads)
> + def->iothreads = n;
> +
> + if (n && VIR_ALLOC_N(def->iothreadids, MAX(n, def->iothreads))
< 0)
> + goto error;
[1]
Yes, the MAX(n, def->iothreads) was the prior code review suggestion...
So if I 'revert' back to just 'n', then let the above code handle the
rest, then these two issues should be resolved.
> +
> + for (i = 0; i < n; i++) {
> + virDomainIOThreadIDDefPtr iothrid = NULL;
> + if (!(iothrid = virDomainIOThreadIDDefParseXML(nodes[i], ctxt)))
> + goto error;
> +
> + if (virDomainIOThreadIDFind(def, iothrid->iothread_id)) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("duplicate iothread id '%u' found"),
> + iothrid->iothread_id);
> + VIR_FREE(iothrid);
> + goto error;
> + }
> + def->iothreadids[def->niothreadids++] = iothrid;
> + }
> + VIR_FREE(nodes);
> +
> /* Extract cpu tunables. */
> if ((n = virXPathULong("string(./cputune/shares[1])", ctxt,
> &def->cputune.shares)) < -1) {
> @@ -17324,6 +17431,66 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def)
> return 0;
> }
>
> +virDomainIOThreadIDDefPtr
> +virDomainIOThreadIDFind(virDomainDefPtr def,
> + unsigned int iothread_id)
> +{
> + size_t i;
> +
> + if (!def->iothreadids || !def->niothreadids)
> + return NULL;
> +
> + for (i = 0; i < def->niothreadids; i++) {
> + if (iothread_id == def->iothreadids[i]->iothread_id)
> + return def->iothreadids[i];
> + }
> +
> + return NULL;
> +}
> +
> +int
> +virDomainIOThreadIDAdd(virDomainDefPtr def,
> + unsigned int iothread_id)
If this function returned the pointer to the added structure you'd be
able to use it later to modify other aspects of it.
OK seems reasonable... had to change to using VIR_APPEND_ELEMENT_COPY too
> +{
> + virDomainIOThreadIDDefPtr iothrid = NULL;
> +
> + if (virDomainIOThreadIDFind(def, iothread_id)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("cannot duplicate iothread_id '%u' in
iothreadids"),
> + iothread_id);
> + return -1;
> + }
> +
> + if (VIR_ALLOC(iothrid) < 0)
> + goto error;
> +
> + iothrid->iothread_id = iothread_id;
> +
> + if (VIR_APPEND_ELEMENT(def->iothreadids, def->niothreadids, iothrid) <
0)
> + goto error;
> +
> + return 0;
> +
> + error:
> + VIR_FREE(iothrid);
> + return -1;
> +}
> +
> +void
> +virDomainIOThreadIDDel(virDomainDefPtr def,
> + unsigned int iothread_id)
> +{
> + int n;
> +
> + for (n = 0; n < def->niothreadids; n++) {
> + if (def->iothreadids[n]->iothread_id == iothread_id) {
> + VIR_FREE(def->iothreadids[n]);
This could use a common freeing function, so that once you add more data
you won't have to tweak it.
So you'd prefer a 'common' free function that just calls VIR_FREE() -
that just seems like excessive overhead, but that's fine.
> + VIR_DELETE_ELEMENT(def->iothreadids, n,
def->niothreadids);
> + return;
> + }
> + }
> +}
> +
> /* Check if vcpupin with same id already exists. */
> bool
> virDomainPinIsDuplicate(virDomainPinDefPtr *def,
> @@ -20666,8 +20833,27 @@ virDomainDefFormatInternal(virDomainDefPtr def,
> virBufferAsprintf(buf, " current='%u'", def->vcpus);
> virBufferAsprintf(buf, ">%u</vcpu>\n", def->maxvcpus);
>
> - if (def->iothreads > 0)
> - virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n",
def->iothreads);
> + if (def->iothreads > 0) {
> + virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n",
> + def->iothreads);
> + /* If we parsed the iothreadids, then write out those that we parsed */
Seems rather obvious.
As a reviewer for now, sure... But what about someone who reads this
code 6, 12, 18 months from now and wonders why the extra loop...
I can adjust the comment to be :
"Only print out iothreadids if we read at least one"
> + for (i = 0; i < def->niothreadids; i++) {
> + if (def->iothreadids[i]->defined)
> + break;
> + }
> + if (i < def->niothreadids) {
> + virBufferAddLit(buf, "<iothreadids>\n");
> + virBufferAdjustIndent(buf, 2);
> + for (i = 0; i < def->niothreadids; i++) {
> + if (!def->iothreadids[i]->defined)
> + continue;
> + virBufferAsprintf(buf, "<iothread
id='%u'/>\n",
> + def->iothreadids[i]->iothread_id);
> + }
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</iothreadids>\n");
> + }
> + }
>
> if (def->cputune.sharesSpecified ||
> (def->cputune.nvcpupin && !virDomainIsAllVcpupinInherited(def))
||
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index e6fa3c9..9e7bdf9 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2041,6 +2041,14 @@ struct _virDomainHugePage {
> unsigned long long size; /* hugepage size in KiB */
> };
>
> +typedef struct _virDomainIOThreadIDDef virDomainIOThreadIDDef;
> +typedef virDomainIOThreadIDDef *virDomainIOThreadIDDefPtr;
> +
> +struct _virDomainIOThreadIDDef {
> + bool defined;
You may want to invert this field, since the case where you add the
thread structs due to the fact that <iohreads> was more than the count
of entries is less common than other places that add the struct.
Made for an awkward looking:
for (i = 0; i < def->niothreadids; i++) {
if (!def->iothreadids[i]->undefined)
break;
}
> + unsigned int iothread_id;
> +};
> +
> typedef struct _virDomainCputune virDomainCputune;
> typedef virDomainCputune *virDomainCputunePtr;
>
> @@ -2132,6 +2140,8 @@ struct _virDomainDef {
> virBitmapPtr cpumask;
>
> unsigned int iothreads;
> + size_t niothreadids;
> + virDomainIOThreadIDDefPtr *iothreadids;
>
> virDomainCputune cputune;
>
> @@ -2590,6 +2600,11 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src,
>
> int virDomainDefAddImplicitControllers(virDomainDefPtr def);
>
> +virDomainIOThreadIDDefPtr
> +virDomainIOThreadIDFind(virDomainDefPtr def, unsigned int iothread_id);
Usually the return type is on the same line and the argument list is
broken if it doesn't fit.
OK - perhaps a relic of previous code where even the first argument made
the line too long - been too long to remember for sure.
> +int virDomainIOThreadIDAdd(virDomainDefPtr def, unsigned int
iothread_id);
> +void virDomainIOThreadIDDel(virDomainDefPtr def, unsigned int iothread_id);
> +
> unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags);
>
> char *virDomainDefFormat(virDomainDefPtr def,
Which gives the following to be squashed in (trying to make progress on
at least the first two patches which compile and pass tests):
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 82c36a4..3ff06b6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2115,6 +2115,14 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int
npin)
}
+void
+virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def)
+{
+ if (def)
+ VIR_FREE(def);
+}
+
+
static void
virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def,
int nids)
@@ -2125,7 +2133,7 @@
virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def,
return;
for (i = 0; i < nids; i++)
- VIR_FREE(def[i]);
+ virDomainIOThreadIDDefFree(def[i]);
VIR_FREE(def);
}
@@ -3322,8 +3330,11 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
unsigned int iothread_id = 1;
while (def->niothreadids != def->iothreads) {
if (!virDomainIOThreadIDFind(def, iothread_id)) {
- if (virDomainIOThreadIDAdd(def, iothread_id) < 0)
+ virDomainIOThreadIDDefPtr iothrid;
+
+ if (!(iothrid = virDomainIOThreadIDAdd(def, iothread_id)))
return -1;
+ iothrid->undefined = true;
}
iothread_id++;
}
@@ -13216,15 +13227,13 @@ virDomainIOThreadIDDefParseXML(xmlNodePtr node,
goto error;
}
- iothrid->defined = true;
-
cleanup:
VIR_FREE(tmp);
ctxt->node = oldnode;
return iothrid;
error:
- VIR_FREE(iothrid);
+ virDomainIOThreadIDDefFree(iothrid);
goto cleanup;
}
@@ -14042,7 +14051,7 @@ virDomainDefParseXML(xmlDocPtr xml,
if (n > def->iothreads)
def->iothreads = n;
- if (n && VIR_ALLOC_N(def->iothreadids, MAX(n, def->iothreads)) < 0)
+ if (n && VIR_ALLOC_N(def->iothreadids, n) < 0)
goto error;
for (i = 0; i < n; i++) {
@@ -14054,7 +14063,7 @@ virDomainDefParseXML(xmlDocPtr xml,
virReportError(VIR_ERR_XML_ERROR,
_("duplicate iothread id '%u' found"),
iothrid->iothread_id);
- VIR_FREE(iothrid);
+ virDomainIOThreadIDDefFree(iothrid);
goto error;
}
def->iothreadids[def->niothreadids++] = iothrid;
@@ -17362,7 +17371,7 @@ virDomainIOThreadIDFind(virDomainDefPtr def,
return NULL;
}
-int
+virDomainIOThreadIDDefPtr
virDomainIOThreadIDAdd(virDomainDefPtr def,
unsigned int iothread_id)
{
@@ -17372,7 +17381,7 @@ virDomainIOThreadIDAdd(virDomainDefPtr def,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot duplicate iothread_id '%u' in
iothreadids"),
iothread_id);
- return -1;
+ return NULL;
}
if (VIR_ALLOC(iothrid) < 0)
@@ -17380,14 +17389,15 @@ virDomainIOThreadIDAdd(virDomainDefPtr def,
iothrid->iothread_id = iothread_id;
- if (VIR_APPEND_ELEMENT(def->iothreadids, def->niothreadids,
iothrid) < 0)
+ if (VIR_APPEND_ELEMENT_COPY(def->iothreadids, def->niothreadids,
+ iothrid) < 0)
goto error;
- return 0;
+ return iothrid;
error:
- VIR_FREE(iothrid);
- return -1;
+ virDomainIOThreadIDDefFree(iothrid);
+ return NULL;
}
void
@@ -17398,7 +17408,7 @@ virDomainIOThreadIDDel(virDomainDefPtr def,
for (n = 0; n < def->niothreadids; n++) {
if (def->iothreadids[n]->iothread_id == iothread_id) {
- VIR_FREE(def->iothreadids[n]);
+ virDomainIOThreadIDDefFree(def->iothreadids[n]);
VIR_DELETE_ELEMENT(def->iothreadids, n, def->niothreadids);
return;
}
@@ -20749,16 +20759,16 @@ virDomainDefFormatInternal(virDomainDefPtr def,
if (def->iothreads > 0) {
virBufferAsprintf(buf, "<iothreads>%u</iothreads>\n",
def->iothreads);
- /* If we parsed the iothreadids, then write out those that we
parsed */
+ /* Only print out iothreadids if we read at least one */
for (i = 0; i < def->niothreadids; i++) {
- if (def->iothreadids[i]->defined)
+ if (!def->iothreadids[i]->undefined)
break;
}
if (i < def->niothreadids) {
virBufferAddLit(buf, "<iothreadids>\n");
virBufferAdjustIndent(buf, 2);
for (i = 0; i < def->niothreadids; i++) {
- if (!def->iothreadids[i]->defined)
+ if (def->iothreadids[i]->undefined)
continue;
virBufferAsprintf(buf, "<iothread id='%u'/>\n",
def->iothreadids[i]->iothread_id);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 01dc1f9..32de301 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2058,10 +2058,12 @@ typedef struct _virDomainIOThreadIDDef
virDomainIOThreadIDDef;
typedef virDomainIOThreadIDDef *virDomainIOThreadIDDefPtr;
struct _virDomainIOThreadIDDef {
- bool defined;
+ bool undefined;
unsigned int iothread_id;
};
+void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def);
+
typedef struct _virDomainCputune virDomainCputune;
typedef virDomainCputune *virDomainCputunePtr;
@@ -2612,9 +2614,10 @@ bool
virDomainDefCheckABIStability(virDomainDefPtr src,
int virDomainDefAddImplicitControllers(virDomainDefPtr def);
-virDomainIOThreadIDDefPtr
-virDomainIOThreadIDFind(virDomainDefPtr def, unsigned int iothread_id);
-int virDomainIOThreadIDAdd(virDomainDefPtr def, unsigned int iothread_id);
+virDomainIOThreadIDDefPtr virDomainIOThreadIDFind(virDomainDefPtr def,
+ unsigned int
iothread_id);
+virDomainIOThreadIDDefPtr virDomainIOThreadIDAdd(virDomainDefPtr def,
+ unsigned int iothread_id);
void virDomainIOThreadIDDel(virDomainDefPtr def, unsigned int iothread_id);
unsigned int virDomainDefFormatConvertXMLFlags(unsigned int flags);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 649ed8f..ac35f1b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -325,6 +325,7 @@ virDomainHypervTypeFromString;
virDomainHypervTypeToString;
virDomainInputDefFree;
virDomainIOThreadIDAdd;
+virDomainIOThreadIDDefFree;
virDomainIOThreadIDDel;
virDomainIOThreadIDFind;
virDomainLeaseDefFree;
8:18 p.m.
New subject: [libvirt] [PATCH v3 2/6] qemu: Use domain iothreadids to IOThread's 'thread_id'
Add 'thread_id' to the virDomainIOThreadIDDef as a means to store the
'thread_id' as returned from the live qemu monitor data.
Remove the iothreadpids list from _qemuDomainObjPrivate and replace with
the new iothreadids 'thread_id' element.
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
Since iothreadids list keeps track of the iothread_id's, these are
now 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 and the usage of a smaller number of <iothreadid> values than
iothreads that exist (and usage of the default numbering scheme).
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/domain_conf.h | 1 +
src/qemu/qemu_cgroup.c | 22 ++++++-------
src/qemu/qemu_command.c | 38 +++++++++++++++++-----
src/qemu/qemu_command.h | 3 ++
src/qemu/qemu_domain.c | 36 --------------------
src/qemu/qemu_domain.h | 3 --
src/qemu/qemu_driver.c | 35 +++++++++-----------
src/qemu/qemu_process.c | 37 ++++++++++-----------
.../qemuxml2argv-iothreads-ids-partial.args | 10 ++++++
.../qemuxml2argv-iothreads-ids-partial.xml | 33 +++++++++++++++++++
.../qemuxml2argv-iothreads-ids.args | 8 +++++
.../qemuxml2argv-iothreads-ids.xml | 33 +++++++++++++++++++
tests/qemuxml2argvtest.c | 2 ++
tests/qemuxml2xmltest.c | 2 ++
14 files changed, 164 insertions(+), 99 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
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9e7bdf9..fb5f2e1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2047,6 +2047,7 @@ typedef virDomainIOThreadIDDef *virDomainIOThreadIDDefPtr;
struct _virDomainIOThreadIDDef {
bool defined;
unsigned int iothread_id;
+ int thread_id;
};
typedef struct _virDomainCputune virDomainCputune;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index e83342d..cdf0aaf 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -802,8 +802,9 @@ qemuRestoreCgroupState(virDomainObjPtr vm)
virCgroupFree(&cgroup_temp);
}
- for (i = 0; i < priv->niothreadpids; i++) {
- if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, i + 1,
+ for (i = 0; i < vm->def->niothreadids; i++) {
+ if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD,
+ vm->def->iothreadids[i]->iothread_id,
false, &cgroup_temp) < 0 ||
virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 ||
virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0 ||
@@ -1175,11 +1176,6 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
if (priv->cgroup == NULL)
return 0;
- if (def->iothreads && priv->niothreadpids == 0) {
- VIR_WARN("Unable to get iothreads' pids.");
- return 0;
- }
-
if (virDomainNumatuneGetMode(vm->def->numa, -1) ==
VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
@@ -1187,16 +1183,18 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
&mem_mask, -1) < 0)
goto cleanup;
- for (i = 0; i < priv->niothreadpids; i++) {
+ for (i = 0; i < def->niothreadids; 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,
+ def->iothreadids[i]->iothread_id,
true, &cgroup_iothread) < 0)
goto cleanup;
/* move the thread for iothread to sub dir */
- if (virCgroupAddTask(cgroup_iothread, priv->iothreadpids[i]) < 0)
+ if (virCgroupAddTask(cgroup_iothread,
+ def->iothreadids[i]->thread_id) < 0)
goto cleanup;
if (period || quota) {
@@ -1221,8 +1219,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 ==
+ def->iothreadids[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..7ae3d8e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -678,6 +678,24 @@ qemuOpenVhostNet(virDomainDefPtr def,
}
int
+qemuDomainParseIOThreadAlias(char *alias,
+ unsigned int *iothread_id)
+{
+ unsigned int idval;
+
+ 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;
+ }
+
+ *iothread_id = idval;
+ return 0;
+}
+
+int
qemuNetworkPrepareDevices(virDomainDefPtr def)
{
int ret = -1;
@@ -3985,11 +4003,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 in
iothreadid"),
+ disk->iothread);
return false;
}
@@ -4209,6 +4227,7 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
if (disk->iothread)
virBufferAsprintf(&opt, ",iothread=iothread%u",
disk->iothread);
}
+
qemuBuildIoEventFdStr(&opt, disk->ioeventfd, qemuCaps);
if (disk->event_idx &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_EVENT_IDX)) {
@@ -8794,14 +8813,15 @@ 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);
+ 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..538ccdf 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -238,6 +238,9 @@ int qemuOpenVhostNet(virDomainDefPtr def,
int *vhostfd,
size_t *vhostfdSize);
+int qemuDomainParseIOThreadAlias(char *alias,
+ unsigned int *iothread_id);
+
int qemuNetworkPrepareDevices(virDomainDefPtr def);
/*
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 603360f..9348bd1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -440,7 +440,6 @@ qemuDomainObjPrivateFree(void *data)
virDomainChrSourceDefFree(priv->monConfig);
qemuDomainObjFreeJob(priv);
VIR_FREE(priv->vcpupids);
- VIR_FREE(priv->iothreadpids);
VIR_FREE(priv->lockState);
VIR_FREE(priv->origname);
@@ -501,18 +500,6 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
virBufferAddLit(buf, "</vcpus>\n");
}
- if (priv->niothreadpids) {
- size_t i;
- virBufferAddLit(buf, "<iothreads>\n");
- virBufferAdjustIndent(buf, 2);
- for (i = 0; i < priv->niothreadpids; i++) {
- virBufferAsprintf(buf, "<iothread pid='%d'/>\n",
- priv->iothreadpids[i]);
- }
- virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "</iothreads>\n");
- }
-
if (priv->qemuCaps) {
size_t i;
virBufferAddLit(buf, "<qemuCaps>\n");
@@ -633,29 +620,6 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data)
VIR_FREE(nodes);
}
- n = virXPathNodeSet("./iothreads/iothread", ctxt, &nodes);
- if (n < 0)
- goto error;
- if (n) {
- priv->niothreadpids = n;
- if (VIR_REALLOC_N(priv->iothreadpids, priv->niothreadpids) < 0)
- goto error;
-
- for (i = 0; i < n; i++) {
- char *pidstr = virXMLPropString(nodes[i], "pid");
- if (!pidstr)
- goto error;
-
- if (virStrToLong_i(pidstr, NULL, 10,
- &(priv->iothreadpids[i])) < 0) {
- VIR_FREE(pidstr);
- goto error;
- }
- VIR_FREE(pidstr);
- }
- VIR_FREE(nodes);
- }
-
if ((n = virXPathNodeSet("./qemuCaps/flag", ctxt, &nodes)) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("failed to parse qemu capabilities
flags"));
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 3225abb..2642350 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -158,9 +158,6 @@ struct _qemuDomainObjPrivate {
int nvcpupids;
int *vcpupids;
- int niothreadpids;
- int *iothreadpids;
-
virDomainPCIAddressSetPtr pciaddrs;
virDomainCCWAddressSetPtr ccwaddrs;
virDomainVirtioSerialAddrSetPtr vioserialaddrs;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f5a3ef9..008258f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5834,14 +5834,16 @@ qemuDomainGetIOThreadsLive(virQEMUDriverPtr driver,
goto endjob;
for (i = 0; i < niothreads; i++) {
+ unsigned int iothread_id;
virBitmapPtr map = NULL;
- if (VIR_ALLOC(info_ret[i]) < 0)
+ if (qemuDomainParseIOThreadAlias(iothreads[i]->name,
+ &iothread_id) < 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)
goto endjob;
+ info_ret[i]->iothread_id = iothread_id;
if (virProcessGetAffinity(iothreads[i]->thread_id, &map, hostcpus) <
0)
goto endjob;
@@ -5896,19 +5898,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;
@@ -6052,16 +6054,11 @@ qemuDomainPinIOThread(virDomainPtr dom,
if (flags & VIR_DOMAIN_AFFECT_LIVE) {
- if (priv->iothreadpids == NULL) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("IOThread affinity is not
supported"));
- goto endjob;
- }
+ virDomainIOThreadIDDefPtr iothrid;
- if (iothread_id > priv->niothreadpids) {
+ if (!(iothrid = virDomainIOThreadIDFind(vm->def, iothread_id))) {
virReportError(VIR_ERR_INVALID_ARG,
- _("iothread value out of range %d > %d"),
- iothread_id, priv->niothreadpids);
+ _("iothread value %d not found"), iothread_id);
goto endjob;
}
@@ -6099,8 +6096,7 @@ qemuDomainPinIOThread(virDomainPtr dom,
goto endjob;
}
} else {
- if (virProcessSetAffinity(priv->iothreadpids[iothread_id - 1],
- pcpumap) < 0) {
+ if (virProcessSetAffinity(iothrid->thread_id, pcpumap) < 0) {
virReportError(VIR_ERR_SYSTEM_ERROR,
_("failed to set cpu affinity for IOThread
%d"),
iothread_id);
@@ -10049,8 +10045,9 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm,
virCgroupFree(&cgroup_temp);
}
- for (i = 0; i < priv->niothreadpids; i++) {
- if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, i + 1,
+ for (i = 0; i < vm->def->niothreadids; i++) {
+ if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD,
+ vm->def->iothreadids[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 d9611c9..e0b3ce2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2267,12 +2267,16 @@ qemuProcessDetectIOThreadPIDs(virQEMUDriverPtr driver,
goto cleanup;
}
- if (VIR_ALLOC_N(priv->iothreadpids, niothreads) < 0)
- goto cleanup;
- priv->niothreadpids = niothreads;
+ for (i = 0; i < vm->def->niothreadids; i++) {
+ unsigned int iothread_id;
- for (i = 0; i < priv->niothreadpids; i++)
- priv->iothreadpids[i] = iothreads[i]->thread_id;
+ if (qemuDomainParseIOThreadAlias(iothreads[i]->name,
+ &iothread_id) < 0)
+ goto cleanup;
+
+ vm->def->iothreadids[i]->thread_id = iothreads[i]->thread_id;
+ vm->def->iothreadids[i]->iothread_id = iothread_id;
+ }
ret = 0;
@@ -2447,7 +2451,6 @@ qemuProcessSetEmulatorAffinity(virDomainObjPtr vm)
static int
qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm)
{
- qemuDomainObjPrivatePtr priv = vm->privateData;
virDomainDefPtr def = vm->def;
virDomainPinDefPtr pininfo;
size_t i;
@@ -2456,20 +2459,15 @@ qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm)
if (!def->cputune.niothreadspin)
return 0;
- if (priv->iothreadpids == NULL) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("IOThread affinity is not
supported"));
- return -1;
- }
-
- for (i = 0; i < def->iothreads; i++) {
- /* set affinity only for existing vcpus */
+ for (i = 0; i < def->niothreadids; i++) {
+ /* set affinity only for existing iothreads */
if (!(pininfo = virDomainPinFind(def->cputune.iothreadspin,
def->cputune.niothreadspin,
- i + 1)))
+ def->iothreadids[i]->iothread_id)))
continue;
- if (virProcessSetAffinity(priv->iothreadpids[i], pininfo->cpumask) < 0)
+ if (virProcessSetAffinity(def->iothreadids[i]->thread_id,
+ pininfo->cpumask) < 0)
goto cleanup;
}
ret = 0;
@@ -2519,8 +2517,9 @@ qemuProcessSetSchedulers(virDomainObjPtr vm)
return -1;
}
- for (i = 0; i < priv->niothreadpids; i++) {
- if (qemuProcessSetSchedParams(i + 1, priv->iothreadpids[i],
+ for (i = 0; i < vm->def->niothreadids; i++) {
+ if (qemuProcessSetSchedParams(vm->def->iothreadids[i]->iothread_id,
+ vm->def->iothreadids[i]->thread_id,
vm->def->cputune.niothreadsched,
vm->def->cputune.iothreadsched) < 0)
return -1;
@@ -5308,8 +5307,6 @@ void qemuProcessStop(virQEMUDriverPtr driver,
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
VIR_FREE(priv->vcpupids);
priv->nvcpupids = 0;
- VIR_FREE(priv->iothreadpids);
- priv->niothreadpids = 0;
virObjectUnref(priv->qemuCaps);
priv->qemuCaps = NULL;
VIR_FREE(priv->pidfile);
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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 8d0a4aa..57de961 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1229,6 +1229,8 @@ 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-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 5a5812f..09f6d8d 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -493,6 +493,8 @@ mymain(void)
DO_TEST("smp");
DO_TEST("iothreads");
+ DO_TEST("iothreads-ids");
+ DO_TEST("iothreads-ids-partial");
DO_TEST_DIFFERENT("cputune-iothreads");
DO_TEST("iothreads-disk");
DO_TEST("iothreads-disk-virtio-ccw");
--
2.1.0
8:18 p.m.
New subject: [libvirt] [PATCH v3 3/6] Implement virDomainAddIOThread and virDomainDelIOThread
Add libvirt API's to manage adding and deleting IOThreads to/from the
domain
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
include/libvirt/libvirt-domain.h | 6 ++
src/driver-hypervisor.h | 12 ++++
src/libvirt-domain.c | 118 +++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 6 ++
4 files changed, 142 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 7be4219..21b06b4 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1615,6 +1615,12 @@ int virDomainPinIOThread(virDomainPtr domain,
unsigned char *cpumap,
int maplen,
unsigned int flags);
+int virDomainAddIOThread(virDomainPtr domain,
+ unsigned int iothread_id,
+ 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..8b8d031 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -393,6 +393,16 @@ typedef int
unsigned int flags);
typedef int
+(*virDrvDomainAddIOThread)(virDomainPtr domain,
+ unsigned int iothread_id,
+ unsigned int flags);
+
+typedef int
+(*virDrvDomainDelIOThread)(virDomainPtr domain,
+ unsigned int iothread_id,
+ unsigned int flags);
+
+typedef int
(*virDrvDomainGetSecurityLabel)(virDomainPtr domain,
virSecurityLabelPtr seclabel);
@@ -1273,6 +1283,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 ccacdb4..ec5e2ff 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -8020,6 +8020,124 @@ virDomainPinIOThread(virDomainPtr domain,
/**
+ * virDomainAddIOThread:
+ * @domain: a domain object
+ * @iothread_id: the specific IOThread ID value to add
+ * @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.
+ *
+ * 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 requires privileged access to the hypervisor.
+ *
+ * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG.
+ * Both flags may be set.
+ * If VIR_DOMAIN_AFFECT_LIVE is set, the change affects a running domain
+ * and may fail if domain is not alive.
+ * If VIR_DOMAIN_AFFECT_CONFIG is set, the change affects persistent state,
+ * and will fail for transient domains. 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.
+ *
+ * Returns 0 in case of success, -1 in case of failure.
+ */
+int
+virDomainAddIOThread(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);
+
+ conn = domain->conn;
+
+ if (conn->driver->domainAddIOThread) {
+ int ret;
+ ret = conn->driver->domainAddIOThread(domain, iothread_id, 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 requires privileged access to the hypervisor.
+ *
+ * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG.
+ * Both flags may be set.
+ * If VIR_DOMAIN_AFFECT_LIVE is set, the change affects a running domain
+ * and may fail if domain is not alive.
+ * If VIR_DOMAIN_AFFECT_CONFIG is set, the change affects persistent state,
+ * and will fail for transient domains. 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.
+ *
+ * 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);
+
+ 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
8:18 p.m.
New subject: [libvirt] [PATCH v3 4/6] remote: Add support for AddIOThread and DelIOThread
Add remote support for the add/delete IOThread API's
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/remote/remote_driver.c | 2 ++
src/remote/remote_protocol.x | 30 +++++++++++++++++++++++++++++-
src/remote_protocol-structs | 12 ++++++++++++
3 files changed, 43 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..c0b3eff 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -1212,6 +1212,18 @@ struct remote_domain_pin_iothread_args {
unsigned int flags;
};
+struct remote_domain_add_iothread_args {
+ remote_nonnull_domain dom;
+ unsigned int iothread_id;
+ 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 +5655,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..95b9bdb 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -834,6 +834,16 @@ struct remote_domain_pin_iothread_args {
} cpumap;
u_int flags;
};
+struct remote_domain_add_iothread_args {
+ remote_nonnull_domain dom;
+ u_int iothread_id;
+ 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 +3027,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
8:18 p.m.
New subject: [libvirt] [PATCH v3 5/6] qemu: Add support to Add/Delete IOThreads
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(a)redhat.com>
---
src/conf/domain_audit.c | 9 +
src/conf/domain_audit.h | 6 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 431 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 447 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 12fd9e9..37e1fbb 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 008258f..f42d4fb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6179,6 +6179,435 @@ qemuDomainPinIOThread(virDomainPtr dom,
return ret;
}
+static int
+qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ unsigned int iothread_id)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ char *alias = NULL;
+ size_t 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;
+ unsigned int idval = 0;
+ virCgroupPtr cgroup_iothread = NULL;
+ char *mem_mask = NULL;
+ virDomainIOThreadIDDefPtr iothrid;
+
+ if (virDomainIOThreadIDFind(vm->def, iothread_id)) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("an IOThread is already using iothread_id
'%u'"),
+ iothread_id);
+ goto cleanup;
+ }
+
+ if (virAsprintf(&alias, "iothread%u", iothread_id) < 0)
+ return -1;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ rc = qemuMonitorAddObject(priv->mon, "iothread", alias, NULL);
+ 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 add the thread_id to the vm->def->iothreadids list.
+ */
+ if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon,
+ &new_iothreads)) < 0)
+ goto exit_monitor;
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto cleanup;
+
+ 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 we've successfully added an IOThread, find out where we added it
+ * in the QEMU IOThread list, so we can add it to our iothreadids list
+ */
+ for (idx = 0; idx < new_niothreads; idx++) {
+ if (qemuDomainParseIOThreadAlias(new_iothreads[idx]->name, &idval) <
0)
+ goto cleanup;
+ if (iothread_id == idval)
+ break;
+ }
+
+ if (idval != iothread_id) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot find new IOThread '%u' in QEMU
monitor."),
+ iothread_id);
+ goto cleanup;
+ }
+
+ if (virDomainIOThreadIDAdd(vm->def, iothread_id) < 0)
+ goto cleanup;
+
+ if (!(iothrid = virDomainIOThreadIDFind(vm->def, iothread_id))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot find just added IOThread '%u'"),
+ iothread_id);
+ goto cleanup;
+ }
+
+ iothrid->thread_id = new_iothreads[idx]->thread_id;
+
+ /* Add IOThread to cgroup if present */
+ if (priv->cgroup) {
+ cgroup_iothread =
+ qemuDomainAddCgroupForThread(priv->cgroup,
+ VIR_CGROUP_THREAD_IOTHREAD,
+ iothread_id, mem_mask,
+ iothrid->thread_id);
+ if (!cgroup_iothread)
+ 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)
+
+ goto cleanup;
+
+ if (qemuDomainHotplugPinThread(vm->def->cpumask, iothread_id,
+ iothrid->thread_id, cgroup_iothread) < 0)
+ goto cleanup;
+
+ if (qemuProcessSetSchedParams(iothread_id, iothrid->thread_id,
+ vm->def->cputune.niothreadsched,
+ vm->def->cputune.iothreadsched) < 0)
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ if (new_iothreads) {
+ for (idx = 0; idx < new_niothreads; idx++)
+ qemuMonitorIOThreadInfoFree(new_iothreads[idx]);
+ 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
+qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ unsigned int iothread_id)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ size_t idx;
+ char *alias = NULL;
+ 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;
+ char *mem_mask = NULL;
+
+ /* Normally would use virDomainIOThreadIDFind, but we need the index
+ * from whence to delete for later...
+ */
+ for (idx = 0; idx < vm->def->niothreadids; idx++) {
+ if (iothread_id == vm->def->iothreadids[idx]->iothread_id)
+ break;
+ }
+
+ if (idx == vm->def->niothreadids) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("cannot find IOThread '%u' in iothreadids
list"),
+ iothread_id);
+ return -1;
+ }
+
+ if (virAsprintf(&alias, "iothread%u", iothread_id) < 0)
+ return -1;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ 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 vm->def->iothreadids list.
+ */
+ if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon,
+ &new_iothreads)) < 0)
+ goto exit_monitor;
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto cleanup;
+
+ 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 (VIR_DELETE_ELEMENT(vm->def->iothreadids, idx,
+ vm->def->niothreadids) < 0)
+ goto cleanup;
+
+ if (qemuDomainDelCgroupForThread(priv->cgroup,
+ VIR_CGROUP_THREAD_IOTHREAD,
+ iothread_id) < 0)
+ goto cleanup;
+
+ virDomainPinDel(&vm->def->cputune.iothreadspin,
+ &vm->def->cputune.niothreadspin,
+ iothread_id);
+
+ ret = 0;
+
+ cleanup:
+ if (new_iothreads) {
+ for (idx = 0; idx < new_niothreads; idx++)
+ qemuMonitorIOThreadInfoFree(new_iothreads[idx]);
+ VIR_FREE(new_iothreads);
+ }
+ VIR_FREE(mem_mask);
+ virDomainAuditIOThread(vm, orig_niothreads, new_niothreads,
+ "update", rc == 0);
+ 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,
+ 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;
+ int ret = -1;
+
+ cfg = virQEMUDriverGetConfig(driver);
+
+ if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+ goto cleanup;
+
+ priv = vm->privateData;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags,
+ &persistentDef) < 0)
+ goto endjob;
+
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot change IOThreads for an inactive
domain"));
+ goto endjob;
+ }
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("IOThreads not supported with this binary"));
+ goto endjob;
+ }
+
+ 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 (add) {
+ if (qemuDomainHotplugAddIOThread(driver, vm, iothread_id) < 0)
+ goto endjob;
+ } else {
+ if (qemuDomainHotplugDelIOThread(driver, vm, iothread_id) < 0)
+ goto endjob;
+ }
+
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
+ goto endjob;
+ }
+
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (add) {
+ if (virDomainIOThreadIDAdd(persistentDef, iothread_id) < 0)
+ goto endjob;
+
+ /* Nothing to do in iothreadspin list (that's a separate command) */
+
+ persistentDef->iothreads++;
+ } else {
+ if (!virDomainIOThreadIDFind(persistentDef, iothread_id)) {
+ 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,
+ 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, 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;
+ size_t i;
+
+ 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;
+
+ /* 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;
+ }
+ }
+
+ ret = qemuDomainChgIOThread(driver, vm, iothread_id, false, flags);
+
+ cleanup:
+ qemuDomObjEndAPI(&vm);
+ return ret;
+}
+
static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr seclabel)
{
virQEMUDriverPtr driver = dom->conn->privateData;
@@ -19910,6 +20339,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
8:45 a.m.
New subject: [libvirt] [PATCH v3 5/6] qemu: Add support to Add/Delete IOThreads
On Tue, Apr 14, 2015 at 21:18:25 -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(a)redhat.com>
---
src/conf/domain_audit.c | 9 +
src/conf/domain_audit.h | 6 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 431 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 447 insertions(+)
...
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 008258f..f42d4fb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6179,6 +6179,435 @@ qemuDomainPinIOThread(virDomainPtr dom,
return ret;
}
+static int
+qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ unsigned int iothread_id)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ char *alias = NULL;
+ size_t 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;
+ unsigned int idval = 0;
+ virCgroupPtr cgroup_iothread = NULL;
+ char *mem_mask = NULL;
+ virDomainIOThreadIDDefPtr iothrid;
+
+ if (virDomainIOThreadIDFind(vm->def, iothread_id)) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("an IOThread is already using iothread_id
'%u'"),
+ iothread_id);
+ goto cleanup;
+ }
+
+ if (virAsprintf(&alias, "iothread%u", iothread_id) < 0)
+ return -1;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ rc = qemuMonitorAddObject(priv->mon, "iothread", alias, NULL);
+ 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 add the thread_id to the vm->def->iothreadids list.
+ */
+ if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon,
+ &new_iothreads)) < 0)
Since we are not doing any fancy iothread naming, this function can
parse the iothread IDs from the alias right away ... [1]
+ goto exit_monitor;
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto cleanup;
+
+ 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 we've successfully added an IOThread, find out where we added it
+ * in the QEMU IOThread list, so we can add it to our iothreadids list
+ */
The message seems obvious when looking at the code.
+ for (idx = 0; idx < new_niothreads; idx++) {
+ if (qemuDomainParseIOThreadAlias(new_iothreads[idx]->name, &idval) <
0)
... [1] so that you don't have to do it manually.
+ goto cleanup;
+ if (iothread_id == idval)
+ break;
+ }
+
+ if (idval != iothread_id) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot find new IOThread '%u' in QEMU
monitor."),
+ iothread_id);
+ goto cleanup;
+ }
+
+ if (virDomainIOThreadIDAdd(vm->def, iothread_id) < 0)
virDomainIOThreadIDAdd could return the pointer to the created item ...
+ goto cleanup;
+
+ if (!(iothrid = virDomainIOThreadIDFind(vm->def, iothread_id))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot find just added IOThread '%u'"),
+ iothread_id);
So that you don't have to look it up right after adding it.
+ goto cleanup;
+ }
+
+ iothrid->thread_id = new_iothreads[idx]->thread_id;
+
+ /* Add IOThread to cgroup if present */
+ if (priv->cgroup) {
+ cgroup_iothread =
+ qemuDomainAddCgroupForThread(priv->cgroup,
+ VIR_CGROUP_THREAD_IOTHREAD,
+ iothread_id, mem_mask,
+ iothrid->thread_id);
+ if (!cgroup_iothread)
+ goto cleanup;
+ }
+
+ /* Inherit def->cpuset */
+ if (vm->def->cpumask) {
Automatic NUMA placement(priv->autoCpuset) needs to be taken into account too.
+ if (qemuDomainHotplugAddPin(vm->def->cpumask,
iothread_id,
+ &vm->def->cputune.iothreadspin,
+ &vm->def->cputune.niothreadspin) < 0)
+
+ goto cleanup;
+
+ if (qemuDomainHotplugPinThread(vm->def->cpumask, iothread_id,
+ iothrid->thread_id, cgroup_iothread) < 0)
+ goto cleanup;
+
+ if (qemuProcessSetSchedParams(iothread_id, iothrid->thread_id,
+ vm->def->cputune.niothreadsched,
+ vm->def->cputune.iothreadsched) < 0)
qemuProcessSetSchedParams won't do anything since the new thread doesn't
have any scheduler assigned.
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ if (new_iothreads) {
+ for (idx = 0; idx < new_niothreads; idx++)
+ qemuMonitorIOThreadInfoFree(new_iothreads[idx]);
+ VIR_FREE(new_iothreads);
+ }
+ VIR_FREE(mem_mask);
+ virDomainAuditIOThread(vm, orig_niothreads, new_niothreads,
+ "update", rc == 0);
+ if (cgroup_iothread)
+ virCgroupFree(&cgroup_iothread);
virCgroupFree() handles NULL just fine.
+ VIR_FREE(alias);
+ return ret;
+
+ exit_monitor:
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+ goto cleanup;
+}
+
+static int
+qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ unsigned int iothread_id)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ size_t idx;
+ char *alias = NULL;
+ 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;
+ char *mem_mask = NULL;
+
+ /* Normally would use virDomainIOThreadIDFind, but we need the index
+ * from whence to delete for later...
+ */
+ for (idx = 0; idx < vm->def->niothreadids; idx++) {
+ if (iothread_id == vm->def->iothreadids[idx]->iothread_id)
+ break;
+ }
+
+ if (idx == vm->def->niothreadids) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("cannot find IOThread '%u' in iothreadids
list"),
+ iothread_id);
+ return -1;
+ }
+
+ if (virAsprintf(&alias, "iothread%u", iothread_id) < 0)
+ return -1;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ 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 vm->def->iothreadids list.
+ */
You've removed the thread here, so thread affinity was destroyed by the
thread exitting.
+ if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon,
+ &new_iothreads)) < 0)
+ goto exit_monitor;
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto cleanup;
+
+ 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)
Why do you need the memory node mask when you are deleting the cgroup?
+ goto cleanup;
+
+ if (VIR_DELETE_ELEMENT(vm->def->iothreadids, idx,
+ vm->def->niothreadids) < 0)
+ goto cleanup;
You've added virDomainIOThreadIDDel
+
+ if (qemuDomainDelCgroupForThread(priv->cgroup,
+ VIR_CGROUP_THREAD_IOTHREAD,
+ iothread_id) < 0)
+ goto cleanup;
+
+ virDomainPinDel(&vm->def->cputune.iothreadspin,
+ &vm->def->cputune.niothreadspin,
+ iothread_id);
+
+ ret = 0;
+
+ cleanup:
+ if (new_iothreads) {
+ for (idx = 0; idx < new_niothreads; idx++)
+ qemuMonitorIOThreadInfoFree(new_iothreads[idx]);
+ VIR_FREE(new_iothreads);
+ }
+ VIR_FREE(mem_mask);
+ virDomainAuditIOThread(vm, orig_niothreads, new_niothreads,
+ "update", rc == 0);
+ 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,
+ 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;
+ int ret = -1;
+
+ cfg = virQEMUDriverGetConfig(driver);
+
+ if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+ goto cleanup;
+
+ priv = vm->privateData;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags,
+ &persistentDef) < 0)
+ goto endjob;
+
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (!virDomainObjIsActive(vm)) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot change IOThreads for an inactive
domain"));
+ goto endjob;
+ }
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("IOThreads not supported with this binary"));
+ goto endjob;
+ }
+
+ if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
Wrong cgroup type. Additionally qemuDomainHotplugAddIOThread() will add
the thread so adding it here doesn't make sense.
+ 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 (add) {
+ if (qemuDomainHotplugAddIOThread(driver, vm, iothread_id) < 0)
+ goto endjob;
+ } else {
+ if (qemuDomainHotplugDelIOThread(driver, vm, iothread_id) < 0)
+ goto endjob;
+ }
+
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
+ goto endjob;
+ }
+
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (add) {
+ if (virDomainIOThreadIDAdd(persistentDef, iothread_id) < 0)
+ goto endjob;
+
+ /* Nothing to do in iothreadspin list (that's a separate command) */
+
+ persistentDef->iothreads++;
+ } else {
+ if (!virDomainIOThreadIDFind(persistentDef, iothread_id)) {
+ 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);
This is the reason why I've requested in the previous review that the
pinning information would be merged into the iothread data structure.
You then would not have to synchronise two data structures.
+ 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,
+ 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, 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;
+ size_t i;
+
+ 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;
+
+ /* 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));
Alternatively you can use vm->def->disks[i]->dst which should be always
set.
+ goto cleanup;
+ }
+ }
+
+ ret = qemuDomainChgIOThread(driver, vm, iothread_id, false, flags);
+
+ cleanup:
+ qemuDomObjEndAPI(&vm);
+ return ret;
+}
+
static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr seclabel)
{
virQEMUDriverPtr driver = dom->conn->privateData;
9:12 a.m.
New subject: [libvirt] [PATCH v3 5/6] qemu: Add support to Add/Delete IOThreads
On Tue, Apr 21, 2015 at 15:45:48 +0200, Peter Krempa wrote:
On Tue, Apr 14, 2015 at 21:18:25 -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(a)redhat.com>
> ---
> src/conf/domain_audit.c | 9 +
> src/conf/domain_audit.h | 6 +
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_driver.c | 431 +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 447 insertions(+)
>
...
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 008258f..f42d4fb 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6179,6 +6179,435 @@ qemuDomainPinIOThread(virDomainPtr dom,
...
> +
> + if (virDomainIOThreadIDAdd(vm->def, iothread_id) < 0)
virDomainIOThreadIDAdd could return the pointer to the created item ...
> + goto cleanup;
> +
> + if (!(iothrid = virDomainIOThreadIDFind(vm->def, iothread_id))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("cannot find just added IOThread
'%u'"),
> + iothread_id);
So that you don't have to look it up right after adding it.
> + goto cleanup;
> + }
> +
> + iothrid->thread_id = new_iothreads[idx]->thread_id;
You are also not marking the thread structure as 'defined' and thus
wouldn't format it later ...
> +
> + /* Add IOThread to cgroup if present */
> + if (priv->cgroup) {
> + cgroup_iothread =
> + qemuDomainAddCgroupForThread(priv->cgroup,
> + VIR_CGROUP_THREAD_IOTHREAD,
> + iothread_id, mem_mask,
> + iothrid->thread_id);
> + if (!cgroup_iothread)
> + goto cleanup;
> + }
> +
> + /* Inherit def->cpuset */
> + if (vm->def->cpumask) {
Automatic NUMA placement(priv->autoCpuset) needs to be taken into account too.
> + if (qemuDomainHotplugAddPin(vm->def->cpumask, iothread_id,
> + &vm->def->cputune.iothreadspin,
> + &vm->def->cputune.niothreadspin) <
0)
> +
> + goto cleanup;
> +
> + if (qemuDomainHotplugPinThread(vm->def->cpumask, iothread_id,
> + iothrid->thread_id, cgroup_iothread) <
0)
> + goto cleanup;
> +
> + if (qemuProcessSetSchedParams(iothread_id, iothrid->thread_id,
> + vm->def->cputune.niothreadsched,
> + vm->def->cputune.iothreadsched) <
0)
qemuProcessSetSchedParams won't do anything since the new thread doesn't
have any scheduler assigned.
> + goto cleanup;
> + }
> +
> + ret = 0;
> +
> + cleanup:
> + if (new_iothreads) {
> + for (idx = 0; idx < new_niothreads; idx++)
> + qemuMonitorIOThreadInfoFree(new_iothreads[idx]);
> + VIR_FREE(new_iothreads);
> + }
> + VIR_FREE(mem_mask);
> + virDomainAuditIOThread(vm, orig_niothreads, new_niothreads,
> + "update", rc == 0);
> + if (cgroup_iothread)
> + virCgroupFree(&cgroup_iothread);
virCgroupFree() handles NULL just fine.
> + VIR_FREE(alias);
> + return ret;
> +
> + exit_monitor:
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
> + goto cleanup;
> +}
Peter
12:26 p.m.
New subject: [libvirt] [PATCH v3 5/6] qemu: Add support to Add/Delete IOThreads
>> + goto cleanup;
>> + }
>> +
>> + iothrid->thread_id = new_iothreads[idx]->thread_id;
You are also not marking the thread structure as 'defined' and thus
wouldn't format it later ...
Changed to 'undefined' now, so this is unnecessary
(although perhaps 'autofill' is a better term....
John
12:24 p.m.
New subject: [libvirt] [PATCH v3 5/6] qemu: Add support to Add/Delete IOThreads
On 04/21/2015 09:45 AM, Peter Krempa wrote:
On Tue, Apr 14, 2015 at 21:18:25 -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(a)redhat.com>
> ---
> src/conf/domain_audit.c | 9 +
> src/conf/domain_audit.h | 6 +
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_driver.c | 431 +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 447 insertions(+)
>
...
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 008258f..f42d4fb 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6179,6 +6179,435 @@ qemuDomainPinIOThread(virDomainPtr dom,
> return ret;
> }
>
> +static int
> +qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + unsigned int iothread_id)
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + char *alias = NULL;
> + size_t 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;
> + unsigned int idval = 0;
> + virCgroupPtr cgroup_iothread = NULL;
> + char *mem_mask = NULL;
> + virDomainIOThreadIDDefPtr iothrid;
> +
> + if (virDomainIOThreadIDFind(vm->def, iothread_id)) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("an IOThread is already using iothread_id
'%u'"),
> + iothread_id);
> + goto cleanup;
> + }
> +
> + if (virAsprintf(&alias, "iothread%u", iothread_id) < 0)
> + return -1;
> +
> + qemuDomainObjEnterMonitor(driver, vm);
> +
> + rc = qemuMonitorAddObject(priv->mon, "iothread", alias, NULL);
> + 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 add the thread_id to the vm->def->iothreadids list.
> + */
> + if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon,
> + &new_iothreads)) < 0)
Since we are not doing any fancy iothread naming, this function can
parse the iothread IDs from the alias right away ... [1]
> + goto exit_monitor;
> +
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + goto cleanup;
> +
> + 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 we've successfully added an IOThread, find out where we added it
> + * in the QEMU IOThread list, so we can add it to our iothreadids list
> + */
The message seems obvious when looking at the code.
> + for (idx = 0; idx < new_niothreads; idx++) {
> + if (qemuDomainParseIOThreadAlias(new_iothreads[idx]->name, &idval)
< 0)
... [1] so that you don't have to do it manually.
IOW:
if (STREQ(alias, new_iothreads[idx]->name))
break
> + goto cleanup;
> + if (iothread_id == idval)
> + break;
> + }
> +
> + if (idval != iothread_id) {
And of course idx != new_niothreads, plus removing 'idval'
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("cannot find new IOThread '%u' in QEMU
monitor."),
> + iothread_id);
> + goto cleanup;
> + }
> +
> + if (virDomainIOThreadIDAdd(vm->def, iothread_id) < 0)
virDomainIOThreadIDAdd could return the pointer to the created item ...
OK, now we have
if (!(iothrid = virDomainIOThreadIDAdd(vm->def, iothread_id)))
goto cleanup;
iothrid->thread_id = new_iothreads[idx]->thread_id;
> + goto cleanup;
> +
> + if (!(iothrid = virDomainIOThreadIDFind(vm->def, iothread_id))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("cannot find just added IOThread '%u'"),
> + iothread_id);
So that you don't have to look it up right after adding it.
> + goto cleanup;
> + }
> +
> + iothrid->thread_id = new_iothreads[idx]->thread_id;
> +
> + /* Add IOThread to cgroup if present */
> + if (priv->cgroup) {
> + cgroup_iothread =
> + qemuDomainAddCgroupForThread(priv->cgroup,
> + VIR_CGROUP_THREAD_IOTHREAD,
> + iothread_id, mem_mask,
> + iothrid->thread_id);
> + if (!cgroup_iothread)
> + goto cleanup;
> + }
> +
> + /* Inherit def->cpuset */
> + if (vm->def->cpumask) {
Automatic NUMA placement(priv->autoCpuset) needs to be taken into account too.
Meaning? There's a reference for that in qemuDomainAddCgroupForThread.
Obviously I'm following the model of qemuDomainHotplugVcpus when
"if (nvcpus > oldvcpus) {"
> + if (qemuDomainHotplugAddPin(vm->def->cpumask,
iothread_id,
> + &vm->def->cputune.iothreadspin,
> + &vm->def->cputune.niothreadspin) <
0)
> +
> + goto cleanup;
> +
> + if (qemuDomainHotplugPinThread(vm->def->cpumask, iothread_id,
> + iothrid->thread_id, cgroup_iothread) <
0)
> + goto cleanup;
> +
> + if (qemuProcessSetSchedParams(iothread_id, iothrid->thread_id,
> + vm->def->cputune.niothreadsched,
> + vm->def->cputune.iothreadsched) < 0)
qemuProcessSetSchedParams won't do anything since the new thread doesn't
have any scheduler assigned.
So it's wrong in qemuDomainHotplugVcpus ? Is someone else fixing that?
Is the expectation that I just remove this?
> + goto cleanup;
> + }
> +
> + ret = 0;
> +
> + cleanup:
> + if (new_iothreads) {
> + for (idx = 0; idx < new_niothreads; idx++)
> + qemuMonitorIOThreadInfoFree(new_iothreads[idx]);
> + VIR_FREE(new_iothreads);
> + }
> + VIR_FREE(mem_mask);
> + virDomainAuditIOThread(vm, orig_niothreads, new_niothreads,
> + "update", rc == 0);
> + if (cgroup_iothread)
> + virCgroupFree(&cgroup_iothread);
virCgroupFree() handles NULL just fine.
OK - again copied from qemuDomainHotplugVcpus
> + VIR_FREE(alias);
> + return ret;
> +
> + exit_monitor:
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
> + goto cleanup;
> +}
> +
> +static int
> +qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + unsigned int iothread_id)
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + size_t idx;
> + char *alias = NULL;
> + 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;
> + char *mem_mask = NULL;
> +
> + /* Normally would use virDomainIOThreadIDFind, but we need the index
> + * from whence to delete for later...
> + */
> + for (idx = 0; idx < vm->def->niothreadids; idx++) {
> + if (iothread_id == vm->def->iothreadids[idx]->iothread_id)
> + break;
> + }
> +
> + if (idx == vm->def->niothreadids) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("cannot find IOThread '%u' in iothreadids
list"),
> + iothread_id);
> + return -1;
> + }
> +
> + if (virAsprintf(&alias, "iothread%u", iothread_id) < 0)
> + return -1;
> +
> + qemuDomainObjEnterMonitor(driver, vm);
> +
> + 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 vm->def->iothreadids list.
> + */
You've removed the thread here, so thread affinity was destroyed by the
thread exitting.
relic of me cut-n-paste from formerly combined code.
removed the comment completely, no replacement.
> + if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon,
> + &new_iothreads)) < 0)
> + goto exit_monitor;
> +
> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
> + goto cleanup;
> +
> + 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)
Why do you need the memory node mask when you are deleting the cgroup?
Oh right - relic of the copy... gone.
> + goto cleanup;
> +
> + if (VIR_DELETE_ELEMENT(vm->def->iothreadids, idx,
> + vm->def->niothreadids) < 0)
> + goto cleanup;
You've added virDomainIOThreadIDDel
yep, change to :
virDomainIOThreadIDDel(vm->def, iothread_id);
> +
> + if (qemuDomainDelCgroupForThread(priv->cgroup,
> + VIR_CGROUP_THREAD_IOTHREAD,
> + iothread_id) < 0)
> + goto cleanup;
> +
> + virDomainPinDel(&vm->def->cputune.iothreadspin,
> + &vm->def->cputune.niothreadspin,
> + iothread_id);
> +
> + ret = 0;
> +
> + cleanup:
> + if (new_iothreads) {
> + for (idx = 0; idx < new_niothreads; idx++)
> + qemuMonitorIOThreadInfoFree(new_iothreads[idx]);
> + VIR_FREE(new_iothreads);
> + }
> + VIR_FREE(mem_mask);
> + virDomainAuditIOThread(vm, orig_niothreads, new_niothreads,
> + "update", rc == 0);
> + 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,
> + 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;
> + int ret = -1;
> +
> + cfg = virQEMUDriverGetConfig(driver);
> +
> + if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> + goto cleanup;
> +
> + priv = vm->privateData;
> +
> + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> + goto cleanup;
> +
> + if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags,
> + &persistentDef) < 0)
> + goto endjob;
> +
> + if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> + if (!virDomainObjIsActive(vm)) {
> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("cannot change IOThreads for an inactive
domain"));
> + goto endjob;
> + }
> +
> + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("IOThreads not supported with this
binary"));
> + goto endjob;
> + }
> +
> + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
Wrong cgroup type. Additionally qemuDomainHotplugAddIOThread() will add
the thread so adding it here doesn't make sense.
See qemuDomainSetVcpusFlags, it too uses VIR_CGROUP_THREAD_EMULATOR so this
is no different... In fact I suppose I could have created some sort of common
API, but didn't.
One thing I just noticed - commit id '6cf1e11' added a "&&
virNumaIsAvailable()"
check in qemuDomainSetVcpusFlags, so I suppose to follow that - I can add
the extra check.
Beyond that I'm not sure what you want.
> + 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 (add) {
> + if (qemuDomainHotplugAddIOThread(driver, vm, iothread_id) < 0)
> + goto endjob;
> + } else {
> + if (qemuDomainHotplugDelIOThread(driver, vm, iothread_id) < 0)
> + goto endjob;
> + }
> +
> + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
> + goto endjob;
> + }
> +
> + if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> + if (add) {
> + if (virDomainIOThreadIDAdd(persistentDef, iothread_id) < 0)
> + goto endjob;
> +
> + /* Nothing to do in iothreadspin list (that's a separate command)
*/
> +
> + persistentDef->iothreads++;
> + } else {
> + if (!virDomainIOThreadIDFind(persistentDef, iothread_id)) {
> + 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);
This is the reason why I've requested in the previous review that the
pinning information would be merged into the iothread data structure.
You then would not have to synchronise two data structures.
Understood ... I started down that path, but then got bogged down in cputune
information inside iothreadid's and the difference with the vCPU code. So I
kept it this way to reduce the number of changes.
I think it's worthy of being done, but I hope a follow-up patch will be acceptable.
> + 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,
> + 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, 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;
> + size_t i;
> +
> + 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;
> +
> + /* 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));
Alternatively you can use vm->def->disks[i]->dst which should be always
set.
OK
> + goto cleanup;
> + }
> + }
> +
> + ret = qemuDomainChgIOThread(driver, vm, iothread_id, false, flags);
> +
> + cleanup:
> + qemuDomObjEndAPI(&vm);
> + return ret;
> +}
> +
> static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr
seclabel)
> {
> virQEMUDriverPtr driver = dom->conn->privateData;
So with all that the changes that would be sqashed in:
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5d3c7fc..e1fdeb5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6188,7 +6188,6 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
unsigned int exp_niothreads = vm->def->iothreads;
int new_niothreads = 0;
qemuMonitorIOThreadInfoPtr *new_iothreads = NULL;
- unsigned int idval = 0;
virCgroupPtr cgroup_iothread = NULL;
char *mem_mask = NULL;
virDomainIOThreadIDDefPtr iothrid;
@@ -6244,29 +6243,20 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
* in the QEMU IOThread list, so we can add it to our iothreadids list
*/
for (idx = 0; idx < new_niothreads; idx++) {
- if (qemuDomainParseIOThreadAlias(new_iothreads[idx]->name, &idval) <
0)
- goto cleanup;
- if (iothread_id == idval)
+ if (STREQ(new_iothreads[idx]->name, alias))
break;
}
- if (idval != iothread_id) {
+ if (idx != new_niothreads) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot find new IOThread '%u' in QEMU
monitor."),
iothread_id);
goto cleanup;
}
- if (virDomainIOThreadIDAdd(vm->def, iothread_id) < 0)
+ if (!(iothrid = virDomainIOThreadIDAdd(vm->def, iothread_id)))
goto cleanup;
- if (!(iothrid = virDomainIOThreadIDFind(vm->def, iothread_id))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot find just added IOThread '%u'"),
- iothread_id);
- goto cleanup;
- }
-
iothrid->thread_id = new_iothreads[idx]->thread_id;
/* Add IOThread to cgroup if present */
@@ -6309,8 +6299,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
VIR_FREE(mem_mask);
virDomainAuditIOThread(vm, orig_niothreads, new_niothreads,
"update", rc == 0);
- if (cgroup_iothread)
- virCgroupFree(&cgroup_iothread);
+ virCgroupFree(&cgroup_iothread);
VIR_FREE(alias);
return ret;
@@ -6333,7 +6322,6 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver,
unsigned int exp_niothreads = vm->def->iothreads;
int new_niothreads = 0;
qemuMonitorIOThreadInfoPtr *new_iothreads = NULL;
- char *mem_mask = NULL;
/* Normally would use virDomainIOThreadIDFind, but we need the index
* from whence to delete for later...
@@ -6360,10 +6348,6 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver,
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 vm->def->iothreadids list.
- */
if ((new_niothreads = qemuMonitorGetIOThreads(priv->mon,
&new_iothreads)) < 0)
goto exit_monitor;
@@ -6381,16 +6365,7 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver,
}
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 (VIR_DELETE_ELEMENT(vm->def->iothreadids, idx,
- vm->def->niothreadids) < 0)
- goto cleanup;
+ virDomainIOThreadIDDel(vm->def, iothread_id);
if (qemuDomainDelCgroupForThread(priv->cgroup,
VIR_CGROUP_THREAD_IOTHREAD,
@@ -6409,7 +6384,6 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver,
qemuMonitorIOThreadInfoFree(new_iothreads[idx]);
VIR_FREE(new_iothreads);
}
- VIR_FREE(mem_mask);
virDomainAuditIOThread(vm, orig_niothreads, new_niothreads,
"update", rc == 0);
VIR_FREE(alias);
@@ -6464,19 +6438,21 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver,
goto endjob;
}
- if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
- false, &cgroup_temp) < 0)
- goto endjob;
+ if (virNumaIsAvailable()) {
+ if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
+ false, &cgroup_temp) < 0)
+ goto endjob;
- if (!(all_nodes = virNumaGetHostNodeset()))
- goto endjob;
+ if (!(all_nodes = virNumaGetHostNodeset()))
+ goto endjob;
- if (!(all_nodes_str = virBitmapFormat(all_nodes)))
- 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 (virCgroupGetCpusetMems(cgroup_temp, &mem_mask) < 0 ||
+ virCgroupSetCpusetMems(cgroup_temp, all_nodes_str) < 0)
+ goto endjob;
+ }
if (add) {
if (qemuDomainHotplugAddIOThread(driver, vm, iothread_id) < 0)
@@ -6492,7 +6468,7 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver,
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
if (add) {
- if (virDomainIOThreadIDAdd(persistentDef, iothread_id) < 0)
+ if (!virDomainIOThreadIDAdd(persistentDef, iothread_id))
goto endjob;
/* Nothing to do in iothreadspin list (that's a separate command) */
@@ -6590,8 +6566,7 @@ qemuDomainDelIOThread(virDomainPtr dom,
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));
+ iothread_id, vm->def->disks[i]->dst);
goto cleanup;
}
}
12:53 p.m.
New subject: [libvirt] [PATCH v3 5/6] qemu: Add support to Add/Delete IOThreads
...
> This is the reason why I've requested in the previous review
that the
> pinning information would be merged into the iothread data structure.
> You then would not have to synchronise two data structures.
>
Understood ... I started down that path, but then got bogged down in cputune
information inside iothreadid's and the difference with the vCPU code. So I
kept it this way to reduce the number of changes.
I think it's worthy of being done, but I hope a follow-up patch will be acceptable.
ug... starting looking at it again with the "existing" view of
iothreadid's fresh in mind and it seems I'll have to jump in during this
series because there's a check in virDomainIOThreadPinDefParseXML for
pin->id > iothreads, which won't be true once "Add" hits, so I'll
work
it in now <sigh>
John
8:18 p.m.
New subject: [libvirt] [PATCH v3 6/6] virsh: Add iothreadadd and iothreaddel commands
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> [--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
--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
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(a)redhat.com>
---
tools/virsh-domain.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++
tools/virsh.pod | 31 ++++++++++
2 files changed, 195 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index bc6054a..680424f 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6883,6 +6883,158 @@ 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 = "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;
+ 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 (virDomainAddIOThread(dom, iothread_id, 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[] = {
@@ -12812,6 +12964,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..091abc6 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1416,6 +1416,37 @@ 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<--config>] [I<--live>] | [I<--current>]]
+
+Add a new IOThread to the domain using the specified I<iothread_id>.
+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
3503
days inactive
3509
days old
13 comments
2 participants
participants (2)
-
John Ferlan -
Peter Krempa