From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:60848)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <dgilbert@redhat.com>) id 1Xrpxd-0004hf-Hp
	for qemu-devel@nongnu.org; Fri, 21 Nov 2014 10:17:30 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <dgilbert@redhat.com>) id 1XrpxX-0001a7-CA
	for qemu-devel@nongnu.org; Fri, 21 Nov 2014 10:17:25 -0500
Received: from mx1.redhat.com ([209.132.183.28]:57600)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <dgilbert@redhat.com>) id 1XrpxX-0001a3-3x
	for qemu-devel@nongnu.org; Fri, 21 Nov 2014 10:17:19 -0500
Date: Fri, 21 Nov 2014 15:16:47 +0000
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Message-ID: <20141121151647.GE2390@work-vm>
References: <546DE6AD.8010007@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <546DE6AD.8010007@huawei.com>
Subject: Re: [Qemu-devel] [RFC] Break cross migration from qemu-1.5 to
 qemu-2.1. because of input/hid rewriting
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Gonglei <arei.gonglei@huawei.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, "kraxel@redhat.com" <kraxel@redhat.com>

* Gonglei (arei.gonglei@huawei.com) 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 / dgilbert@redhat.com / Manchester, UK