Re: [PATCH 1/3] x86/pv: Options to disable and/or compile out 32bit PV support

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 21/04/2020 07:02, Jan Beulich wrote:
> On 20.04.2020 20:05, Andrew Cooper wrote:
>> On 20/04/2020 15:05, Jan Beulich wrote:
>>> I'm in particular
>>> concerned that we may gain a large number of such printk()s over
>>> time, if we added them in such cases.
>> The printk() was a bit of an afterthought, but deliberately avoiding the
>> -EINVAL path was specifically not.
>>
>> In the case that the user tries to use `pv=no-32` without CONFIG_PV32,
>> they should see something other than
>>
>> (XEN) parameter "pv=no-32" unknown!
> Why - to this specific build of Xen the parameter is unknown.

Because it is unnecessarily problematic and borderline obnoxious to
users, as well as occasional Xen developers.

"you've not got the correct CONFIG_$X for that to be meaningful" is
specifically useful to separate from "I've got no idea".

>> I don't think overloading the return value is a clever move, because
>> then every parse function has to take care of ensuring that -EOPNOTSUPP
>> (or ENODEV?) never clobbers -EINVAL.
> I didn't suggest overloading the return value. Instead I
> specifically want this to go the -EINVAL path.
>
>> We could have a generic helper which looks like:
>>
>> static inline void ignored_param(const char *cfg, const char *name,
>> const char *s, const char *ss)
>> {
>>     printk(XENLOG_INFO "%s disabled - ignoring '%s=%*.s' setting\n",
>> cfg, name, s, (int)(ss - s));
>> }
>>
>> which at least would keep all the users consistent.
> Further bloating the binary with (almost) useless string literals.
> I'd specifically like to avoid this.

I don't accept that as a valid argument.

We're talking about literally tens of bytes (which will merge anyway, so
0 in practice), and a resulting UI which helps people get out of
problems rather than penalises them for having gotten into a problem to
begin with.

I will absolutely prioritise a more helpful UI over a handful of bytes.

~Andrew