[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/4] qtest: fix "device_del" out-of-order events
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH 0/4] qtest: fix "device_del" out-of-order events |
Date: |
Wed, 13 Sep 2017 12:35:17 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 13.09.2017 12:28, Peter Xu wrote:
> On Wed, Sep 13, 2017 at 11:53:16AM +0200, Thomas Huth wrote:
>> On 13.09.2017 11:36, Peter Xu wrote:
>>> It starts from a "make check" failure on one of my private tree. The
>>> problem is that when we do "device_del" we normally looking for two
>>> things: one response (which is mostly empty), and a REMOVE event. The
>>> tricky point is the event can either be there before/after the empty
>>> response. So I added qmp_device_del() to make sure the order does not
>>> matter, then use it where proper.
>>>
>>> Since I'm at it, I also added the sister helper qmp_device_add(), it
>>> helps to remove LOCs.
>>
>> I've had a similar idea a couple of weeks ago, see here:
>>
>> http://patchwork.ozlabs.org/patch/801487/
>
> Good to know!
>
>>
>>> I still don't 100% sure why my private tree can trigger this error,
>>> while the master cannot. Anyway, I think this is something we should
>>> have, no matter what.
>>
>> Did you maybe touch the USB tests in your private tree?
>
> No. But I changed some internals of QMP there, I guess that's the
> reason that caused the misorder to happen more easily.
>
>> As far as I know, some test currently use QMP in a bad way, for example
>> usb_test_hotplug() only checks for the DEVICE_DELETED at the end, but
>> forgets to read back the final return value. That return value is then
>> presented to the next part of the code that uses QMP instead ... it
>> currently only works more or less by accident, but as soon as you try to
>> add new code inbetween, it certainly will fail.
>> ==> We really got to clean this up (either with my patch or your patch
>> series).
>
> Agree.
>
> I think your patch is nicer on the interface (as you have mentioned in
> the other reply), I can try to review it later.
>
> However it seems that your patch didn't really solve the problem I
> encountered (mis-ordered message arrivals). It would be good if you
> want to solve it together, or I can draft patch upon yours.
True, I'll try to respin my patch, including the ideas from your patch...
Thomas
- [Qemu-devel] [PATCH 0/4] qtest: fix "device_del" out-of-order events, Peter Xu, 2017/09/13
- [Qemu-devel] [PATCH 1/4] libqtest: add qmp_device_del(), Peter Xu, 2017/09/13
- [Qemu-devel] [PATCH 2/4] tests: use qmp_device_del() where proper, Peter Xu, 2017/09/13
- [Qemu-devel] [PATCH 3/4] libqtest: add qmp_device_add(), Peter Xu, 2017/09/13
- [Qemu-devel] [PATCH 4/4] tests: use qmp_device_add() where proper, Peter Xu, 2017/09/13
- Re: [Qemu-devel] [PATCH 0/4] qtest: fix "device_del" out-of-order events, Thomas Huth, 2017/09/13