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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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/
> backend advertises multi-touch support. In some cases this

Can you mention under which backend node those devices appear?

> 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?

> 
> 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. "?

> + *
> + * 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
>  #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
> 

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