On Fri, Dec 23, 2016 at 05:29:01PM +0300, Vladimir Sementsov-Ogievskiy wrote:
Just a few comments. I need to resume review of this series tomorrow.
> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index 1acb256..7d24cf6 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -248,6 +248,15 @@ void block_job_user_pause(BlockJob *job);
> bool block_job_user_paused(BlockJob *job);
>
> /**
> + * block_job_should_pause:
> + * @job: The job being queried.
> + *
> + * Returns whether the job is currently paused, or will pause
> + * as soon as it reaches a sleeping point.
s/sleeping point/pause point/
> + */
> +bool block_job_should_pause(BlockJob *job);
> +
> +/**
> * block_job_resume:
> * @job: The job to be resumed.
> *
> @@ -309,7 +318,11 @@ int block_job_complete_sync(BlockJob *job, Error **errp);
> */
> void block_job_iostatus_reset(BlockJob *job);
>
> -/**
> +
> +BlockErrorAction block_job_get_error_action(BlockJob *job,
> + BlockdevOnError on_err, int
> error);
Missing doc comment.
> +
> +/*
> * block_job_txn_new:
> *
> * Allocate and return a new block job transaction. Jobs can be added to the
> diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
> index 388b7b2..e15905f 100755
> --- a/tests/qemu-iotests/055
> +++ b/tests/qemu-iotests/055
> @@ -553,4 +553,4 @@ class TestDriveCompression(iotests.QMPTestCase):
> self.do_test_compress_pause('blockdev-backup', format,
> target='drive1')
>
> if __name__ == '__main__':
> - iotests.main(supported_fmts=['raw', 'qcow2'])
> + iotests.main(supported_fmts=['raw', 'qcow2'],
> supported_cache_modes=['none'])
The test has no real dependency on cache=none. Did you just add this to
influence performance in the hope that the test runs more
deterministically?
> diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
> index 9e87e1c..3d5e137 100644
> --- a/tests/qemu-iotests/129
> +++ b/tests/qemu-iotests/129
> @@ -66,8 +66,10 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
> result = self.vm.qmp("stop")
> self.assert_qmp(result, 'return', {})
> result = self.vm.qmp("query-block-jobs")
> - self.assert_qmp(result, 'return[0]/busy', True)
> - self.assert_qmp(result, 'return[0]/ready', False)
> + if result['return']:
> + # make additional check if block job is not released yet
> + self.assert_qmp(result, 'return[0]/busy', True)
> + self.assert_qmp(result, 'return[0]/ready', False)
This is silencing a failure rather than improving the test case. Why
does the test case fail for you? The rate limit is 1 KB/s, so the job
should have no chance of completing.
signature.asc
Description: PGP signature
