Re: [PATCH v3 12/12] xen/domain: Allocate d->vcpu[] in domain_create()

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>, Wei Liu <wei.liu2@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Tim Deegan <tim@xen.org>, Xen-devel <xen-devel@lists.xen.org>,
	Julien Grall <julien.grall@arm.com>
Subject: Re: [PATCH v3 12/12] xen/domain: Allocate d->vcpu[] in domain_create()
Date: Fri, 31 Aug 2018 11:58:31 +0100	[thread overview]
Message-ID: <3d06cdb9-2966-8e23-1a98-b9d346d1fa1a@citrix.com> (raw)
In-Reply-To: <5B891B8702000078001E3D6D@prv1-mh.provo.novell.com>

On 31/08/18 11:42, Jan Beulich wrote:
>>>> On 31.08.18 at 12:33, <wei.liu2@citrix.com> wrote:
>> On Wed, Aug 29, 2018 at 09:03:36AM -0600, Jan Beulich wrote:
>>>>>> On 29.08.18 at 16:40, <andrew.cooper3@citrix.com> wrote:
>>>> For ARM, the call to arch_domain_create() needs to have completed before
>>>> domain_max_vcpus() will return the correct upper bound.
>>>>
>>>> For each arch's dom0's, drop the temporary max_vcpus parameter, and allocation
>>>> of dom0->vcpu.
>>>>
>>>> With d->max_vcpus now correctly configured before evtchn_init(), the poll mask
>>>> can be constructed suitably for the domain, rather than for the worst-case
>>>> setting.
>>>>
>>>> Due to the evtchn_init() fixes, it no longer calls domain_max_vcpus(), and
>>>> ARM's two implementations of vgic_max_vcpus() no longer need work around the
>>>> out-of-order call.
>>>>
>>>> From this point on, d->max_vcpus and d->vcpus[] are valid for any domain which
>>>> can be looked up by domid.
>>>>
>>>> The XEN_DOMCTL_max_vcpus hypercall is modified to reject any call attempt with
>>>> max != d->max_vcpus, which does match the older semantics (not that it is
>>>> obvious from the code).  The logic to allocate d->vcpu[] is dropped, but at
>>>> this point the hypercall still needs making to allocate each vcpu.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>> in principle, but as said before the lack of renaming of the domctl
>>> makes my ack dependent upon some other REST maintainer
>>> agreeing with your position there (the more that you've added
>>> the comment to the implementation rather than the public header).
>> I don't see much value in renaming something that is due to be removed
>> soon.
> I would agree if "soon" meant "soon" for sure. But we all know how things
> get delayed. What I'd like to avoid is shipping 4.12 with a mis-named
> domctl.

I do intend to get this fixed within the 4.12 timeframe.

However, irrespective of the timeframe, this isn't a hypercall which
anyone is realistically going to look at.  If you want to get pedantic
about naming, it should have been named set_max_vcpus from the outset.

This is one example where the effort required to adjust the
inconsistency completely dwarfs the outcome.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel