[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 18/38] ahci-test: Drop dependence on global_q
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH v7 18/38] ahci-test: Drop dependence on global_qtest |
Date: |
Mon, 11 Sep 2017 20:21:35 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 09/11/2017 08:20 PM, John Snow wrote:
>
>
> On 09/11/2017 01:20 PM, Eric Blake wrote:
>> Managing parallel connections to two different monitors via
>> the implicit global_qtest makes it hard to copy-and-paste code
>> to tests that are not aware of the implicit state; the
>> management of global_qtest is even harder to follow because
>> it was masked behind set_context().
>>
>> Instead, explicitly pass QTestState* around (generally, by
>> reusing the member already present in ahci->parent QOSState),
>> and call explicit qtest_* functions on all places that
>> interact with a monitor.
>>
>> We can assert that the conversion is correct by checking that
>> global_qtest remains NULL throughout the test (a later patch
>> that changes global_qtest to not be a public global variable
>> will drop the assertions).
>>
>> Bonus: there was one spots that was creating a needless temporary
>> variable to execute the 'cont' command, rather than just directly
>> passing the literal command through qtest_qmp(). Fixing that
>> gets us one step closer to enabling -Wformat checking on
>> constructed JSON.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>
> still LGTM, you can probably still see why I was a little iffy about
> making the global qstate ubiquitous instead of doing it this way.
>
> (Though it does make the calls a little more verbose, they are IMO a lot
> easier to reason about in tests that deal with mixed-state and
> migrations and so on, which -- most of my qtest experience was from AHCI
> -- there is a lot of here. (Sorry Markus, I'm a
lol! I hit send too soon, I tabbed away to look for Markus' previous
email which used some German term to refer to people who preferred
explicit state.
I wound up not finding it and then getting distracted.
Enjoy.
- Re: [Qemu-devel] [PATCH v7 13/38] libqos: Use explicit QTestState for fw_cfg operations, (continued)
- [Qemu-devel] [PATCH v7 16/38] libqos: Use explicit QTestState for ahci operations, Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 21/38] vhost-user-test: Drop dependence on global_qtest, Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 20/38] postcopy-test: Drop dependence on global_qtest, Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 22/38] qmp-test: Drop dependence on global_qtest, Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 19/38] ivshmem-test: Drop dependence on global_qtest, Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 18/38] ahci-test: Drop dependence on global_qtest, Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 17/38] libqos: Use explicit QTestState for remaining libqos operations, Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 23/38] tests/boot-sector: Drop dependence on global_qtest, Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 24/38] tests/acpi-utils: Drop dependence on global_qtest, Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 25/38] wdt_ib700-test: Drop dependence on global_qtest, Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 27/38] libqtest: Swap order of qtest_init() and qtest_start(), Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 30/38] qtest: Avoid passing raw strings through hmp(), Eric Blake, 2017/09/11