Re: [Qemu-devel] [RFC PATCH 03/33] blockjob: Implement block_job_set_speed() centrally

2018-04-24 Thread Eric Blake
On 04/24/2018 10:24 AM, 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 
> ---

> +++ b/include/block/blockjob.h
> @@ -29,6 +29,8 @@
>  #include "block/block.h"
>  #include "qemu/ratelimit.h"
>  
> +#define SLICE_TIME 1ULL /* ns */

I know this is just code motion; but would it be any more legible as:

#define SLICE_TIME (100ULL * 1000 * 1000) /* ns */

to make it easier to see as 100 ms without having to count the 0s?

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [RFC PATCH 03/33] blockjob: Implement block_job_set_speed() centrally

2018-04-24 Thread Kevin Wolf
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 
---
 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 1ULL /* ns */
+
 typedef struct BlockJobDriver BlockJobDriver;
 typedef struct BlockJobTxn BlockJobTxn;
 
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 642adce68b..870bd346a8 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -41,9 +41,6 @@ struct BlockJobDriver {
 /** String describing the operation, part of query-block-jobs QMP API */
 BlockJobType job_type;
 
-/** Optional callback for job types that support setting a speed limit */
-void (*set_speed)(BlockJob *job, int64_t speed, Error **errp);
-
 /** Mandatory: Entrypoint for the Coroutine. */
 CoroutineEntry *start;
 
diff --git a/block/backup.c b/block/backup.c
index 7585c4391e..8468fd9f94 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -27,7 +27,6 @@
 #include "qemu/error-report.h"
 
 #define BACKUP_CLUSTER_SIZE_DEFAULT (1 << 16)
-#define SLICE_TIME 1ULL /* ns */
 
 typedef struct BackupBlockJob {
 BlockJob common;
@@ -190,17 +189,6 @@ static int coroutine_fn backup_before_write_notify(
 return backup_do_cow(job, req->offset, req->bytes, NULL, true);
 }
 
-static void backup_set_speed(BlockJob *job, int64_t speed, Error **errp)
-{
-BackupBlockJob *s = container_of(job, BackupBlockJob, common);
-
-if (speed < 0) {
-error_setg(errp, QERR_INVALID_PARAMETER, "speed");
-return;
-}
-ratelimit_set_speed(&s->common.limit, speed, SLICE_TIME);
-}
-
 static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret)
 {
 BdrvDirtyBitmap *bm;
@@ -540,7 +528,6 @@ static const BlockJobDriver backup_job_driver = {
 .instance_size  = sizeof(BackupBlockJob),
 .job_type   = BLOCK_JOB_TYPE_BACKUP,
 .start  = backup_run,
-.set_speed  = backup_set_speed,
 .commit = backup_commit,
 .abort  = backup_abort,
 .clean  = backup_clean,
diff --git a/block/commit.c b/block/commit.c
index beec5d0ad6..46cbeaec3e 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -31,8 +31,6 @@ enum {
 COMMIT_BUFFER_SIZE = 512 * 1024, /* in bytes */
 };
 
-#define SLICE_TIME 1ULL /* ns */
-
 typedef struct CommitBlockJob {
 BlockJob common;
 BlockDriverState *commit_top_bs;
@@ -216,21 +214,9 @@ out:
 block_job_defer_to_main_loop(&s->common, commit_complete, data);
 }
 
-static void commit_set_speed(BlockJob *job, int64_t speed, Error **errp)
-{
-CommitBlockJob *s = container_of(job, CommitBlockJob, common);
-
-if (speed < 0) {
-error_setg(errp, QERR_INVALID_PARAMETER, "speed");
-return;
-}
-ratelimit_set_speed(&s->common.limit, speed, SLICE_TIME);
-}
-
 static const BlockJobDriver commit_job_driver = {
 .instance_size = sizeof(CommitBlockJob),
 .job_type  = BLOCK_JOB_TYPE_COMMIT,
-.set_speed = commit_set_speed,
 .start = commit_run,
 };
 
diff --git a/block/mirror.c b/block/mirror.c
index aece2b185c..de495fddc4 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -22,7 +22,6 @@
 #include "qemu/ratelimit.h"
 #include "qemu/bitmap.h"
 
-#define SLICE_TIME1ULL /* ns */
 #define MAX_IN_FLIGHT 16
 #define MAX_IO_BYTES (1 << 20) /* 1 Mb */
 #define DEFAULT_MIRROR_BUF_SIZE (MAX_IN_FLIGHT * MAX_IO_BYTES)
@@ -904,17 +903,6 @@ immediate_exit:
 block_job_defer_to_main_loop(&s->common, mirror_exit, data);
 }
 
-static void mirror_set_speed(BlockJob *job, int64_t speed, Error **errp)
-{
-MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
-
-if (speed < 0) {
-error_setg(errp, QERR_INVALID_PARAMETER, "speed");
-return;
-}
-ratelimit_set_speed(&s->common.limit, speed, SLICE_TIME);
-}
-
 static void mirror_complete(BlockJob *job, Error **errp)
 {
 MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
@@ -999,7 +987,6 @@ static void mirror_drain(BlockJob *job)
 static const BlockJobDriver mirror_job_driver = {
 .instance_size  = sizeof(MirrorBlockJob),
 .job_type   = BLOCK_JOB_TYPE_MIRROR,
-.set_speed  = mirror_set_speed,
 .st