diff mbox series

[v2,09/13] block/backup: remove yield_and_check

Message ID 20180119205847.7141-10-jsnow@redhat.com
State New
Headers show
Series blockjob: refactor mirror_throttle | expand

Commit Message

John Snow Jan. 19, 2018, 8:58 p.m. UTC
This is a respin of the same functionality as mirror_throttle,
so trash this and replace it with the generic version.

yield_and_check returned true if canceled, false otherwise.
block_job_relax returns -ECANCELED if canceled, 0 otherwise.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 block/backup.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

Comments

Max Reitz Feb. 7, 2018, 10:33 p.m. UTC | #1
On 2018-01-19 21:58, John Snow wrote:
> This is a respin of the same functionality as mirror_throttle,
> so trash this and replace it with the generic version.
> 
> yield_and_check returned true if canceled, false otherwise.
> block_job_relax returns -ECANCELED if canceled, 0 otherwise.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  block/backup.c | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/block/backup.c b/block/backup.c
> index b4204c0ee4..0624c3b322 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -334,29 +334,17 @@ static void backup_complete(BlockJob *job, void *opaque)
>      g_free(data);
>  }
>  
> -static bool coroutine_fn yield_and_check(BackupBlockJob *job)
> +static uint64_t get_delay_ns(BackupBlockJob *job)
>  {
>      uint64_t delay_ns = 0;
>  
> -    if (block_job_is_cancelled(&job->common)) {
> -        return true;
> -    }
> -
> -    /* we need to yield so that bdrv_drain_all() returns.
> -     * (without, VM does not reboot)
> -     */
>      if (job->common.speed) {
>          delay_ns = ratelimit_calculate_delay(&job->limit,
>                                               job->bytes_read);
>          job->bytes_read = 0;
>      }
>  
> -    block_job_relax(&job->common, delay_ns);
> -    if (block_job_is_cancelled(&job->common)) {
> -        return true;
> -    }
> -
> -    return false;
> +    return delay_ns;
>  }
>  
>  static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
> @@ -369,7 +357,7 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
>      hbitmap_iter_init(&hbi, job->copy_bitmap, 0);
>      while ((cluster = hbitmap_iter_next(&hbi)) != -1) {
>          do {
> -            if (yield_and_check(job)) {
> +            if (block_job_relax(&job->common, get_delay_ns(job))) {
>                  return 0;
>              }
>              ret = backup_do_cow(job, cluster * job->cluster_size,
> @@ -465,7 +453,7 @@ static void coroutine_fn backup_run(void *opaque)
>              bool error_is_read;
>              int alloced = 0;
>  
> -            if (yield_and_check(job)) {
> +            if (block_job_relax(&job->common, get_delay_ns(job))) {
>                  break;
>              }
>  
> 

I'd very much prefer an explicit block_job_relax(...) == -ECANCELED, I
have to say.

If you coerce me, I can give an R-b, but you'll have to wrest it from me
by force!

Max
diff mbox series

Patch

diff --git a/block/backup.c b/block/backup.c
index b4204c0ee4..0624c3b322 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -334,29 +334,17 @@  static void backup_complete(BlockJob *job, void *opaque)
     g_free(data);
 }
 
-static bool coroutine_fn yield_and_check(BackupBlockJob *job)
+static uint64_t get_delay_ns(BackupBlockJob *job)
 {
     uint64_t delay_ns = 0;
 
-    if (block_job_is_cancelled(&job->common)) {
-        return true;
-    }
-
-    /* we need to yield so that bdrv_drain_all() returns.
-     * (without, VM does not reboot)
-     */
     if (job->common.speed) {
         delay_ns = ratelimit_calculate_delay(&job->limit,
                                              job->bytes_read);
         job->bytes_read = 0;
     }
 
-    block_job_relax(&job->common, delay_ns);
-    if (block_job_is_cancelled(&job->common)) {
-        return true;
-    }
-
-    return false;
+    return delay_ns;
 }
 
 static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
@@ -369,7 +357,7 @@  static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
     hbitmap_iter_init(&hbi, job->copy_bitmap, 0);
     while ((cluster = hbitmap_iter_next(&hbi)) != -1) {
         do {
-            if (yield_and_check(job)) {
+            if (block_job_relax(&job->common, get_delay_ns(job))) {
                 return 0;
             }
             ret = backup_do_cow(job, cluster * job->cluster_size,
@@ -465,7 +453,7 @@  static void coroutine_fn backup_run(void *opaque)
             bool error_is_read;
             int alloced = 0;
 
-            if (yield_and_check(job)) {
+            if (block_job_relax(&job->common, get_delay_ns(job))) {
                 break;
             }