[libvirt] [PATCH v5 4/5] qemu: Add support to pin IOThreads to specific CPU

Pavel Hrdina phrdina at redhat.com
Mon Mar 9 20:42:17 UTC 2015


On Fri, Mar 06, 2015 at 09:05:44AM -0500, John Ferlan wrote:
> Add qemuDomainPinIOThread to handle setting the CPU affinity
> for a specific IOThread
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  include/libvirt/libvirt-domain.h |   9 ++
>  src/qemu/qemu_driver.c           | 221 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 230 insertions(+)
> 

[...]

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index b37474f..aad08b2 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5780,6 +5780,226 @@ qemuDomainGetIOThreadsInfo(virDomainPtr dom,
>      return ret;
>  }
>  
> +static int
> +qemuDomainPinIOThread(virDomainPtr dom,
> +                      unsigned int iothread_id,
> +                      unsigned char *cpumap,
> +                      int maplen,
> +                      unsigned int flags)
> +{
> +    int ret = -1;
> +    virQEMUDriverPtr driver = dom->conn->privateData;
> +    virQEMUDriverConfigPtr cfg = NULL;
> +    virDomainObjPtr vm;
> +    virCapsPtr caps = NULL;
> +    virDomainDefPtr persistentDef = NULL;
> +    virBitmapPtr pcpumap = NULL;
> +    bool doReset = false;
> +    qemuDomainObjPrivatePtr priv;
> +    virDomainVcpuPinDefPtr *newIOThreadsPin = NULL;
> +    size_t newIOThreadsPinNum = 0;
> +    virCgroupPtr cgroup_iothread = NULL;
> +    virObjectEventPtr event = NULL;
> +    char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = "";
> +    char *str = NULL;
> +    virTypedParameterPtr eventParams = NULL;
> +    int eventNparams = 0;
> +    int eventMaxparams = 0;
> +
> +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> +                  VIR_DOMAIN_AFFECT_CONFIG, -1);
> +
> +    cfg = virQEMUDriverGetConfig(driver);
> +
> +    if (!(vm = qemuDomObjFromDomain(dom)))
> +        goto cleanup;
> +    priv = vm->privateData;
> +
> +    if (virDomainPinIOThreadEnsureACL(dom->conn, vm->def, flags) < 0)
> +        goto cleanup;
> +
> +    if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> +        goto cleanup;
> +
> +    if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("Changing affinity for IOThread dynamically is "
> +                         "not allowed when CPU placement is 'auto'"));
> +        goto cleanup;
> +    }
> +
> +    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)
> +        persistentDef = vm->def;

This is wrong and you cannot do that.  Imagine that the live definition is
already different than the persistent definition.  Just remove those two lines.

> +
> +    /* Coverity didn't realize that targetDef must be set if we got here.  */
> +    sa_assert(persistentDef);
> +
> +    if (!(pcpumap = virBitmapNewData(cpumap, maplen)))
> +        goto endjob;
> +
> +    if (virBitmapIsAllClear(pcpumap)) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("Empty iothread cpumap list for pinning"));
> +        goto endjob;
> +    }
> +
> +    /* pinning to all physical cpus means resetting,
> +     * so check if we can reset setting.
> +     */
> +    if (virBitmapIsAllSet(pcpumap))
> +        doReset = true;
> +
> +    if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> +
> +        if (priv->iothreadpids == NULL) {
> +            virReportError(VIR_ERR_OPERATION_INVALID,
> +                           "%s", _("IOThread affinity is not supported"));
> +            goto endjob;
> +        }
> +
> +        if (iothread_id > priv->niothreadpids) {
> +            virReportError(VIR_ERR_INVALID_ARG,
> +                           _("iothread value out of range %d > %d"),
> +                           iothread_id, priv->niothreadpids);
> +            goto endjob;
> +        }
> +
> +        if (vm->def->cputune.iothreadspin) {
> +            /* The VcpuPinDefCopy works for IOThreads too */
> +            newIOThreadsPin =
> +                virDomainVcpuPinDefCopy(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 (virDomainIOThreadsPinAdd(&newIOThreadsPin, &newIOThreadsPinNum,
> +                                     cpumap, maplen, iothread_id) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("failed to update iothreadspin"));
> +            virDomainVcpuPinDefArrayFree(newIOThreadsPin, newIOThreadsPinNum);

This free function needs to be called in cleanup section because we are jumping
to endjob from several places.

> +            goto endjob;
> +        }
> +
> +        /* Configure the corresponding cpuset cgroup before set affinity. */
> +        if (virCgroupHasController(priv->cgroup,
> +                                   VIR_CGROUP_CONTROLLER_CPUSET)) {
> +            if (virCgroupNewIOThread(priv->cgroup, iothread_id,
> +                                     false, &cgroup_iothread) < 0)
> +                goto endjob;
> +            if (qemuSetupCgroupIOThreadsPin(cgroup_iothread,
> +                                            newIOThreadsPin,
> +                                            newIOThreadsPinNum,
> +                                            iothread_id) < 0) {
> +                virReportError(VIR_ERR_OPERATION_INVALID,
> +                               _("failed to set cpuset.cpus in cgroup"
> +                                 " for iothread %d"), iothread_id);
> +                goto endjob;
> +            }
> +        } else {
> +            if (virProcessSetAffinity(priv->iothreadpids[iothread_id - 1],
> +                                      pcpumap) < 0) {
> +                virReportError(VIR_ERR_SYSTEM_ERROR,
> +                               _("failed to set cpu affinity for IOThread %d"),
> +                               iothread_id);
> +                goto endjob;
> +            }
> +        }
> +
> +        if (doReset) {
> +            virDomainIOThreadsPinDel(vm->def, iothread_id);
> +        } else {
> +            if (vm->def->cputune.iothreadspin)
> +                virDomainVcpuPinDefArrayFree(vm->def->cputune.iothreadspin,
> +                                             vm->def->cputune.niothreadspin);
> +
> +            vm->def->cputune.iothreadspin = newIOThreadsPin;
> +            vm->def->cputune.niothreadspin = newIOThreadsPinNum;
> +            newIOThreadsPin = NULL;
> +        }
> +
> +        if (newIOThreadsPin)
> +            virDomainVcpuPinDefArrayFree(newIOThreadsPin, newIOThreadsPinNum);

Those two exact lines should be in cleanup section.

> +
> +        if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
> +            goto endjob;
> +
> +        if (snprintf(paramField, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                     VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN, iothread_id) < 0) {
> +            goto endjob;
> +        }
> +
> +        str = virBitmapFormat(pcpumap);
> +        if (virTypedParamsAddString(&eventParams, &eventNparams,
> +                                    &eventMaxparams, paramField, str) < 0)
> +            goto endjob;
> +
> +        event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
> +    }
> +
> +    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> +        if (iothread_id > persistentDef->iothreads) {
> +            virReportError(VIR_ERR_INVALID_ARG,
> +                           _("iothread value out of range %d > %d"),
> +                           iothread_id, persistentDef->iothreads);
> +            goto endjob;
> +        }
> +
> +        if (doReset) {
> +            virDomainIOThreadsPinDel(persistentDef, iothread_id);
> +        } else {
> +            if (!persistentDef->cputune.iothreadspin) {
> +                if (VIR_ALLOC(persistentDef->cputune.iothreadspin) < 0)
> +                    goto endjob;
> +                persistentDef->cputune.niothreadspin = 0;
> +            }
> +            if (virDomainIOThreadsPinAdd(&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"));
> +                goto endjob;
> +            }
> +        }
> +
> +        ret = virDomainSaveConfig(cfg->configDir, persistentDef);
> +        goto endjob;
> +    }
> +
> +    ret = 0;
> +
> + endjob:
> +    qemuDomainObjEndJob(driver, vm);
> +
> + cleanup:
> +    if (cgroup_iothread)
> +        virCgroupFree(&cgroup_iothread);
> +    if (event)
> +        qemuDomainEventQueue(driver, event);
> +    VIR_FREE(str);
> +    virBitmapFree(pcpumap);
> +    qemuDomObjEndAPI(&vm);
> +    virObjectUnref(caps);
> +    virObjectUnref(cfg);
> +    return ret;
> +}
> +
>  static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr seclabel)
>  {
>      virQEMUDriverPtr driver = dom->conn->privateData;
> @@ -19328,6 +19548,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
>      .domainGetVcpus = qemuDomainGetVcpus, /* 0.4.4 */
>      .domainGetMaxVcpus = qemuDomainGetMaxVcpus, /* 0.4.4 */
>      .domainGetIOThreadsInfo = qemuDomainGetIOThreadsInfo, /* 1.2.14 */
> +    .domainPinIOThread = qemuDomainPinIOThread, /* 1.2.14 */
>      .domainGetSecurityLabel = qemuDomainGetSecurityLabel, /* 0.6.1 */
>      .domainGetSecurityLabelList = qemuDomainGetSecurityLabelList, /* 0.10.0 */
>      .nodeGetSecurityModel = qemuNodeGetSecurityModel, /* 0.6.1 */
> -- 
> 2.1.0
> 

Otherwise looks good.  ACK with the fixes mentioned above.

Pavel




More information about the libvir-list mailing list