Re: [PATCH v2] xen/kbdif: Add features to control keyboard and pointer

[Oleksandr Andrushchenko <andr2000@gmail.com>]

On 04/27/2018 06:11 PM, Konrad Rzeszutek Wilk wrote:
> On Fri, Apr 27, 2018 at 09:58:11AM +0300, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
>> It is now not fully possible to control if and which virtual devices
>> are created by the frontend, e.g. keyboard and pointer devices
>> are always created and multi-touch device is created if the
> s/is/are/
why? "and multi-touch *device is* created"

>> backend advertises multi-touch support. In some cases this
> Can you mention under which backend node those devices appear?
These are created under frontend nodes as these are
to configure individual frontends
>> behavior is not desirable and better control over the frontend's
>> configuration is required.
>> Add new XenStore feature fields, so it is possible to individually
>> control set of exposed virtual devices for each guest OS:
>>   - set feature-keyboard to 0 if no keyboard device needs to be created
>>   - set feature-pointer to 0 if no pointer device needs to be created
> I am thinking that this should be just called 'feature-disable-keyboard'
> or such. And it being there in the first place would signify '1' by default?
I just tried to be aligned with multi-touch which is
"feature-multi-touch". But if you are ok with
"feature-disable-keyboard/pointer" then I can re-work it
this way.
>> Keep old behavior by default.
>>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> ---
>>   xen/include/public/io/kbdif.h | 19 ++++++++++++++++++-
>>   1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/include/public/io/kbdif.h b/xen/include/public/io/kbdif.h
>> index 3ce54e9a44c1..ac92e466fd9c 100644
>> --- a/xen/include/public/io/kbdif.h
>> +++ b/xen/include/public/io/kbdif.h
>> @@ -49,7 +49,22 @@
>>    *
>>    * Capable backend advertises supported features by publishing
>>    * corresponding entries in XenStore and puts 1 as the value of the entry.
>> - * If a feature is not supported then 0 must be set or feature entry omitted.
>> + * If not otherwise noted if a feature is not supported then 0 must be set
>> + * or feature entry omitted.
> Huh? I am not sure what you are saying there.
>> + *
>> + * feature-keyboard
>> + *      Values:         <uint>
>> + *
>> + *      If no virtual keyboard device to be exposed by the frontend then
>> + *      this must be set to 0. If feature entry omitted or not set its
>> + *      value defaults to 1.
> Are you saying:
> "If there is no need to expose a virtual keyboard device then this must be
> set to 0. By default it is 1 and it is assumed that any frontend that does
> not probe this flag will assume the value of 1. "?
yeap ;)
>> + *
>> + * feature-pointer
>> + *      Values:         <uint>
>> + *
>> + *      If no virtual pointer device to be exposed by the frontend then
>> + *      this must be set to 0. If feature entry omitted or not set its
>> + *      value defaults to 1.
> Ditto?
>>    *
>>    * feature-abs-pointer
>>    *      Values:         <uint>
>> @@ -177,6 +192,8 @@
>>   
>>   #define XENKBD_DRIVER_NAME             "vkbd"
>>   
>> +#define XENKBD_FIELD_FEAT_KEYBOARD     "feature-keyboard"
>> +#define XENKBD_FIELD_FEAT_POINTER      "feature-pointer"
> How about just call it '
>
> feature-disable-keyboard
> feature-disable-keyboard
See above, I'm fine with that. If we agree on "disable"
semantics I will re-work the patch.
>>   #define XENKBD_FIELD_FEAT_ABS_POINTER  "feature-abs-pointer"
>>   #define XENKBD_FIELD_FEAT_MTOUCH       "feature-multi-touch"
>>   #define XENKBD_FIELD_REQ_ABS_POINTER   "request-abs-pointer"
>> -- 
>> 2.17.0
>>
Thank you,
Oleksandr

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