From patchwork Tue Dec 13 13:10:04 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pierre Ossman X-Patchwork-Id: 87877 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp2194059qgi; Tue, 13 Dec 2016 05:11:59 -0800 (PST) X-Received: by 10.200.34.5 with SMTP id o5mr80721854qto.32.1481634719916; Tue, 13 Dec 2016 05:11:59 -0800 (PST) Return-Path: Received: from lists.gnu.org (lists.gnu.org. [208.118.235.17]) by mx.google.com with ESMTPS id e17si28106431qtf.243.2016.12.13.05.11.59 for (version=TLS1 cipher=AES128-SHA bits=128/128); Tue, 13 Dec 2016 05:11:59 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; Authentication-Results: mx.google.com; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+patch=linaro.org@nongnu.org Received: from localhost ([::1]:38328 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cGms9-0003aZ-A6 for patch@linaro.org; Tue, 13 Dec 2016 08:11:57 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36132) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cGmqQ-0002vH-0I for qemu-devel@nongnu.org; Tue, 13 Dec 2016 08:10:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cGmqO-0006TG-KD for qemu-devel@nongnu.org; Tue, 13 Dec 2016 08:10:10 -0500 Received: from lists.cendio.se ([2a00:801:107:f000::b]:45340 helo=mail.cendio.se) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cGmqO-0006PX-EP for qemu-devel@nongnu.org; Tue, 13 Dec 2016 08:10:08 -0500 Received: from mjolnir.ossman.eu (unknown [IPv6:2001:470:df41:0:38ac:b4cf:438f:764c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.cendio.se (Postfix) with ESMTPSA id 10081C0A4AEC; Tue, 13 Dec 2016 14:10:05 +0100 (CET) To: Gerd Hoffmann References: <1481616362.27088.74.camel@redhat.com> From: Pierre Ossman Organization: Cendio AB Message-ID: <7ba29954-e3ad-529c-9978-b10d2788f9e4@cendio.se> Date: Tue, 13 Dec 2016 14:10:04 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1481616362.27088.74.camel@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:801:107:f000::b Subject: Re: [Qemu-devel] VNC LED state buggy, interop issue X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" On 13/12/16 09:06, Gerd Hoffmann wrote: >> b) Remove the assumption from the code and the protocol. > > Patches are welcome. > I was just aiming for a consensus on the intended behaviour, rather than fix the bug right now. :) But all right, attached patch is an attempt. Btw, is there any client that implements this extension yet? I couldn't find anything. Regards -- Pierre Ossman Software Development Cendio AB http://cendio.com Teknikringen 8 http://twitter.com/ThinLinc 583 30 Linköping http://facebook.com/ThinLinc Phone: +46-13-214600 http://plus.google.com/+CendioThinLinc A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? >From c27ac2026c84d7fd0da2c27a7ebe08f4359cc977 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Tue, 13 Dec 2016 14:03:18 +0100 Subject: [PATCH] vnc: track LED state separately Piggy-backing on the modifier state array made it difficult to send out updates at the proper times. Signed-off-by: Pierre Ossman --- ui/vnc.c | 56 +++++++++++++------------------------------------------- ui/vnc.h | 3 ++- 2 files changed, 15 insertions(+), 44 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index 2c28a59..c449a7d 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -1231,8 +1231,6 @@ void vnc_disconnect_finish(VncState *vs) vnc_update_server_surface(vs->vd); } - if (vs->vd->lock_key_sync) - qemu_remove_led_event_handler(vs->led); vnc_unlock_output(vs); qemu_mutex_destroy(&vs->output_mutex); @@ -1665,69 +1663,38 @@ static void press_key(VncState *vs, int keysym) qemu_input_event_send_key_delay(vs->vd->key_delay_ms); } -static int current_led_state(VncState *vs) -{ - int ledstate = 0; - - if (vs->modifiers_state[0x46]) { - ledstate |= QEMU_SCROLL_LOCK_LED; - } - if (vs->modifiers_state[0x45]) { - ledstate |= QEMU_NUM_LOCK_LED; - } - if (vs->modifiers_state[0x3a]) { - ledstate |= QEMU_CAPS_LOCK_LED; - } - - return ledstate; -} - static void vnc_led_state_change(VncState *vs) { - int ledstate = 0; - if (!vnc_has_feature(vs, VNC_FEATURE_LED_STATE)) { return; } - ledstate = current_led_state(vs); vnc_lock_output(vs); vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE); vnc_write_u8(vs, 0); vnc_write_u16(vs, 1); vnc_framebuffer_update(vs, 0, 0, 1, 1, VNC_ENCODING_LED_STATE); - vnc_write_u8(vs, ledstate); + vnc_write_u8(vs, vs->vd->ledstate); vnc_unlock_output(vs); vnc_flush(vs); } static void kbd_leds(void *opaque, int ledstate) { - VncState *vs = opaque; - int caps, num, scr; - bool has_changed = (ledstate != current_led_state(vs)); + VncDisplay *vd = opaque; + VncState *client; trace_vnc_key_guest_leds((ledstate & QEMU_CAPS_LOCK_LED), (ledstate & QEMU_NUM_LOCK_LED), (ledstate & QEMU_SCROLL_LOCK_LED)); - caps = ledstate & QEMU_CAPS_LOCK_LED ? 1 : 0; - num = ledstate & QEMU_NUM_LOCK_LED ? 1 : 0; - scr = ledstate & QEMU_SCROLL_LOCK_LED ? 1 : 0; + if (ledstate != vd->ledstate) + return; - if (vs->modifiers_state[0x3a] != caps) { - vs->modifiers_state[0x3a] = caps; - } - if (vs->modifiers_state[0x45] != num) { - vs->modifiers_state[0x45] = num; - } - if (vs->modifiers_state[0x46] != scr) { - vs->modifiers_state[0x46] = scr; - } + vd->ledstate = ledstate; - /* Sending the current led state message to the client */ - if (has_changed) { - vnc_led_state_change(vs); + QTAILQ_FOREACH(client, &vd->clients, next) { + vnc_led_state_change(client); } } @@ -3083,8 +3050,6 @@ void vnc_start_protocol(VncState *vs) vnc_write(vs, "RFB 003.008\n", 12); vnc_flush(vs); vnc_read_when(vs, protocol_version, 12); - if (vs->vd->lock_key_sync) - vs->led = qemu_add_led_event_handler(kbd_leds, vs); vs->mouse_mode_notifier.notify = check_pointer_type_change; qemu_add_mouse_mode_change_notifier(&vs->mouse_mode_notifier); @@ -3191,6 +3156,8 @@ static void vnc_display_close(VncDisplay *vd) } g_free(vd->tlsaclname); vd->tlsaclname = NULL; + if (vd->lock_key_sync) + qemu_remove_led_event_handler(vd->led); } int vnc_display_password(const char *id, const char *password) @@ -3758,6 +3725,9 @@ void vnc_display_open(const char *id, Error **errp) } #endif vd->lock_key_sync = lock_key_sync; + if (lock_key_sync) + vd->led = qemu_add_led_event_handler(kbd_leds, vd); + vd->ledstate = 0; vd->key_delay_ms = key_delay_ms; device_id = qemu_opt_get(opts, "display"); diff --git a/ui/vnc.h b/ui/vnc.h index d20b154..d8c9de5 100644 --- a/ui/vnc.h +++ b/ui/vnc.h @@ -154,6 +154,8 @@ struct VncDisplay DisplayChangeListener dcl; kbd_layout_t *kbd_layout; int lock_key_sync; + QEMUPutLEDEntry *led; + int ledstate; int key_delay_ms; QemuMutex mutex; @@ -304,7 +306,6 @@ struct VncState size_t read_handler_expect; /* input */ uint8_t modifiers_state[256]; - QEMUPutLEDEntry *led; bool abort; QemuMutex output_mutex; -- 2.7.4