Re: [Xen-devel] [PATCH 00/12] add per-domain and per-cpupool generic parameters

[Juergen Gross <jgross@xxxxxxxx>]

On 18/09/18 15:57, George Dunlap wrote:
> On 09/18/2018 02:36 PM, Juergen Gross wrote:
>> On 18/09/18 15:25, George Dunlap wrote:
>>> On 09/18/2018 12:32 PM, Juergen Gross wrote:
>>>> On 18/09/18 13:20, Jan Beulich wrote:
>>>>>>>> On 18.09.18 at 13:10, <jgross@xxxxxxxx> wrote:
>>>>>> On 18/09/18 12:32, Jan Beulich wrote:
>>>>>>>>>> On 18.09.18 at 08:02, <jgross@xxxxxxxx> wrote:
>>>>>>>> Instead of using binary hypervisor interfaces for new parameters of
>>>>>>>> domains or cpupools this patch series adds support for generic text
>>>>>>>> based parameter parsing.
>>>>>>>>
>>>>>>>> Parameters are defined via new macros similar to those of boot
>>>>>>>> parameters. Parsing of parameter strings is done via the already
>>>>>>>> existing boot parameter parsing function which is extended a little
>>>>>>>> bit.
>>>>>>>>
>>>>>>>> Parameter settings can either be specified in configuration files of
>>>>>>>> domains or cpupools, or they can be set via new xl sub-commands.
>>>>>>>
>>>>>>> Without having looked at any of the patches yet (not even their
>>>>>>> descriptions) I'm still wondering what the benefit of textual parameters
>>>>>>> really is: Just like "binary" ones, they become part of the public
>>>>>>> interface, and hence subsequently can't be changed any more or
>>>>>>> less than the ones we currently have (in particular, anything valid in
>>>>>>> a guest config file will imo need to remain to be valid and meaningful
>>>>>>> down the road).
>>>>>>
>>>>>> So lets look what would be needed for adding something like the
>>>>>> per-domain xpti parameter using binary interfaces:
>>>>>>
>>>>>> 1 an extension of some domctl interface, maybe bumping of the domctl
>>>>>>   interface version
>>>>>> 2 adding the logic to domctl handling
>>>>>> 3 adding libxc support
>>>>>> 4 adding libxl support
>>>>>> 5 adding a new xl sub-command
>>>>>> 6 adding domain config support
>>>>>> 7 adding documentation
>>>>>>
>>>>>> With my approach only 2 (in a modified form, parameter handling instead
>>>>>> of domctl, but comparable in the needed effort) and 7 are needed.
>>>>>>
>>>>>> So once the framework is in place it is _much_ easier to add new
>>>>>> features.
>>>>>
>>>>> All the above would hold if the individual options were expressed as
>>>>> e.g. flags in an extensible bit vector.
>>>>
>>>> Who would translate the new option into a bit vector? This would be the
>>>> tools (libxc/libxl/xl), so those need to be modified for each new bit.
>>>
>>> A bit vector would only allow on/off; it wouldn't allow you to set
>>> numeric parameters, for example.
>>>
>>> I like the idea of being able to add configuration parameters without
>>> having a huge amount of boilerplate; and also of being able to backport
>>> parameters like xpti without having to worry so much about compatibility.
>>>
>>> But I'm not a fan of the idea of using a "string blob" to accomplish
>>> that.  It's convenient for the exact use case you seem to be
>>> contemplating: having a user add the string into the xl config file, and
>>> having nobody but the hypervisor interpret it.  But what about tools
>>> that may want to set that parameter?  Or tools that want to query the
>>> parameter, or "introspect" on the domain settings or whatever?  Now they
>>> have to have a bunch of code to generate and parse the string code.
>>>
>>> Could we have a reasonably generic structure / union, with a parameter
>>> number, that we could pass in instead?  Something like:
>>>
>>> struct domain_parameter {
>>>   int param_num;
>>>   int val;
>>> }
>>>
>>> And then have a list somewhere of string values -> parameter numbers
>>> that callers could use to translate strings into values?
>>>
>>> That way the above list would look like:
>>>
>>> 1. Add new parameter in Xen
>>> 2. Add a string name -> parameter number in a header somewhere
>>> 3. Add a libxl #define with that parameter number
>>>
>>> You'd have to recompile xl against the new header, but you were probably
>>> going to do that anyway.
>>
>> The string variant is much more flexible.
>>
>> It is easy possible to e.g. add a per-domain trace parameter to specify
>> rather complex trace instrumentations. Doing something like that via a
>> struct based interface is in the best case complicated.
> 
> ...or, for instance, specifying the runqueue layout of a cpupool (for
> schedulers like credit2 which allow such things).  Yes, that is true;
> but probably a very niche case.
> 
>> Another advantage of the string based variant is that you don't need a
>> central header. You can define the parameters where you are implementing
>> them. No need to expand switch statements and headers, just a local
>> definition and maybe a handler function.
> 
> I don't see the lack of central header as a big advantage -- how hard is
> it to add a single line to a list somewhere?

That's not very hard.

You need additional entries for connecting the domctl with the parameter
setting:

/* central header: */
#define PARAM_XPTI 13

/* domctl handling: */
switch (param) {
case PARAM_XPTI: ret = do_param_xpti_setting(value);
    break;

/* pv-dom header: */
int do_param_xpti_setting(int val);

/* pv-dom handler: */
int do_param_xpti_setting(int val)
{
...
}

So you need to touch at least four files in the hypervisor, plus the
parsing added in xl.

The string-only variant needs:

/* pv-dom handler: */
static int do_param_xpti_setting(...)
{
...
}
custom_domain_param("xpti", ...);

And that's all. See the difference?

> 
> *Not* having a language-level construct around (either an enum or a
> #define) means that programs can't take advantage of the preprocessor /
> type system to catch bugs; someone calling
> libxl_domain_param("xptl=off"); won't get a compile-time error, only a
> run-time one; someone calling
> libxl_domain_param(LIBXL_DOMAIN_PARAM_XPTL, 0) will.

Yes. We should be very careful which parameters should be supported
that way. I wouldn't want to do standard domain configuration stuff
(e.g. number of vcpus, memory size, ...) this way, just additional
stuff like xpti, l1tf mitigation, maybe test interfaces like tracing,
setting of special thresholds.

> I'm not completely opposed to the "string blob" idea, but it would be
> nice if at least for the common case of simple boolean / integer values,
> we could avoid having a string blob.
> 
> What about
> 
> struct parameter {
>   int param_number;
>   union {
>     int val;
>     char special[]
>   };
> }

I think this would be a good interface for replacing current domctl
or sysctl interfaces which are used implicitly e.g. during domain
creation. This would bring us a step closer to stable sysctl and domctl
interfaces.


Juergen

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