On 2012年10月10日 22:59, Martin Kletzander wrote:
On 10/10/2012 01:14 PM, Osier Yang wrote:
Setting pinning policy for vcpu which exceed current vcpus number makes no sense, and it could cause problem for APIs which associate the vcpu thread id with cgroup. --- src/conf/domain_conf.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 33e1e7f..87e0c6b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8746,9 +8746,9 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (n&& VIR_ALLOC_N(def->cputune.vcpupin, n)< 0) goto no_memory;
- if (n> def->maxvcpus) { + if (n> def->vcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("vcpupin nodes must be less than maxvcpus")); + "%s", _("vcpupin nodes must be less than current vcpus")); goto error; }
What about XML that specifies vcpu pinning for all the vcpus that might be there, but now enables only one. And other pinning just won't be used until those vcpus are in plugged. Or am I missing some problems this could cause?
That means we will keep the <vcpupin> for unplugged vcpus too. Assuming we go this way, then: It needs to maitain these unused yet <vcpupin> info in def->cputune, which cause a further work to do: sort the def->cputune.vcpupin by vcpuid, and then use def->vcpus instead of def->cputune.nvcpupin in various places to only work on the current onlined vcpus, and it's very likely missing the change in some corner. On one hand, personally I'd like manage the <vcpupin> dynamically, I.e. add it for plugged vcpu, remove it for unplugged vcpu. Keeping it always there looks strange. On the other hand, regardless of which way is better, better to keep it simple for us now, especially I won't want to see upcoming bugs bite me in the edge. :-) Regards, Osier