Re: [libvirt] [PATCH 1/3] conf: Do not allow vcpupin's cpuid exceed current vcpus number
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