From mboxrd@z Thu Jan 1 00:00:00 1970 From: Owen Smith Subject: Re: [PATCH 1/3] xenfb: Unregister keyboard event handler correctly Date: Thu, 11 Sep 2014 11:32:07 +0000 Message-ID: <6624BC057AF4E240B6D036F5CC505B12041F0030__4106.61994615129$1410435259$gmane$org@AMSPEX01CL02.citrite.net> References: <1410356546-22264-1-git-send-email-owen.smith@citrix.com> <1410356546-22264-2-git-send-email-owen.smith@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org Cc: Stefano Stabellini , "qemu-devel@nongnu.org" , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.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 correctly > > 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 *xendev) > > if (rc != 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 = qemu_add_mouse_event_handler(xenfb_mouse_event, > in, > > in->abs_pointer_wanted, > > "Xen PVFB Mouse"); > > + qemu_activate_mouse_event_handler(in->qmouse); > > This change is not described in the commit message. > Why is this necessary? Is it related to the keyboard change? > I'll split this change into a separate patch for v2. Without this, the xenfb_mouse_event callback was not being called in response 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. > > > + if (in->qkbd) { > > + qemu_remove_kbd_event_handler(in->qkbd); > > + } > > + in->qkbd = qemu_add_kbd_event_handler(xenfb_key_event, in); > > } > > > > static void input_disconnect(struct XenDevice *xendev) { > > struct XenInput *in = container_of(xendev, struct XenInput, > > c.xendev); > > > > + if (in->qkbd) { > > + qemu_remove_kbd_event_handler(in->qkbd); > > + in->qkbd = NULL; > > + } > > if (in->qmouse) { > > qemu_remove_mouse_event_handler(in->qmouse); > > in->qmouse = NULL; > > } > > - qemu_add_kbd_event_handler(NULL, NULL); > > common_unbind(&in->c); > > } > > > > -- > > 2.1.0 > >