Re: [Qemu-block] [PATCH 04/42] blockjob: Implement block_job_set_speed() centrally

[Max Reitz]

On 2018-05-09 18:25, Kevin Wolf wrote:
> All block job drivers support .set_speed and all of them duplicate the
> same code to implement it. Move that code to blockjob.c and remove the
> now useless callback.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> ---
>  include/block/blockjob.h     |  2 ++
>  include/block/blockjob_int.h |  3 ---
>  block/backup.c               | 13 -------------
>  block/commit.c               | 14 --------------
>  block/mirror.c               | 14 --------------
>  block/stream.c               | 14 --------------
>  blockjob.c                   | 12 ++++--------
>  7 files changed, 6 insertions(+), 66 deletions(-)
> 
> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index 22bf418209..5aa8a6aaec 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -29,6 +29,8 @@
>  #include "block/block.h"
>  #include "qemu/ratelimit.h"
>  
> +#define SLICE_TIME 100000000ULL /* ns */
> +
>  typedef struct BlockJobDriver BlockJobDriver;
>  typedef struct BlockJobTxn BlockJobTxn;

SLICE_TIME can be anything.  I don't like something that can be anything
to be in a header file.  I can see that you still need it in mirror, so
it needs to be in a header; but maybe rename it to...
THROTTLE_SLICE_TIME?  At least JOB_SLICE_TIME?

Apart from that:

Reviewed-by: Max Reitz <address@hidden>

signature.asc
Description: OpenPGP digital signature