[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v5 09/10] qemu: Add support to Add/Delete IOThreads




On 04/27/2015 10:45 AM, Peter Krempa wrote:
> On Fri, Apr 24, 2015 at 12:06:01 -0400, John Ferlan wrote:
>> Add qemuDomainAddIOThread and qemuDomainDelIOThread in order to add or
>> remove an IOThread to/from the host either for live or config optoins
>>
>> The implementation for the 'live' option will use the iothreadpids list
>> in order to make decision, while the 'config' option will use the
>> iothreadids list.  Additionally, for deletion each may have to adjust
>> the iothreadpin list.
>>
>> IOThreads are implemented by qmp objects, the code makes use of the existing
>> qemuMonitorAddObject or qemuMonitorDelObject APIs.
>>
>> Signed-off-by: John Ferlan <jferlan redhat com>
>> ---
>>  src/conf/domain_audit.c  |   9 ++
>>  src/conf/domain_audit.h  |   6 +
>>  src/libvirt_private.syms |   1 +
>>  src/qemu/qemu_driver.c   | 380 +++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 396 insertions(+)
>>
> 
> ...
> 
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 1fac0b8..4ae63c6 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -6154,6 +6154,384 @@ 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;
>> +    virCgroupPtr cgroup_iothread = NULL;
>> +    char *mem_mask = NULL;
>> +    virDomainIOThreadIDDefPtr iothrid;
>> +    virBitmapPtr cpumask;
>> +
>> +    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 (STREQ(new_iothreads[idx]->name, alias))
>> +            break;
>> +    }
>> +
>> +    if (idx == new_niothreads) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       _("cannot find new IOThread '%u' in QEMU monitor."),
>> +                       iothread_id);
>> +        goto cleanup;
>> +    }
>> +
>> +    if (!(iothrid = virDomainIOThreadIDAdd(vm->def, 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;
>> +    }
>> +
>> +    if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
>> +        cpumask = priv->autoCpuset;
>> +    else
>> +        cpumask = vm->def->cpumask;
>> +
>> +    if (cpumask) {
>> +        if (qemuDomainHotplugPinThread(cpumask, iothread_id,
>> +                                       iothrid->thread_id, cgroup_iothread) < 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);
>> +    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;
>> +
>> +    /* 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;
>> +
>> +    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;
>> +
>> +    virDomainIOThreadIDDel(vm->def, iothread_id);
>> +
>> +    virDomainIOThreadSchedDelId(vm->def, iothread_id);
>> +
>> +    if (qemuDomainDelCgroupForThread(priv->cgroup,
>> +                                     VIR_CGROUP_THREAD_IOTHREAD,
>> +                                     iothread_id) < 0)
>> +        goto cleanup;
>> +
>> +    ret = 0;
>> +
>> + cleanup:
>> +    if (new_iothreads) {
>> +        for (idx = 0; idx < new_niothreads; idx++)
>> +            qemuMonitorIOThreadInfoFree(new_iothreads[idx]);
>> +        VIR_FREE(new_iothreads);
>> +    }
>> +    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;
> 
> It's nice that you've finally deleted the piece of code that has to deal
> with the DMA memory allocation issue with vCPU hotplug. You also could
> have deleted the corresponding variables that are now unused ... 
> 
>> +    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 (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))
>> +                goto endjob;
>> +
>> +            persistentDef->iothreads++;
>> +        } else {
>> +            virDomainIOThreadIDDefPtr iothrid;
>> +            if (!(iothrid = virDomainIOThreadIDFind(persistentDef,
>> +                                                    iothread_id))) {
>> +                virReportError(VIR_ERR_INVALID_ARG,
>> +                               _("cannot find IOThread '%u' in persistent "
>> +                                 "iothreadids"),
>> +                               iothread_id);
>> +                goto cleanup;
>> +            }
>> +
>> +            virDomainIOThreadIDDel(persistentDef, iothread_id);
>> +            virDomainIOThreadSchedDelId(persistentDef, 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;
>> +}
> 
> ACK if you clean up qemuDomainChgIOThread()
> 

done... with the following squashed in.

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e1a6265..443341b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6452,10 +6452,6 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver,
     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;
 
@@ -6527,19 +6523,9 @@ qemuDomainChgIOThread(virQEMUDriverPtr driver,
     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;



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]