[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH for-2.12 v2] qemu-iotests: update 185 output
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH for-2.12 v2] qemu-iotests: update 185 output |
Date: |
Wed, 11 Apr 2018 12:32:28 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
On 2018-04-10 10:11, Stefan Hajnoczi wrote:
> On Wed, Apr 04, 2018 at 06:16:12PM +0200, Max Reitz wrote:
>> On 2018-04-04 17:01, Stefan Hajnoczi wrote:
>> === Start mirror job and exit qemu ===
>>
>> This seems to be independent of whether there is actually data on
>> TEST_IMG (the commit source), so something doesn't seem quite right with
>> the block job throttling here...?
>
> I've been playing around with this test failure. Let's leave it broken
> (!) in QEMU 2.12 because this test has uncovered a block job ratelimit
> issue that's not a regression. The fix shouldn't be rushed.
OK for me.
> block/mirror.c calculates delay_ns but then discards it:
>
> static void coroutine_fn mirror_run(void *opaque)
> {
> ...
>
> for (;;) {
> ...delay_ns is calculated based on job speed...
>
> ret = 0;
> trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
> if (block_job_is_cancelled(&s->common) && s->common.force) {
> break;
> } else if (!should_complete) {
> delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0);
> ^--- we discard it!
> block_job_sleep_ns(&s->common, delay_ns);
> }
> s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> }
>
> ...
> }
Uh, nice.
> This is why the mirror-based tests are completing even though we'd
> expect them to only reach the "next" iteration due to the job speed.
>
> Increasing the 4 MB write operation in the test to >16 MB (the mirror
> buffer size) doesn't solve the problem because the next QMP command will
> race with the job's 0 ns sleep. It would just make the test unreliable.
>
> I'll work on the following for QEMU 2.13:
>
> 1. Avoid spurious block_job_enter() from block_job_drain(). This
> eliminates the extra block job iteration that changed the output in
> the first place.
>
> 2. Honor the job speed in block/mirror.c.
>
> The end result will be that the test output will not require changes.
Thanks!
Max
signature.asc
Description: OpenPGP digital signature