| Message ID | 1417079052-9372-1-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 2014/11/27 17:04, Gerd Hoffmann wrote: > Cc: Gonglei <arei.gonglei@huawei.com> > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/input/hid.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/hw/input/hid.c b/hw/input/hid.c > index 8f6fbb3..6fb963f 100644 > --- a/hw/input/hid.c > +++ b/hw/input/hid.c > @@ -519,6 +519,27 @@ static int hid_post_load(void *opaque, int version_id) > HIDState *s = opaque; > > hid_set_next_idle(s); > + > + if (s->n == QUEUE_LENGTH && (s->kind == HID_TABLET || > + s->kind == HID_MOUSE)) { > + /* > + * Handle ptr device migration from old qemu with full queue. > + * > + * Throw away everything but the last event, so we propagate > + * at least the current button state to the guest. Also keep > + * current position for the tablet, signal "no motion" for the > + * mouse. > + */ > + HIDPointerEvent evt; > + evt = s->ptr.queue[(s->head+s->n) & QUEUE_MASK]; s->n is QUEUE_LENGTH, can we directly delete it? evt = s->ptr.queue[s->head & QUEUE_MASK] Best regards, -Gonglei > + if (s->kind == HID_MOUSE) { > + evt.xdx = 0; > + evt.ydy = 0; > + } > + s->ptr.queue[0] = evt; > + s->head = 0; > + s->n = 1; > + } > return 0; > } >
Hi, > > + evt = s->ptr.queue[(s->head+s->n) & QUEUE_MASK]; > > s->n is QUEUE_LENGTH, can we directly delete it? > evt = s->ptr.queue[s->head & QUEUE_MASK] I prefer to make clear in the code that we want the last ring element not the first and leave in the "+n", even if we could take it out. cheers, Gerd
On 2014/11/27 19:04, Gerd Hoffmann wrote: > Hi, > >>> + evt = s->ptr.queue[(s->head+s->n) & QUEUE_MASK]; >> >> s->n is QUEUE_LENGTH, can we directly delete it? >> evt = s->ptr.queue[s->head & QUEUE_MASK] > > I prefer to make clear in the code that we want the last ring element > not the first and leave in the "+n", even if we could take it out. > OK. Another question, whether or not we handle this scenario before calling hid_set_next_idle(s) ? Maybe it is safer, because hid_idle_timer will call hid_pointer_event(), callback function of hs->event(hs). Thanks, -Gonglei
On Do, 2014-11-27 at 19:16 +0800, Gonglei wrote: > On 2014/11/27 19:04, Gerd Hoffmann wrote: > > > Hi, > > > >>> + evt = s->ptr.queue[(s->head+s->n) & QUEUE_MASK]; > >> > >> s->n is QUEUE_LENGTH, can we directly delete it? > >> evt = s->ptr.queue[s->head & QUEUE_MASK] > > > > I prefer to make clear in the code that we want the last ring element > > not the first and leave in the "+n", even if we could take it out. > > > OK. > > Another question, whether or not we handle this scenario > before calling hid_set_next_idle(s) ? Maybe it is safer, because > hid_idle_timer will call hid_pointer_event(), callback function > of hs->event(hs). Not needed, the timer will not called before vmload is completely finished. cheers, Gerd
On 2014/11/27 19:19, Gerd Hoffmann wrote: > On Do, 2014-11-27 at 19:16 +0800, Gonglei wrote: >> On 2014/11/27 19:04, Gerd Hoffmann wrote: >> >>> Hi, >>> >>>>> + evt = s->ptr.queue[(s->head+s->n) & QUEUE_MASK]; >>>> >>>> s->n is QUEUE_LENGTH, can we directly delete it? >>>> evt = s->ptr.queue[s->head & QUEUE_MASK] >>> >>> I prefer to make clear in the code that we want the last ring element >>> not the first and leave in the "+n", even if we could take it out. >>> >> OK. >> >> Another question, whether or not we handle this scenario >> before calling hid_set_next_idle(s) ? Maybe it is safer, because >> hid_idle_timer will call hid_pointer_event(), callback function >> of hs->event(hs). > > Not needed, the timer will not called before vmload is completely > finished. > Yep, as the Qemu big lock. I will test this patch later, thanks for your work. :) Regards, -Gonglei
On 2014/11/27 19:22, Gonglei wrote: > On 2014/11/27 19:19, Gerd Hoffmann wrote: > >> On Do, 2014-11-27 at 19:16 +0800, Gonglei wrote: >>> On 2014/11/27 19:04, Gerd Hoffmann wrote: >>> >>>> Hi, >>>> >>>>>> + evt = s->ptr.queue[(s->head+s->n) & QUEUE_MASK]; >>>>> >>>>> s->n is QUEUE_LENGTH, can we directly delete it? >>>>> evt = s->ptr.queue[s->head & QUEUE_MASK] >>>> >>>> I prefer to make clear in the code that we want the last ring element >>>> not the first and leave in the "+n", even if we could take it out. >>>> >>> OK. >>> >>> Another question, whether or not we handle this scenario >>> before calling hid_set_next_idle(s) ? Maybe it is safer, because >>> hid_idle_timer will call hid_pointer_event(), callback function >>> of hs->event(hs). >> >> Not needed, the timer will not called before vmload is completely >> finished. >> > > Yep, as the Qemu big lock. > I will test this patch later, thanks for your work. :) > Patch works for me. Thanks. Tested-by: Gonglei <arei.gonglei@huawei.com> Reviewed-by: Gonglei <arei.gonglei@huawei.com> Regards, -Gonglei
diff --git a/hw/input/hid.c b/hw/input/hid.c index 8f6fbb3..6fb963f 100644 --- a/hw/input/hid.c +++ b/hw/input/hid.c @@ -519,6 +519,27 @@ static int hid_post_load(void *opaque, int version_id) HIDState *s = opaque; hid_set_next_idle(s); + + if (s->n == QUEUE_LENGTH && (s->kind == HID_TABLET || + s->kind == HID_MOUSE)) { + /* + * Handle ptr device migration from old qemu with full queue. + * + * Throw away everything but the last event, so we propagate + * at least the current button state to the guest. Also keep + * current position for the tablet, signal "no motion" for the + * mouse. + */ + HIDPointerEvent evt; + evt = s->ptr.queue[(s->head+s->n) & QUEUE_MASK]; + if (s->kind == HID_MOUSE) { + evt.xdx = 0; + evt.ydy = 0; + } + s->ptr.queue[0] = evt; + s->head = 0; + s->n = 1; + } return 0; }
Cc: Gonglei <arei.gonglei@huawei.com> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/input/hid.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)