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

On 08/03/2012 12:36 AM, Hu Tao wrote: > From: Tang Chen <tangchen(a)cn.fujitsu.com&gt; > > 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(a)cn.fujitsu.com&gt; > Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com&gt; > --- > @@ -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?

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

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).