From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44622) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XS2bo-0003aH-Lg for qemu-devel@nongnu.org; Thu, 11 Sep 2014 07:32:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XS2bi-0001PS-85 for qemu-devel@nongnu.org; Thu, 11 Sep 2014 07:32:16 -0400 Received: from smtp.ctxuk.citrix.com ([185.25.65.24]:35893 helo=SMTP.EU.CITRIX.COM) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XS2bi-0001Ok-1R for qemu-devel@nongnu.org; Thu, 11 Sep 2014 07:32:10 -0400 From: Owen Smith Date: Thu, 11 Sep 2014 11:32:07 +0000 Message-ID: <6624BC057AF4E240B6D036F5CC505B12041F0030@AMSPEX01CL02.citrite.net> References: <1410356546-22264-1-git-send-email-owen.smith@citrix.com> <1410356546-22264-2-git-send-email-owen.smith@citrix.com> In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 1/3] xenfb: Unregister keyboard event handler correctly List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: "qemu-devel@nongnu.org" , "xen-devel@lists.xen.org" > -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: 11 September 2014 01:33 > To: Owen Smith > Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; Stefano Stabellini > Subject: Re: [PATCH 1/3] xenfb: Unregister keyboard event handler correct= ly >=20 > On Wed, 10 Sep 2014, Owen smith wrote: > > Keyboard event handler was replaced with a new handler on disconnect. > > Use the unregister function to remove keyboard handler. > > > > Signed-off-by: Owen smith > > --- > > hw/display/xenfb.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index > > 07ddc9d..2c39753 100644 > > --- a/hw/display/xenfb.c > > +++ b/hw/display/xenfb.c > > @@ -62,6 +62,7 @@ struct XenInput { > > int abs_pointer_wanted; /* Whether guest supports absolute pointer= */ > > int button_state; /* Last seen pointer button state */ > > int extended; > > + QEMUPutKbdEntry *qkbd; > > QEMUPutMouseEntry *qmouse; > > }; > > > > @@ -364,7 +365,6 @@ static int input_initialise(struct XenDevice *xende= v) > > if (rc !=3D 0) > > return rc; > > > > - qemu_add_kbd_event_handler(xenfb_key_event, in); > > return 0; > > } > > > > @@ -383,17 +383,26 @@ static void input_connected(struct XenDevice > *xendev) > > in->qmouse =3D qemu_add_mouse_event_handler(xenfb_mouse_event, > in, > > in->abs_pointer_wanted, > > "Xen PVFB Mouse"); > > + qemu_activate_mouse_event_handler(in->qmouse); >=20 > This change is not described in the commit message. > Why is this necessary? Is it related to the keyboard change? >=20 I'll split this change into a separate patch for v2. Without this, the xenfb_mouse_event callback was not being called in respon= se to mouse events, as events are only delivered to the first matching handler= in the input chain. Calling qemu_add_kbd_event_handler activates the handler internally, but qemu_add_mouse_event_handler does not activate the handler. >=20 > > + if (in->qkbd) { > > + qemu_remove_kbd_event_handler(in->qkbd); > > + } > > + in->qkbd =3D qemu_add_kbd_event_handler(xenfb_key_event, in); > > } > > > > static void input_disconnect(struct XenDevice *xendev) { > > struct XenInput *in =3D container_of(xendev, struct XenInput, > > c.xendev); > > > > + if (in->qkbd) { > > + qemu_remove_kbd_event_handler(in->qkbd); > > + in->qkbd =3D NULL; > > + } > > if (in->qmouse) { > > qemu_remove_mouse_event_handler(in->qmouse); > > in->qmouse =3D NULL; > > } > > - qemu_add_kbd_event_handler(NULL, NULL); > > common_unbind(&in->c); > > } > > > > -- > > 2.1.0 > >