[libvirt] [PATCH 04/17] Enable cpuset cgroup and synchronous vcpupin info to cgroup.

[Hu Tao]

On Mon, Aug 06, 2012 at 03:41:29PM -0600, Eric Blake wrote:
> On 08/03/2012 12:36 AM, Hu Tao wrote:
> > From: Tang Chen <tangchen at cn.fujitsu.com>
> > 
> > vcpu threads pin are implemented using sched_setaffinity(), but
> > not controlled by cgroup. This patch does the following things:
> > 
> >     1) enable cpuset cgroup
> >     2) reflect all the vcpu threads pin info to cgroup
> > 
> > Signed-off-by: Tang Chen <tangchen at cn.fujitsu.com>
> > Signed-off-by: Hu Tao <hutao at cn.fujitsu.com>
> > ---
> 
> > @@ -3667,9 +3669,38 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
> >      if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> >  
> >          if (priv->vcpupids != NULL) {
> > +            /* Add config to vm->def first, because qemuSetupCgroupVcpuPin needs it. */
> > +            if (virDomainVcpuPinAdd(vm->def, cpumap, maplen, vcpu) < 0) {
> > +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                               _("failed to update or add vcpupin xml of "
> > +                                 "a running domain"));
> > +                goto cleanup;
> > +            }
> 
> If this succeeds,
> 
> > +
> > +            /* Configure the corresponding cpuset cgroup before set affinity. */
> > +            if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) {
> > +                if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup_dom, 0) == 0 &&
> > +                    virCgroupForVcpu(cgroup_dom, vcpu, &cgroup_vcpu, 0) == 0 &&
> > +                    qemuSetupCgroupVcpuPin(cgroup_vcpu, vm->def, vcpu) < 0) {
> > +                    virReportError(VIR_ERR_OPERATION_INVALID,
> > +                                   _("failed to set cpuset.cpus in cgroup"
> > +                                     " for vcpu %d"), vcpu);
> > +                    goto cleanup;
> 
> but this fails, then where do we roll vm->def back to its pre-attempt state?
> 
> > +                }
> > +            } else {
> > +                /* Here, we should go on because even if cgroup is not active,
> > +                 * we can still use virProcessInfoSetAffinity.
> > +                 */
> > +                VIR_WARN("cpuset cgroup is not active");
> > +            }
> > +
> >              if (virProcessInfoSetAffinity(priv->vcpupids[vcpu],
> > -                                          cpumap, maplen, maxcpu) < 0)
> > +                                          cpumap, maplen, maxcpu) < 0) {
> > +                virReportError(VIR_ERR_SYSTEM_ERROR,
> > +                               _("failed to set cpu affinity for vcpu %d"),
> > +                               vcpu);
> >                  goto cleanup;
> 
> Same situation - if set_affinity failed, where do we roll back to the
> original vm->def state (also, should we roll back to the original cpuset
> state)?
> 
> If we don't roll back, then a failure can leave the domain in an unknown
> state for cpu pinning.  Or are we just declaring that if these functions
> fail, the domain is in an unknown set of cpu pinning?

Oh yes, this is indeed a problem here, I'll fix it.

> 
> Do we need both cpuset and set_affinity?  Or is cpuset a superset of
> affinity (that is, if I alter cpuset, can I completely skip out on
> calling set_affinity and still get the same results)?  If cpuset gives
> stronger pinning than affinity, then maybe we don't need to try both
> methods, which gives us a bit better mechanism for rollbacks; the only

The man page[1] says cpusets are integrated with sched_setaffinity(),
so I think it is better to choose one of them. If cpuset is avaiable, we
use cpuset. Otherwise we use sched_setaffinity().

> remaining thing is making sure you avoid altering vm->def except on
> success (perhaps by using a temporary structure rather than vm->def at
> the time you attempt the pinning).

Yes.


[1] man 7 cpuset

-- 
Thanks,
Hu Tao