Re: [Qemu-block] [Qemu-devel] [PATCH v7 18/38] ahci-test: Drop dependence on global_qtest

[John Snow]

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

Reviewed-by: John Snow <address@hidden>