On Wed, 9 Sep 2020 21:59:24 +0300
Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> wrote:
> Better to return status together with setting errp. It allows to avoid
> error propagation in the caller.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> ---
Reviewed-by: Greg Kurz <gr...@kaod.org>
> include/block/blockjob.h | 2 +-
> blockjob.c | 18 ++++++++----------
> 2 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index 35faa3aa26..d200f33c10 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -139,7 +139,7 @@ bool block_job_has_bdrv(BlockJob *job, BlockDriverState
> *bs);
> * Set a rate-limiting parameter for the job; the actual meaning may
> * vary depending on the job type.
> */
> -void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp);
> +bool block_job_set_speed(BlockJob *job, int64_t speed, Error **errp);
>
> /**
> * block_job_query:
> diff --git a/blockjob.c b/blockjob.c
> index 470facfd47..afddf7a1fb 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -254,28 +254,30 @@ static bool job_timer_pending(Job *job)
> return timer_pending(&job->sleep_timer);
> }
>
> -void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
> +bool block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
> {
> int64_t old_speed = job->speed;
>
> - if (job_apply_verb(&job->job, JOB_VERB_SET_SPEED, errp)) {
> - return;
> + if (job_apply_verb(&job->job, JOB_VERB_SET_SPEED, errp) < 0) {
> + return false;
> }
> if (speed < 0) {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "speed",
> "a non-negative value");
> - return;
> + return false;
> }
>
> ratelimit_set_speed(&job->limit, speed, BLOCK_JOB_SLICE_TIME);
>
> job->speed = speed;
> if (speed && speed <= old_speed) {
> - return;
> + return true;
> }
>
> /* kick only if a timer is pending */
> job_enter_cond(&job->job, job_timer_pending);
> +
> + return true;
> }
>
> int64_t block_job_ratelimit_get_delay(BlockJob *job, uint64_t n)
> @@ -448,12 +450,8 @@ void *block_job_create(const char *job_id, const
> BlockJobDriver *driver,
>
> /* Only set speed when necessary to avoid NotSupported error */
> if (speed != 0) {
> - Error *local_err = NULL;
> -
> - block_job_set_speed(job, speed, &local_err);
> - if (local_err) {
> + if (!block_job_set_speed(job, speed, errp)) {
> job_early_fail(&job->job);
> - error_propagate(errp, local_err);
> return NULL;
> }
> }
