[libvirt] [PATCH v4 4/9] Move iothreadspin information into iothreadids
Peter Krempa
pkrempa at redhat.com
Thu Apr 23 13:46:18 UTC 2015
On Thu, Apr 23, 2015 at 08:58:26 -0400, John Ferlan wrote:
>
>
> On 04/23/2015 08:02 AM, Peter Krempa wrote:
> > On Tue, Apr 21, 2015 at 19:31:25 -0400, John Ferlan wrote:
> >> Remove the iothreadspin array from cputune and replace with a cpumask
> >> to be stored in the iothreadids list
> >>
> >> Signed-off-by: John Ferlan <jferlan at redhat.com>
> >> ---
> >> docs/formatdomain.html.in | 10 ++--
> >> src/conf/domain_conf.c | 118 +++++++++++++++++++++-------------------------
> >> src/conf/domain_conf.h | 2 +-
> >> src/qemu/qemu_cgroup.c | 13 ++---
> >> src/qemu/qemu_driver.c | 79 +++++++++----------------------
> >> src/qemu/qemu_process.c | 7 +--
> >> 6 files changed, 86 insertions(+), 143 deletions(-)
> >>
> >> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> >> index 518f7c5..7af6bd7 100644
> >> --- a/docs/formatdomain.html.in
> >> +++ b/docs/formatdomain.html.in
> >> @@ -624,11 +624,11 @@
> >> and attribute <code>cpuset</code> of element <code>vcpu</code> is
> >> not specified, the IOThreads are pinned to all the physical CPUs
> >> by default. There are two required attributes, the attribute
> >> - <code>iothread</code> specifies the IOThread id and the attribute
> >> - <code>cpuset</code> specifying which physical CPUs to pin to. The
> >> - <code>iothread</code> value begins at "1" through the number of
> >> - <a href="#elementsIOThreadsAllocation"><code>iothreads</code></a>
> >> - allocated to the domain. A value of "0" is not permitted.
> >> + <code>iothread</code> specifies the IOThread ID and the attribute
> >> + <code>cpuset</code> specifying which physical CPUs to pin to. See
> >> + the <code>iothreadids</code>
> >> + <a href="#elementsIOThreadsAllocation"><code>description</code></a>
> >> + for valid <code>iothread</code> values.
> >
> > This description is fair enough. It hints that the implicit ones are
> > numbered too.
> >
> >> <span class="since">Since 1.2.9</span>
> >> </dd>
> >> <dt><code>shares</code></dt>
> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> >> index bd25d52..969e56f 100644
> >> --- a/src/conf/domain_conf.c
> >> +++ b/src/conf/domain_conf.c
> >> @@ -2118,8 +2118,10 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin)
> >> void
> >> virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def)
> >> {
> >> - if (def)
> >> + if (def) {
> >> + virBitmapFree(def->cpumask);
> >> VIR_FREE(def);
> >
> > This fixes the syntax check failure from 1/9.
> >
>
> Byproduct of rewrites...
>
> I've change 1/9 to be (so that each patch passes check/syntax-check)
>
> + if (!def)
> + return;
> + VIR_FREE(def);
>
>
> Which will change this to be:
> if (!def)
> return;
> + virBitmapFree(def->cpumask);
> VIR_FREE(def);
>
>
>
> >> + }
> >> }
> >>
> >>
> >> @@ -2341,9 +2343,6 @@ void virDomainDefFree(virDomainDefPtr def)
> >>
> >> virDomainPinDefFree(def->cputune.emulatorpin);
> >>
> >> - virDomainPinDefArrayFree(def->cputune.iothreadspin,
> >> - def->cputune.niothreadspin);
> >> -
> >> for (i = 0; i < def->cputune.nvcpusched; i++)
> >> virBitmapFree(def->cputune.vcpusched[i].ids);
> >> VIR_FREE(def->cputune.vcpusched);
> >> @@ -13316,74 +13315,77 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node,
> >> * and an iothreadspin has the form
> >> * <iothreadpin iothread='1' cpuset='2'/>
> >> */
> >> -static virDomainPinDefPtr
> >> +static int
> >> virDomainIOThreadPinDefParseXML(xmlNodePtr node,
> >> xmlXPathContextPtr ctxt,
> >> - int iothreads)
> >> + virDomainDefPtr def)
> >> {
> >> - virDomainPinDefPtr def;
> >> + int ret = -1;
> >> + virDomainIOThreadIDDefPtr iothrid;
> >> + virBitmapPtr cpumask;
> >> xmlNodePtr oldnode = ctxt->node;
> >> unsigned int iothreadid;
> >> char *tmp = NULL;
> >>
> >> - if (VIR_ALLOC(def) < 0)
> >> - return NULL;
> >> -
> >> ctxt->node = node;
> >>
> >> if (!(tmp = virXPathString("string(./@iothread)", ctxt))) {
> >> virReportError(VIR_ERR_XML_ERROR, "%s",
> >> _("missing iothread id in iothreadpin"));
> >> - goto error;
> >> + goto cleanup;
> >> }
> >>
> >> if (virStrToLong_uip(tmp, NULL, 10, &iothreadid) < 0) {
> >> virReportError(VIR_ERR_XML_ERROR,
> >> _("invalid setting for iothread '%s'"), tmp);
> >> - goto error;
> >> + goto cleanup;
> >> }
> >> VIR_FREE(tmp);
> >>
> >> if (iothreadid == 0) {
> >> virReportError(VIR_ERR_XML_ERROR, "%s",
> >> _("zero is an invalid iothread id value"));
> >> - goto error;
> >> - }
> >> -
> >> - /* IOThreads are numbered "iothread1...iothread<n>", where
> >> - * "n" is the iothreads value */
> >> - if (iothreadid > iothreads) {
> >> - virReportError(VIR_ERR_XML_ERROR, "%s",
> >> - _("iothread id must not exceed iothreads"));
> >
> > [1]
> >
> >> - goto error;
> >> + goto cleanup;
> >> }
> >>
> >> - def->id = iothreadid;
> >> -
> >> if (!(tmp = virXMLPropString(node, "cpuset"))) {
> >> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >> _("missing cpuset for iothreadpin"));
> >> - goto error;
> >> + goto cleanup;
> >> }
> >>
> >> - if (virBitmapParse(tmp, 0, &def->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0)
> >> - goto error;
> >> + if (virBitmapParse(tmp, 0, &cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0)
> >> + goto cleanup;
> >>
> >> - if (virBitmapIsAllClear(def->cpumask)) {
> >> + if (virBitmapIsAllClear(cpumask)) {
> >> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> >> _("Invalid value of 'cpuset': %s"),
> >> tmp);
> >> - goto error;
> >> + goto cleanup;
> >> + }
> >> +
> >> + if (!(iothrid = virDomainIOThreadIDFind(def, iothreadid))) {
> >> + if (!(iothrid = virDomainIOThreadIDAdd(def, iothreadid)))
> >
> > Why are you adding a iothread definition here? This code is executed
> > after the iothread info should already be parsed so adding a thread here
> > doesn't make sense. The section here should ressemble the check [1] that
> > you've removed.
> >
>
> Because you required that the iothreadpin information to be included in
> the iothreadid; however, if someone hasn't defined <iothreadids>'s, then
> there won't be any until the PostParse is called - thus we have to add
> one here so that we can store the pin information for an iothread
Okay, I didn't see that because I've remembered a different version of
patch 1 where you allocated the full list upfront.
By the way, the approach in this series now has the following loophole:
1) Specify <iothreads> > nelemes(<iothreadids>)
2) Specify iothreadpin for thread 9999.
Now one of the threads is added with id 9999 due to the code above.
Thus I think that the PostParse callback code should probably be removed
and the missing threads should be numbered right after <iohtreadids> is
parsed so that this patch doesn't allow that.
>
> This means someone could have:
>
> <iothreads>4</iothreads>
> <cputune>
> ...
> <iothreadpin iothread="1" cpuset="5,6"/>
> ...
> </cputune>
>
> NOTE: No <iothreadids>
>
> So I believe it stays as is
>
> >> + goto cleanup;
> >> + iothrid->autofill = true;
> >> + }
> >> +
> >> + if (iothrid->cpumask) {
> >> + virReportError(VIR_ERR_INTERNAL_ERROR,
> >> + _("duplicate iothreadpin for same iothread '%u'"),
> >> + iothreadid);
> >> + goto cleanup;
> >> }
> >>
> >> + iothrid->cpumask = cpumask;
> >> + cpumask = NULL;
> >> + ret = 0;
> >> +
> >> cleanup:
> >> VIR_FREE(tmp);
> >> + virBitmapFree(cpumask);
> >> ctxt->node = oldnode;
> >> - return def;
> >> -
> >> - error:
> >> - VIR_FREE(def);
> >> - goto cleanup;
> >> + return ret;
> >> }
> >>
> >>
> >> @@ -14250,28 +14252,11 @@ virDomainDefParseXML(xmlDocPtr xml,
> >> goto error;
> >> }
> >>
> >> - if (n && VIR_ALLOC_N(def->cputune.iothreadspin, n) < 0)
> >> - goto error;
> >> -
> >> for (i = 0; i < n; i++) {
> >> - virDomainPinDefPtr iothreadpin = NULL;
> >> - iothreadpin = virDomainIOThreadPinDefParseXML(nodes[i], ctxt,
> >> - def->iothreads);
> >> - if (!iothreadpin)
> >> + if (virDomainIOThreadPinDefParseXML(nodes[i], ctxt, def) < 0)
> >> goto error;
> >> -
> >> - if (virDomainPinIsDuplicate(def->cputune.iothreadspin,
> >> - def->cputune.niothreadspin,
> >> - iothreadpin->id)) {
> >> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >> - _("duplicate iothreadpin for same iothread"));
> >> - virDomainPinDefFree(iothreadpin);
> >> - goto error;
> >> - }
> >> -
> >> - def->cputune.iothreadspin[def->cputune.niothreadspin++] =
> >> - iothreadpin;
> >> }
> >> + def->cputune.niothreadspin = n;
> >
> > This variable should be removed along with the iothread pinning
> > structure, shouldn't it?
> >
>
> OK
>
> >> VIR_FREE(nodes);
> >>
> >> if ((n = virXPathNodeSet("./cputune/vcpusched", ctxt, &nodes)) < 0) {
> >> @@ -14384,7 +14369,7 @@ virDomainDefParseXML(xmlDocPtr xml,
> >>
> >> if (virDomainNumatuneHasPlacementAuto(def->numa) &&
> >> !def->cpumask && !def->cputune.vcpupin &&
> >> - !def->cputune.emulatorpin && !def->cputune.iothreadspin)
> >> + !def->cputune.emulatorpin && !def->cputune.niothreadspin)
> >
> > This could be replaced by a function that checks whether any of the
> > threads has pinning info present rather than counting the pinning info.
> >
>
> OK
>
> >> def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO;
> >>
> >> if ((n = virXPathNodeSet("./resource", ctxt, &nodes)) < 0) {
> >> @@ -20837,20 +20822,25 @@ virDomainDefFormatInternal(virDomainDefPtr def,
> >> VIR_FREE(cpumask);
> >> }
> >>
> >> - for (i = 0; i < def->cputune.niothreadspin; i++) {
> >> - char *cpumask;
> >> - /* Ignore the iothreadpin which inherit from "cpuset of "<vcpu>." */
> >> - if (virBitmapEqual(def->cpumask, def->cputune.iothreadspin[i]->cpumask))
> >> - continue;
> >> + if (def->cputune.niothreadspin) {
> >
> > The check above is useless. If neither of the iothreads has pin info, it
> > would be skipped ...
> >
>
> yep
>
> >> + for (i = 0; i < def->niothreadids; i++) {
> >> + char *cpumask;
> >>
> >> - virBufferAsprintf(buf, "<iothreadpin iothread='%u' ",
> >> - def->cputune.iothreadspin[i]->id);
> >> + /* Ignore iothreadids with no cpumask or a cpumask that
> >> + * inherits from "cpuset of "<vcpu>." */
> >> + if (!def->iothreadids[i]->cpumask ||
> >> + virBitmapEqual(def->cpumask, def->iothreadids[i]->cpumask))
> >> + continue;
> >
> > ... here. Also why are you explicitly rejecting cpumask that is equal to
> > the default one? If a user explicitly specifies it that way then the
> > info would be lost. Additionally, def->cpumask here is used on the
> > iothread automatically without filling it to the structure if it's used
> > so there's no need to do that.
> >
>
> Prior review - it's already there - see commit id '938fb12f'.... I'm
> fairly certain I was specifically asked to do that...
It doesn't make much sense though. The default pinning is used from
def->cpumap if the specific is not present. If the specific pinning is
removed it should be freed.
>
> If it needs to change that should be a separate patch IMO... I think if
> you look a few lines above you'll see the same/similar check for vcpupin
>
> >>
> >> - if (!(cpumask = virBitmapFormat(def->cputune.iothreadspin[i]->cpumask)))
> >> - goto error;
> >> + virBufferAsprintf(buf, "<iothreadpin iothread='%u' ",
> >> + def->iothreadids[i]->iothread_id);
> >>
> >> - virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask);
> >> - VIR_FREE(cpumask);
> >> + if (!(cpumask = virBitmapFormat(def->iothreadids[i]->cpumask)))
> >> + goto error;
> >> +
> >> + virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask);
> >> + VIR_FREE(cpumask);
> >> + }
> >> }
> >>
> >> for (i = 0; i < def->cputune.nvcpusched; i++) {
> >> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> >> index cc98f3d..c71e1b8 100644
> >> --- a/src/conf/domain_conf.h
> >> +++ b/src/conf/domain_conf.h
> >> @@ -2057,6 +2057,7 @@ struct _virDomainIOThreadIDDef {
> >> bool autofill;
> >> unsigned int iothread_id;
> >> int thread_id;
> >> + virBitmapPtr cpumask;
> >> };
> >>
> >> void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def);
> >> @@ -2075,7 +2076,6 @@ struct _virDomainCputune {
> >> virDomainPinDefPtr *vcpupin;
> >> virDomainPinDefPtr emulatorpin;
> >> size_t niothreadspin;
> >
> > This one should be removed too.
> >
>
> yep
>
> >> - virDomainPinDefPtr *iothreadspin;
> >>
> >> size_t nvcpusched;
> >> virDomainThreadSchedParamPtr vcpusched;
> >> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> >> index cdf0aaf..5282449 100644
> >> --- a/src/qemu/qemu_cgroup.c
> >> +++ b/src/qemu/qemu_cgroup.c
> >> @@ -1149,7 +1149,7 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
> >> virCgroupPtr cgroup_iothread = NULL;
> >> qemuDomainObjPrivatePtr priv = vm->privateData;
> >> virDomainDefPtr def = vm->def;
> >> - size_t i, j;
> >> + size_t i;
> >> unsigned long long period = vm->def->cputune.period;
> >> long long quota = vm->def->cputune.quota;
> >> char *mem_mask = NULL;
> >> @@ -1214,18 +1214,11 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
> >> /* default cpu masks */
> >> if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
> >> cpumask = priv->autoCpuset;
> >> + else if (def->iothreadids[i]->cpumask)
> >> + cpumask = def->iothreadids[i]->cpumask;
> >
> > This has to be the first case. Even if
> > VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO is specified, it cannot override what
> > the user configured pinning.
> >
>
> OK, but see qemuSetupCgroupForVcpu and qemuSetupCgroupForEmulator...
> Vcpu has one way of doing it and Emulator slightly different, but both
> check AUTO first.
>
> VCPU checks AUTO first, then sets to default. Then it checks the vcpupin
> and resets to whatever is there.
>
> Emulator checks AUTO first, then it checks if emulatorpin is set and
> uses that, and finally falls back to default if defined.
Um, I made a mistake when I refactored qemuSetupCgroupForEmulator.
qemuSetupCgroupForVcpu is the right approach.
>
> I followed emulatorpin
>
> If I'm wrong, then emulator and vcpu are wrong, but I think fixing that
> is a separate patch.
yes.
>
> >> else
> >> cpumask = def->cpumask;
> >>
> >> - /* specific cpu mask */
> >> - for (j = 0; j < def->cputune.niothreadspin; j++) {
> >> - if (def->cputune.iothreadspin[j]->id ==
> >> - def->iothreadids[i]->iothread_id) {
> >> - cpumask = def->cputune.iothreadspin[j]->cpumask;
> >> - break;
> >> - }
> >
> > Finally, this can be removed!
> >
> >> - }
> >> -
> >> if (cpumask &&
> >> qemuSetupCgroupCpusetCpus(cgroup_iothread, cpumask) < 0)
> >> goto cleanup;
> >> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> >> index 0f95cc7..ee13d08 100644
> >> --- a/src/qemu/qemu_driver.c
> >> +++ b/src/qemu/qemu_driver.c
> >> @@ -5894,19 +5894,14 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef,
> >> goto cleanup;
> >>
> >> for (i = 0; i < targetDef->niothreadids; i++) {
> >> - virDomainPinDefPtr pininfo;
> >> -
> >> if (VIR_ALLOC(info_ret[i]) < 0)
> >> goto cleanup;
> >>
> >> /* 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,
> >> - targetDef->iothreadids[i]->iothread_id);
> >> - if (!pininfo) {
> >> + cpumask = targetDef->iothreadids[i]->cpumask;
> >> + if (!cpumask) {
> >> if (targetDef->cpumask) {
> >> cpumask = targetDef->cpumask;
> >> } else {
> >> @@ -5915,8 +5910,6 @@ qemuDomainGetIOThreadsConfig(virDomainDefPtr targetDef,
> >> virBitmapSetAll(bitmap);
> >> cpumask = bitmap;
> >> }
> >> - } else {
> >> - cpumask = pininfo->cpumask;
> >> }
> >> if (virBitmapToData(cpumask, &info_ret[i]->cpumap,
> >> &info_ret[i]->cpumaplen) < 0)
> >> @@ -5999,8 +5992,6 @@ qemuDomainPinIOThread(virDomainPtr dom,
> >> virDomainDefPtr persistentDef = NULL;
> >> virBitmapPtr pcpumap = NULL;
> >> qemuDomainObjPrivatePtr priv;
> >> - virDomainPinDefPtr *newIOThreadsPin = NULL;
> >> - size_t newIOThreadsPinNum = 0;
> >> virCgroupPtr cgroup_iothread = NULL;
> >> virObjectEventPtr event = NULL;
> >> char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = "";
> >> @@ -6050,33 +6041,21 @@ qemuDomainPinIOThread(virDomainPtr dom,
> >> if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> >>
> >> virDomainIOThreadIDDefPtr iothrid;
> >> + virBitmapPtr cpumask;
> >>
> >> if (!(iothrid = virDomainIOThreadIDFind(vm->def, iothread_id))) {
> >> virReportError(VIR_ERR_INVALID_ARG,
> >> - _("iothread value %d not found"), iothread_id);
> >> + _("iothreadid %d not found"), iothread_id);
> >
> > I'd compromise between those two.
> >
> > "iothread %d not foud"
> >
>
> OK
>
> >> goto endjob;
> >> }
> >>
> >> - if (vm->def->cputune.iothreadspin) {
> >> - newIOThreadsPin =
> >> - virDomainPinDefCopy(vm->def->cputune.iothreadspin,
> >> - vm->def->cputune.niothreadspin);
> >> - if (!newIOThreadsPin)
> >> - goto endjob;
> >> -
> >> - newIOThreadsPinNum = vm->def->cputune.niothreadspin;
> >> - } else {
> >> - if (VIR_ALLOC(newIOThreadsPin) < 0)
> >> - goto endjob;
> >> - newIOThreadsPinNum = 0;
> >> - }
> >> -
> >> - if (virDomainPinAdd(&newIOThreadsPin, &newIOThreadsPinNum,
> >> - cpumap, maplen, iothread_id) < 0) {
> >> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >> - _("failed to update iothreadspin"));
> >> + if (!(cpumask = virBitmapNewData(cpumap, maplen)))
> >> goto endjob;
> >> - }
> >> +
> >> + if (!iothrid->cpumask)
> >> + vm->def->cputune.niothreadspin++;
> >
> > This should go. If you add a function that checks whether pinning s
> > specified it will not be needed.
> >
>
> Yep
>
> >> + virBitmapFree(iothrid->cpumask);
> >> + iothrid->cpumask = cpumask;
> >>
> >> /* Configure the corresponding cpuset cgroup before set affinity. */
> >> if (virCgroupHasController(priv->cgroup,
> >> @@ -6099,14 +6078,6 @@ qemuDomainPinIOThread(virDomainPtr dom,
> >> }
> >> }
> >>
> >> - if (vm->def->cputune.iothreadspin)
> >> - virDomainPinDefArrayFree(vm->def->cputune.iothreadspin,
> >> - vm->def->cputune.niothreadspin);
> >> -
> >> - vm->def->cputune.iothreadspin = newIOThreadsPin;
> >> - vm->def->cputune.niothreadspin = newIOThreadsPinNum;
> >> - newIOThreadsPin = NULL;
> >> -
> >> if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
> >> goto endjob;
> >>
> >> @@ -6124,31 +6095,25 @@ qemuDomainPinIOThread(virDomainPtr dom,
> >> }
> >>
> >> if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> >> + virDomainIOThreadIDDefPtr iothrid;
> >> + virBitmapPtr cpumask;
> >> +
> >> /* Coverity didn't realize that targetDef must be set if we got here. */
> >> sa_assert(persistentDef);
> >>
> >> - if (iothread_id > persistentDef->iothreads) {
> >> + if (!(iothrid = virDomainIOThreadIDFind(persistentDef, iothread_id))) {
> >> virReportError(VIR_ERR_INVALID_ARG,
> >> - _("iothread value out of range %d > %d"),
> >> - iothread_id, persistentDef->iothreads);
> >> + _("iothreadid %d not found"), iothread_id);
> >> goto endjob;
> >> }
> >>
> >> - if (!persistentDef->cputune.iothreadspin) {
> >> - if (VIR_ALLOC(persistentDef->cputune.iothreadspin) < 0)
> >> - goto endjob;
> >> - persistentDef->cputune.niothreadspin = 0;
> >> - }
> >> - if (virDomainPinAdd(&persistentDef->cputune.iothreadspin,
> >> - &persistentDef->cputune.niothreadspin,
> >> - cpumap,
> >> - maplen,
> >> - iothread_id) < 0) {
> >> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >> - _("failed to update or add iothreadspin xml "
> >> - "of a persistent domain"));
> >> + if (!(cpumask = virBitmapNewData(cpumap, maplen)))
> >> goto endjob;
> >> - }
> >> +
> >> + if (!iothrid->cpumask)
> >> + persistentDef->cputune.niothreadspin++;
> >
> > Again, this should not be necessary.
> >
>
> yep
>
> >> + virBitmapFree(iothrid->cpumask);
> >> + iothrid->cpumask = cpumask;
> >>
> >> ret = virDomainSaveConfig(cfg->configDir, persistentDef);
> >> goto endjob;
> >
> > The code looks much better now, but this patch has still some nits.
> >
> > Peter
> >
>
>
> All the following changes to be squashed in:
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a191b02..9876557 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2115,6 +2115,19 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int
> npin)
Adding patches in plaintex in thunderbird or other client that breaks
lines makes them unusable so I don't usually bother checking it.
> }
Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150423/75696268/attachment-0001.sig>
More information about the libvir-list
mailing list