diff mbox

[v3,08/15] blockjob: Simplify block_job_finish_sync

Message ID 1436500012-32593-9-git-send-email-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng July 10, 2015, 3:46 a.m. UTC
With job->completed and job->ret to replace BlockFinishData.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 blockjob.c | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

Comments

John Snow July 13, 2015, 11:08 p.m. UTC | #1
On 07/09/2015 11:46 PM, Fam Zheng wrote:
> With job->completed and job->ret to replace BlockFinishData.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  blockjob.c | 23 ++---------------------
>  1 file changed, 2 insertions(+), 21 deletions(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index fb1d9e7..11b48f5 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -179,43 +179,24 @@ struct BlockFinishData {
>      int ret;
>  };
>  
> -static void block_job_finish_cb(void *opaque, int ret)
> -{
> -    struct BlockFinishData *data = opaque;
> -
> -    data->cancelled = block_job_is_cancelled(data->job);
> -    data->ret = ret;
> -    data->cb(data->opaque, ret);
> -}
> -
>  static int block_job_finish_sync(BlockJob *job,
>                                   void (*finish)(BlockJob *, Error **errp),
>                                   Error **errp)
>  {
> -    struct BlockFinishData data;
>      BlockDriverState *bs = job->bs;
>      Error *local_err = NULL;
>  
>      assert(bs->job == job);
>  
> -    /* Set up our own callback to store the result and chain to
> -     * the original callback.
> -     */
> -    data.job = job;
> -    data.cb = job->cb;
> -    data.opaque = job->opaque;
> -    data.ret = -EINPROGRESS;
> -    job->cb = block_job_finish_cb;
> -    job->opaque = &data;
>      finish(job, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return -EBUSY;
>      }
> -    while (data.ret == -EINPROGRESS) {
> +    while (!job->completed) {
>          aio_poll(bdrv_get_aio_context(bs), true);
>      }
> -    return (data.cancelled && data.ret == 0) ? -ECANCELED : data.ret;
> +    return (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
>  }
>  
>  /* A wrapper around block_job_cancel() taking an Error ** parameter so it may be
> 

Hmm ... I'm less sure of this one because I can't imagine why it's split
out the way it is to begin with.

Looks sane to me, though...

Reviewed-by: John Snow <jsnow@redhat.com>
diff mbox

Patch

diff --git a/blockjob.c b/blockjob.c
index fb1d9e7..11b48f5 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -179,43 +179,24 @@  struct BlockFinishData {
     int ret;
 };
 
-static void block_job_finish_cb(void *opaque, int ret)
-{
-    struct BlockFinishData *data = opaque;
-
-    data->cancelled = block_job_is_cancelled(data->job);
-    data->ret = ret;
-    data->cb(data->opaque, ret);
-}
-
 static int block_job_finish_sync(BlockJob *job,
                                  void (*finish)(BlockJob *, Error **errp),
                                  Error **errp)
 {
-    struct BlockFinishData data;
     BlockDriverState *bs = job->bs;
     Error *local_err = NULL;
 
     assert(bs->job == job);
 
-    /* Set up our own callback to store the result and chain to
-     * the original callback.
-     */
-    data.job = job;
-    data.cb = job->cb;
-    data.opaque = job->opaque;
-    data.ret = -EINPROGRESS;
-    job->cb = block_job_finish_cb;
-    job->opaque = &data;
     finish(job, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return -EBUSY;
     }
-    while (data.ret == -EINPROGRESS) {
+    while (!job->completed) {
         aio_poll(bdrv_get_aio_context(bs), true);
     }
-    return (data.cancelled && data.ret == 0) ? -ECANCELED : data.ret;
+    return (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
 }
 
 /* A wrapper around block_job_cancel() taking an Error ** parameter so it may be