Re: [Qemu-devel] [PATCH 06/10] blockjob: move iostatus reset inside block_job_user_resume

[John Snow]

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> Outside blockjob.c, the block_job_iostatus_reset function is used once
> in the monitor and once in BlockBackend.  When we introduce the block
> job mutex, block_job_iostatus_reset's client is going to be the block
> layer (for which blockjob.c will take the block job mutex) rather than
> the monitor (which will take the block job mutex by itself).
> 

Ah, sure. So we leave the block-backend call alone, here. We're moving
it from "public API" to "specifically block-layer API" to help
facilitate different mutex semantics. OK.

> The monitor's call to block_job_iostatus_reset from the monitor comes
> just before the sole call to block_job_user_resume, so reset the
> iostatus directly from block_job_iostatus_reset.  This will avoid
> the need to introduce separate block_job_iostatus_reset and
> block_job_iostatus_reset_locked APIs.
> 
> After making this change, move the function together with the others
> that were moved in the previous patch.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>

Reviewed-by: John Snow <address@hidden>

> ---
>  blockdev.c |  1 -
>  blockjob.c | 11 ++++++-----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index c5b2c2c..fbc376b 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3726,7 +3726,6 @@ void qmp_block_job_resume(const char *device, Error 
> **errp)
>      }
>  
>      trace_qmp_block_job_resume(job);
> -    block_job_iostatus_reset(job);
>      block_job_user_resume(job);
>      aio_context_release(aio_context);
>  }
> diff --git a/blockjob.c b/blockjob.c
> index c5f1d19..c9cb5b1 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -481,6 +481,7 @@ void block_job_user_resume(BlockJob *job)
>  {
>      if (job && job->user_paused && job->pause_count > 0) {
>          job->user_paused = false;
> +        block_job_iostatus_reset(job);
>          block_job_resume(job);
>      }
>  }
> @@ -496,11 +497,6 @@ void block_job_cancel(BlockJob *job)
>      }
>  }
>  
> -void block_job_iostatus_reset(BlockJob *job)
> -{
> -    job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
> -}
> -
>  static int block_job_finish_sync(BlockJob *job,
>                                   void (*finish)(BlockJob *, Error **errp),
>                                   Error **errp)
> @@ -751,6 +747,11 @@ void block_job_yield(BlockJob *job)
>      block_job_pause_point(job);
>  }
>  
> +void block_job_iostatus_reset(BlockJob *job)
> +{
> +    job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
> +}
> +
>  void block_job_event_ready(BlockJob *job)
>  {
>      job->ready = true;
>