[libvirt] [PATCH v3 5/6] qemu: Add support to Add/Delete IOThreads

[Peter Krempa]

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 at 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
-------------- 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/20150421/2c843856/attachment-0001.sig>