Re: [PATCH v2 3/5] xen/domain: Stricter configuration checking

From: "Jan Beulich" <JBeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Jackson <Ian.Jackson@citrix.com>,
	Julien Grall <julien.grall@arm.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH v2 3/5] xen/domain: Stricter configuration checking
Date: Tue, 13 Nov 2018 09:56:55 -0700	[thread overview]
Message-ID: <5BEB025702000078001FB92F@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <f6a5ba12-ca24-213d-71e6-fa0982c689d9@citrix.com>

>>> On 13.11.18 at 15:39, <andrew.cooper3@citrix.com> wrote:
> On 13/11/2018 14:36, Wei Liu wrote:
>> On Tue, Nov 13, 2018 at 07:14:24AM -0700, Jan Beulich wrote:
>>>>>> On 12.11.18 at 17:16, <andrew.cooper3@citrix.com> wrote:
>>>> Currently, a number of options passed for domain creation are ignored, or have
>>>> implicit fallback behaviour.  This is bad for forwards compatibility, and for
>>>> end users to be certain that they got the configuration they asked for.
>>>>
>>>> With this change:
>>>>  * ARM now strictly requires that XEN_DOMCTL_CDF_hap is passed.  Previously,
>>>>    only XEN_DOMCTL_CDF_hvm_guest was checked.
>>>>  * For x86, requesting HAP without HVM is now prohibited, as the combination
>>>>    makes no sense.
>>>>  * For x86, requesting HAP on a non-HAP capable system will fail, rather than
>>>>    silently fall back to Shadow.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> ---
>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>> CC: Wei Liu <wei.liu2@citrix.com>
>>>> CC: Stefano Stabellini <sstabellini@kernel.org>
>>>> CC: Julien Grall <julien.grall@arm.com>
>>>> CC: Ian Jackson <Ian.Jackson@citrix.com>
>>>> CC: Wei Liu <wei.liu2@citrix.com>
>>>>
>>>> Semi RFC because this may cause a user-visible change in behaviour.  However,
>>>> if the user has gone to the effort of specifying hap=1, silently falling back
>>>> to shadow is unexpected, and IMO, a bug.
>>> My view on this to a fair part depends on whether the tool stack
>>> would guard us from actually getting into such a situation in the
>>> hypervisor. Getting an unspecific -EINVAL back without further
>>> help towards diagnosis by the tool stack would make such a
>>> change undesirable imo.
>> If you want toolstack to tell you what goes wrong, this sanitisation
>> function should be shared with the toolstack, and presumably with some
>> if __XEN_TOOLS__ trickeries to return / print out the culprit.
> 
> Some bits of logic could be shared like that, but some can't.
> 
> As a different idea, could we hand back an up-to-128 byte string in the
> failure case?  There is space for that in the domctl.u because we've got
> no other error information we need to propagate backwards.

I like the idea in general; I'm not entirely sure if this will scale with future
additions, as the caller might legitimately expect an explanatory string to
come back in all cases. (Of course absence of an explanation could be
signaled via an empty string.)

Jan

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