Re: [Qemu-devel] [RFC] Break cross migration from qemu-1.5 to qemu-2.1. because of input/hid rewriting

[Dr. David Alan Gilbert]

* Gonglei (address@hidden) wrote:
> Hi, Gerd
> 
> I encounter a problem that breaking  migration from qemu-1.5 to qemu-2.1.
> The error message as below:
>  qemu-system-x86_64: hw/input/hid.c:121: hid_pointer_event: Assertion `hs->n 
> < 16' failed.
> Qemu assert in hid_pointer_event().

What is your test to reproduce this?

Dave

> I get the value of hs->n which is 16 by  reproduction. And the code of 
> qemu-1.5 :
> 
> static void hid_pointer_event(void *opaque,
>                               int x1, int y1, int z1, int buttons_state)
> {
>     HIDState *hs = opaque;
>     unsigned use_slot = (hs->head + hs->n - 1) & QUEUE_MASK;
>     unsigned previous_slot = (use_slot - 1) & QUEUE_MASK;
> 
>     if (hs->n == QUEUE_LENGTH) {
>         /* Queue full.  Discard old button state, combine motion normally.  */
>         hs->ptr.queue[use_slot].buttons_state = buttons_state;
>     }
> 
> Which indicate it is legal when hs->n == QUEUE_LENGTH.
> 
> But now:
> static void hid_pointer_event(DeviceState *dev, QemuConsole *src,
>                               InputEvent *evt)
> {
>     static const int bmap[INPUT_BUTTON_MAX] = {
>         [INPUT_BUTTON_LEFT]   = 0x01,
>         [INPUT_BUTTON_RIGHT]  = 0x02,
>         [INPUT_BUTTON_MIDDLE] = 0x04,
>     };
>     HIDState *hs = (HIDState *)dev;
>     HIDPointerEvent *e;
> 
>     assert(hs->n < QUEUE_LENGTH);
>     e = &hs->ptr.queue[(hs->head + hs->n) & QUEUE_MASK];
> ...
> 
> static void hid_pointer_sync(DeviceState *dev)
> {
>     HIDState *hs = (HIDState *)dev;
>     HIDPointerEvent *prev, *curr, *next;
>     bool event_compression = false;
> 
>     if (hs->n == QUEUE_LENGTH-1) {
>         /*
>          * Queue full.  We are losing information, but we at least
>          * keep track of most recent button state.
>          */
>         return;
>     }
> 
> What about this patch:
> 
> diff --git a/hw/input/hid.c b/hw/input/hid.c
> index 148c003..56e0637 100644
> --- a/hw/input/hid.c
> +++ b/hw/input/hid.c
> @@ -116,7 +116,7 @@ static void hid_pointer_event(DeviceState *dev, 
> QemuConsole *src,
>      HIDState *hs = (HIDState *)dev;
>      HIDPointerEvent *e;
> 
> -    assert(hs->n < QUEUE_LENGTH);
> +    assert(hs->n <= QUEUE_LENGTH);
>      e = &hs->ptr.queue[(hs->head + hs->n) & QUEUE_MASK];
> 
>      switch (evt->kind) {
> 
> Best regards,
> -Gonglei
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK