Re: [PATCH] Update virtio input device specification

[Ladi Prosek <lprosek@redhat.com>]

On Thu, Apr 6, 2017 at 2:25 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Apr 06, 2017 at 02:03:32PM +0200, Ladi Prosek wrote:
>> >> >>  \item[VIRTIO_INPUT_CFG_PROP_BITS]
>> >> >> -\field{subsel} is not used and MUST be zero.
>> >> >> +\field{subsel} is zero.
>> >> >>  Returns input properties (INPUT_PROP_*) of the device, in \field{u.bitmap}.
>> >> >>
>> >> >> +Same as EVIOCGPROP ioctl for Linux evdev devices.
>> >> >> +
>> >> >
>> >> > Above line confuses more than it clarifies IMHO.
>> >> >
>> >> > Referred to headers contain no information about this ioctl.
>> >>
>> >> I see EVIOCGPROP in input.h. With no further details, but that's true
>> >> for most of the stuff in the headers.
>> >>
>> >> Ideally evdev would have a good documentation that we can point to.
>> >> The headers are not sufficient, readers will have to dig into Linux
>> >> source code to understand the semantics. That's unfortunate but the
>> >> alternative would be to provide an exhaustive documentation of the
>> >> Linux input layer in this spec and keep it up to date with the
>> >> implementation. And that's not feasible.
>> >
>> > I'm not really sure what to do here. Are we really exposing
>> > such a large interface that we can't document it?
>> > This is what the spec is about, if people have to dig into
>> > code they can and will interpret it incorrectly, they
>> > will make assumptions based on a specific implementation and
>> > then their implementation will not work when you make
>> > changes to the host.
>>
>> As I wrote above, if we cannot go pragmatic and abstract evdev away as
>> much as we can, then maybe we shouldn't be including this device in
>> the spec.
>
> I'm not sure what the rest of the TC will say, but we can try.
>
> It does IMHO no harm to add extra information.  We can include a text along
> the lines of "Device behaviour mirrors the behaviour of the evdev layer
> of linux, making a pass-through implementation on top of evdev easy.
> Related evdev ioctl names are provided for reference".
>
>
> But the pragmatic approach would be to add just enough info to address
> questions that people reading the spec actually come up with.
> You can't wave *all* questions away just by saying "look it up in the
> source".

Ok, got it. I'll beef up the text a bit and send a patch against master. Thanks!

>> For what it's worth, I didn't have any major issues implementing the
>> Windows driver based on the previous version of Gerd's spec.
>
> Then I guess it shouldn't be hard to answer the original question?
>
>
>
> --
> MST