From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=38084 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PLi7q-00013I-QF for qemu-devel@nongnu.org; Thu, 25 Nov 2010 15:09:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PLi7c-0000cV-UI for qemu-devel@nongnu.org; Thu, 25 Nov 2010 15:09:02 -0500 Received: from moutng.kundenserver.de ([212.227.17.8]:63318) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PLi7c-0000c2-J5 for qemu-devel@nongnu.org; Thu, 25 Nov 2010 15:08:48 -0500 Message-ID: <4CEEC255.2020309@mail.berlios.de> Date: Thu, 25 Nov 2010 21:08:53 +0100 From: Stefan Weil MIME-Version: 1.0 Subject: Re: [Qemu-devel] [Bug 427612] Re: kvm sends caps lock key up event twice References: <20090910235343.14184.29717.malonedeb@gangotri.canonical.com> <20101124214406.30688.1630.malone@wampee.canonical.com> In-Reply-To: <20101124214406.30688.1630.malone@wampee.canonical.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bug 427612 <427612@bugs.launchpad.net> Cc: Benjamin Drung , qemu-devel@nongnu.org Am 24.11.2010 22:44, schrieb Benjamin Drung: > Attached a new version of my patch. You find two branches linked to this > bug for maverick and natty. The patch sets SDL_DISABLE_LOCK_KEYS and get > rid of the complete workaround in qemu-kvm. This requires SDL >= 1.2.14. For newer versions of SDL (those which use SDL_DISABLE_LOCK_KEYS to disable special handling of the lock keys), your patch looks ok. It is a working solution for maintainers of the latest Ubuntu releases. But what about older versions? For those, your patch will make QEMU's keyboard emulation unusable. Most older versions don't know SDL_DISABLE_LOCK_KEYS. As far as I know, some SDL versions (from Debian and Ubuntu) even used SDL_DISABLE_LOCK_KEYS with an inverted meaning. I don't think we can simply ignore old or incompatible versions of SDL. Therefore I suggest adding a runtime version check, so the emulation can either use the old code (SDL < 1.2.14) or your new code (SDL >= 1.2.14). The crucial point is whether "old" versions must be supported or not. This is something which the QEMU community should decide. > Stefan Weil wrote: >> The patch might fix part of the problem, but there remain more issues: >> >> * SDL also sends an SDL_KEYUP event for caps lock when the >> environment variable SDL_DISABLE_LOCK_KEYS is set. >> This mode is very useful but currently unsupported by qemu/kvm. > > Addressed by new patch. > >> * Num lock and caps lock are handled in a similar way by SDL. >> The patch only handles caps lock. Maybe this is less important >> because keyboard layouts which remap num lock are rare >> (I don't know any). > > Addressed by new patch. > >> * The keyboard status LEDs and the qemu client's keyboard status >> can become unsynchronized if the input focus changes from qemu >> to other applications. > > Is this a regression of my patch or is it the case for the unpatched > qemu too? That is no regression, but a weakness which existed from the beginning. Your patch neither makes it better nor makes it worse. > > ** Patch added: "caps-lock-key-up-event-v2.patch" > https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/427612/+attachment/1743919/+files/caps-lock-key-up-event-v2.patch Just a small remark: Inline patches (instead of links) are preferred on qemu-devel because they make reading easier, and it's also easier for reviewers to add comments. Regards, Stefan