Re: [Qemu-devel] [PATCH] i386/vmmouse: Properly reset state

[Philippe Mathieu-Daudé]

On 8/29/19 8:12 PM, Jan Kiszka wrote:
> On 29.08.19 20:00, Philippe Mathieu-Daudé wrote:
>> Hi Jan,
>>
>> On 8/27/19 9:49 PM, Eduardo Habkost wrote:
>>> On Sun, Aug 25, 2019 at 04:58:18PM +0200, Jan Kiszka wrote:
>>>> On 21.07.19 10:58, Jan Kiszka wrote:
>>>>> From: Jan Kiszka <address@hidden>
>>>>>
>>>>> nb_queue was not zeroed so that we no longer delivered events if a
>>>>> previous guest left the device in an overflow state.
>>>>>
>>>>> The state of absolute does not matter as the next
>>>>> vmmouse_update_handler
>>>>> call will align it again.
>>>>>
>>>>> Signed-off-by: Jan Kiszka <address@hidden>
>>>>> ---
>>>>>    hw/i386/vmmouse.c | 1 +
>>>>>    1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c
>>>>> index 5d2d278be4..e335bd07da 100644
>>>>> --- a/hw/i386/vmmouse.c
>>>>> +++ b/hw/i386/vmmouse.c
>>>>> @@ -257,6 +257,7 @@ static void vmmouse_reset(DeviceState *d)
>>>>>        VMMouseState *s = VMMOUSE(d);
>>>>>
>>>>>        s->queue_size = VMMOUSE_QUEUE_SIZE;
>>>>> +    s->nb_queue = 0;
>>
>> Don't we also need to reset the status in case vmmouse_get_status() is
>> called directly after reset?
>>
>>           s->status = 0;
>>
> 
> Thanks for checking. We call vmmouse_disable() here, and that sets
> status to
> 0xffff anyway.

I missed that, you are correct :)

So:
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

>>>>>
>>>>>        vmmouse_disable(s);
>>>>>    }
>>>>> -- 
>>>>> 2.16.4
>>>>>
>>>>>
>>>>
>>>> Ping - or who is looking after this?
>>>
>>> Despite being in hw/i386, I think we can say vmmouse.c doesn't
>>> have a maintainer.  Last time someone changed vmmouse.c in a
>>> meaningful way (not just adapting to API changes or removing
>>> duplicate code) was in 2012.
>>>
>>
>> Well it does has a few:
>>
>> $ ./scripts/get_maintainer.pl -f hw/i386/vmmouse.c
>> "Michael S. Tsirkin" <address@hidden> (supporter:PC)
>> Marcel Apfelbaum <address@hidden> (supporter:PC)
>> Paolo Bonzini <address@hidden> (maintainer:X86 TCG CPUs)
>> Richard Henderson <address@hidden> (maintainer:X86 TCG CPUs)
>> Eduardo Habkost <address@hidden> (maintainer:X86 TCG CPUs)
>>
>> However the correct section should rather be "PC Chipset".
>>
>>> But the change makes sense to me, so:
>>>
>>> Reviewed-by: Eduardo Habkost <address@hidden>
>>>
>>> I'll queue it.
>>>