[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/1] genius: add genius serial mouse emulation
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 1/1] genius: add genius serial mouse emulation |
Date: |
Tue, 14 Jan 2014 17:37:56 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130923 Thunderbird/17.0.9 |
Il 14/01/2014 00:47, Romain Naour ha scritto:
> Hi,
>
> Thanks for review.
>
> Le 13/01/2014 23:45, Eric Blake a écrit :
>> On 01/13/2014 03:33 PM, Romain Naour wrote:
>>> This patch adds the emulation for a serial Genius mouse using
>>> Mouse Systems protocol (5bytes).
>>> This protocol is compatible with most 3-button serial mouse.
>>>
>>> Signed-off-by: Romain Naour <address@hidden>
>>> ---
>>> backends/Makefile.objs | 2 +-
>>> backends/gnmouse.c | 339
>>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>> include/sysemu/char.h | 3 +
>>> qapi-schema.json | 1 +
>>> qemu-char.c | 4 +
>>> qemu-options.hx | 10 ++
>>> 6 files changed, 358 insertions(+), 1 deletion(-)
>>> create mode 100644 backends/gnmouse.c
>>> +++ b/backends/gnmouse.c
>>> @@ -0,0 +1,339 @@
>>> +/*
>>> + * QEMU Genius GM-6 serial mouse emulation
>>> + *
>>> + * Adapted from msmouse
>>> + *
>>> + * Copyright (c) 2012 Romain Naour
>> It is now 2014.
> Fixed
>>
>>
>>> +++ b/qapi-schema.json
>>> @@ -3617,6 +3617,7 @@
>>> 'null' : 'ChardevDummy',
>>> 'mux' : 'ChardevMux',
>>> 'msmouse': 'ChardevDummy',
>>> + 'gnmouse': 'ChardevDummy',
>>> 'braille': 'ChardevDummy',
>>> 'stdio' : 'ChardevStdio',
>>> 'console': 'ChardevDummy',
>> Ideally, you should also document that this branch was not always
>> available in the union (I was expecting to see a doc line with '(since
>> 2.0)' somewhere in the patch); but looking at the existing schema,
>> you're just copying bad practice of adding to an already
>> under-documented union.
>>
>
> You mean something like that ?
> ##
> # @ChardevBackendKind:
> #
> # A union referencing different chardev backend configuration' info.
> #
> # Since: 1.4
> #
> # gnmouse backend since: 2.0
> ##
> { 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
> [snip]
>
The standard is more like
# A union referencing different chardev backend configuration' info.
#
# @gnmouse: Send mouse data using the Genius protocol (since 2.0).
#
# Since: 1.4
##
but that format would require doucmenting all items. :(
You can leave this out of v2.
However, I'd be happy if you changed the documentation like this
@item -chardev msmouse,address@hidden
Forward events from QEMU's emulated mouse to the guest using the
Microsoft protocol. @option{msmouse} does not take any options.
@item -chardev gnmouse ,address@hidden
Forward events from QEMU's emulated mouse to the guest using the Genius
(Mouse Systems) protocol. @option{gnmouse} does not take any options.