Re: [Xen-devel] [PATCH RFC 0/8] libxl, xl, public/io: PV backends feature control

[Joao Martins <joao.m.martins@xxxxxxxxxx>]

On 02/07/2018 11:16 AM, Roger Pau Monné wrote:
> On Thu, Nov 02, 2017 at 06:06:08PM +0000, Joao Martins wrote:
>> Hey folks,
>>
>> Presented herewith is an attempt to implement PV backends feature control
>> as discussed in the list 
>> (https://lists.xen.org/archives/html/xen-devel/2017-09/msg00766.html)
>>
>> Given that this a simple proposal hence I thought to include all changes
>> involved in the same patchset such that everyone see all the changes and has 
>> a
>> better estimate (but restricted to xen-devel just for the RFC purposes).
>>
>> The motivation here is to allow system administrators more fine grained
>> control of the device features being used by guest.
>>
>> The only change I made compared to the proposed discussed above was to use
>> "require" instead of "request" as the prefix because there is a feature which
> 
> require in the above context, like:
> 
> require-multi-queue-max-queues=2
> 
> Seems to imply that the guest _must_ have support for multiqueue and
> use exactly two queues.
> 
> What about using 'config' prefix?
> 
> config-multi-queue-max-queues=2
> config-feature-persistent=0
>

Hmm, 'config' sounds better indeed. We mainly chose 'require' because the domain
shall only be booted with those configs. I am fine with both.

>> has "request" in it. But if "request" is still preferred as a prefix I can 
>> change
>> it up.
>>
>> The scheme proposed is quite simple:
>>
>> * The directory "require" is created (inside the backend path) and within 
>> that
>> directory the features/capabilities names and values are written.
> 
> I'm quite sure I'm missing something, but what's the point in having
> this require directory in the xenstore backend path?
> 
> AFAICT you should just write this directly to the backend directory,
> and backends should be modified to check if there's a value already
> present before writing the default one.
> It's also an option which I can go with if folks prefer it.

Creating a config/require directory for requested features simply sounded
cleaner to me, and would also allow you to know what config you passed on to the
backend vs the one the backend exposed (better separation). And writing over the
currently reserved entries would be a little confusing when dealing with new
features (e.g. you write multi-queue-max-queues on a non multi queue backend and
the entry being present is a little misleading even though you would restrict
its access with backend-only permissions).

>> * Toolstack constructs a key value store of features, and user specifies 
>> those
>> through special entry names prefixed also as "require". Toolstack is 
>> stateless thus sys
>> admin has full control over what to pass to the backend. In other words it
>> doesn't look at particular feature names/values.
>>
>> * The backend will then use that for seeding its maximum feature set to the
>> frontend.
> 
> Oh, I see. So the backend picks up the suggested config from this
> directory together with it's capabilities and then produces the final
> set of features exposed to the frontend.
> 
/nods

> In order to prevent adding more logic to the backends, would it make
> sense to export the backend capabilities in /sys/ (or sysctl on BSDs)
> and do those calculations from the toolstack itself, so that the
> toolstack directly writes the features to the backend top level
> xenstore directory?

I had suggested in answer to Paul's comment[0] to create this maximum featureset
of the backend in its top level directory. Pasting it here that part (with one
addition):

/local/domain/X/backend/<backend_type>/features/<feature-name>-desc = "Short
description
of <feature-name>"
/local/domain/X/backend/<backend_type>/features/<feature-name>-defval =
"<something>"
/local/domain/X/backend/<backend_type>/features/<feature-name>-type =
"uint|int|string|bool" (but could be done with regexp instead of this entry)

e.g.

/local/domain/X/backend/vif/features/multi-queue-max-queues-desc = "Number of
queues of the interface"
/local/domain/X/backend/vif/features/multi-queue-max-queues-defval = "8"
/local/domain/X/backend/vif/features/multi-queue-max-queues-type = "uint"

Just wondering about the handling of these that would complicate the backend
implementation (e.g. types, error checking). But I am not seeing another good
way of doing this.

Joao

[0] https://lists.xen.org/archives/html/xen-devel/2017-11/msg00347.html

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