diff mbox series

[RFC,v4,14/21] blockjobs: add block_job_txn_apply function

Message ID 20180223235142.21501-15-jsnow@redhat.com
State New
Headers show
Series blockjobs: add explicit job management | expand

Commit Message

John Snow Feb. 23, 2018, 11:51 p.m. UTC
Simply apply a function transaction-wide.
A few more uses of this in forthcoming patches.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 blockjob.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

Comments

Eric Blake Feb. 27, 2018, 7:52 p.m. UTC | #1
On 02/23/2018 05:51 PM, John Snow wrote:
> Simply apply a function transaction-wide.
> A few more uses of this in forthcoming patches.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   blockjob.c | 24 +++++++++++++++---------
>   1 file changed, 15 insertions(+), 9 deletions(-)
> 

> @@ -565,13 +577,7 @@ static void block_job_completed_txn_success(BlockJob *job)
>           }
>       }
>       /* We are the last completed job, commit the transaction. */
> -    QLIST_FOREACH_SAFE(other_job, &txn->jobs, txn_list, next) {
> -        ctx = blk_get_aio_context(other_job->blk);
> -        aio_context_acquire(ctx);
> -        assert(other_job->ret == 0);

We lost this assertion.  Hopefully that doesn't matter in the long run.

> -        block_job_completed_single(other_job);
> -        aio_context_release(ctx);
> -    }
> +    block_job_txn_apply(txn, block_job_completed_single);
>   }
>   
>   /* Assumes the block_job_mutex is held */
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Kevin Wolf Feb. 28, 2018, 4:32 p.m. UTC | #2
Am 24.02.2018 um 00:51 hat John Snow geschrieben:
> Simply apply a function transaction-wide.
> A few more uses of this in forthcoming patches.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  blockjob.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index 431ce9c220..8f02c03880 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -467,6 +467,19 @@ static void block_job_cancel_async(BlockJob *job)
>      job->cancelled = true;
>  }
>  
> +static void block_job_txn_apply(BlockJobTxn *txn, void fn(BlockJob *))
> +{
> +    AioContext *ctx;
> +    BlockJob *job, *next;
> +
> +    QLIST_FOREACH_SAFE(job, &txn->jobs, txn_list, next) {
> +        ctx = blk_get_aio_context(job->blk);
> +        aio_context_acquire(ctx);
> +        fn(job);
> +        aio_context_release(ctx);
> +    }
> +}
> +
>  static void block_job_do_dismiss(BlockJob *job)
>  {
>      assert(job);
> @@ -552,9 +565,8 @@ static void block_job_completed_txn_abort(BlockJob *job)
>  
>  static void block_job_completed_txn_success(BlockJob *job)
>  {
> -    AioContext *ctx;
>      BlockJobTxn *txn = job->txn;
> -    BlockJob *other_job, *next;
> +    BlockJob *other_job;
>      /*
>       * Successful completion, see if there are other running jobs in this
>       * txn.
> @@ -565,13 +577,7 @@ static void block_job_completed_txn_success(BlockJob *job)
>          }
>      }
>      /* We are the last completed job, commit the transaction. */
> -    QLIST_FOREACH_SAFE(other_job, &txn->jobs, txn_list, next) {
> -        ctx = blk_get_aio_context(other_job->blk);
> -        aio_context_acquire(ctx);
> -        assert(other_job->ret == 0);

Can we just move the assertion up a few lines (after checking
other_job->completed) instead of removing it?

> -        block_job_completed_single(other_job);
> -        aio_context_release(ctx);
> -    }
> +    block_job_txn_apply(txn, block_job_completed_single);
>  }

Kevin
diff mbox series

Patch

diff --git a/blockjob.c b/blockjob.c
index 431ce9c220..8f02c03880 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -467,6 +467,19 @@  static void block_job_cancel_async(BlockJob *job)
     job->cancelled = true;
 }
 
+static void block_job_txn_apply(BlockJobTxn *txn, void fn(BlockJob *))
+{
+    AioContext *ctx;
+    BlockJob *job, *next;
+
+    QLIST_FOREACH_SAFE(job, &txn->jobs, txn_list, next) {
+        ctx = blk_get_aio_context(job->blk);
+        aio_context_acquire(ctx);
+        fn(job);
+        aio_context_release(ctx);
+    }
+}
+
 static void block_job_do_dismiss(BlockJob *job)
 {
     assert(job);
@@ -552,9 +565,8 @@  static void block_job_completed_txn_abort(BlockJob *job)
 
 static void block_job_completed_txn_success(BlockJob *job)
 {
-    AioContext *ctx;
     BlockJobTxn *txn = job->txn;
-    BlockJob *other_job, *next;
+    BlockJob *other_job;
     /*
      * Successful completion, see if there are other running jobs in this
      * txn.
@@ -565,13 +577,7 @@  static void block_job_completed_txn_success(BlockJob *job)
         }
     }
     /* We are the last completed job, commit the transaction. */
-    QLIST_FOREACH_SAFE(other_job, &txn->jobs, txn_list, next) {
-        ctx = blk_get_aio_context(other_job->blk);
-        aio_context_acquire(ctx);
-        assert(other_job->ret == 0);
-        block_job_completed_single(other_job);
-        aio_context_release(ctx);
-    }
+    block_job_txn_apply(txn, block_job_completed_single);
 }
 
 /* Assumes the block_job_mutex is held */