From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58622) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fB1mR-0000Un-B7 for qemu-devel@nongnu.org; Tue, 24 Apr 2018 13:31:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fB1mQ-00005L-1O for qemu-devel@nongnu.org; Tue, 24 Apr 2018 13:31:03 -0400 Date: Tue, 24 Apr 2018 13:30:46 -0400 From: Jeff Cody Message-ID: <20180424173046.GW12302@localhost.localdomain> References: <20180424123527.19168-1-stefanha@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180424123527.19168-1-stefanha@redhat.com> Subject: Re: [Qemu-devel] [PATCH] block/mirror: honor ratelimit again List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-devel@nongnu.org, Kevin Wolf , qemu-block@nongnu.org, Max Reitz , Liang Li On Tue, Apr 24, 2018 at 01:35:27PM +0100, Stefan Hajnoczi wrote: > Commit b76e4458b1eb3c32e9824fe6aa51f67d2b251748 ("block/mirror: change > the semantic of 'force' of block-job-cancel") accidentally removed the > ratelimit in the mirror job. > > Reintroduce the ratelimit but keep the block-job-cancel force=true > behavior that was added in commit > b76e4458b1eb3c32e9824fe6aa51f67d2b251748. > > Note that block_job_sleep_ns() returns immediately when the job is > cancelled. Therefore it's safe to unconditionally call > block_job_sleep_ns() - a cancelled job does not sleep. > > This commit fixes the non-deterministic qemu-iotests 185 output. The > test relies on the ratelimit to make the job sleep until the 'quit' > command is processed. Previously the job could complete before the > 'quit' command was received since there was no ratelimit. > > Cc: Liang Li > Cc: Jeff Cody > Cc: Kevin Wolf > Signed-off-by: Stefan Hajnoczi > --- > block/mirror.c | 8 +++++--- > tests/qemu-iotests/185.out | 4 ++-- > 2 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/block/mirror.c b/block/mirror.c > index 820f512c7b..9436a8d5ee 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -868,12 +868,14 @@ static void coroutine_fn mirror_run(void *opaque) > } > > ret = 0; > + > + if (s->synced && !should_complete) { > + delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0); > + } > trace_mirror_before_sleep(s, cnt, s->synced, delay_ns); > + block_job_sleep_ns(&s->common, 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); > - block_job_sleep_ns(&s->common, delay_ns); > } > s->last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); > } > diff --git a/tests/qemu-iotests/185.out b/tests/qemu-iotests/185.out > index 2c4b04de73..992162f418 100644 > --- a/tests/qemu-iotests/185.out > +++ b/tests/qemu-iotests/185.out > @@ -36,9 +36,9 @@ Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 backing_file=TEST_DIR/t.q > {"return": {}} > Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=67108864 cluster_size=65536 lazy_refcounts=off refcount_bits=16 > {"return": {}} > +{"return": {}} > +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} > {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "mirror"}} > -{"return": {}} > -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} > {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "mirror"}} > > === Start backup job and exit qemu === > -- > 2.14.3 > Thanks, Applied to my block branch: git://github.com/codyprime/qemu-kvm-jtc block -Jeff