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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, Apr 27, 2018 at 06:19:35PM +0300, Oleksandr Andrushchenko wrote:
> 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@xxxxxxxx>
> > > 
> > > 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.

I think disable works nicer - that way you know for sure what the purpose
is and there is no confusion about the other features.

And the absence of it by default implies you are OK having it be used.

> > > Keep old behavior by default.
> > > 
> > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> > > ---
> > >   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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel