Re: [Qemu-devel] [PATCH 01/11] block: keep a list of block jobs

[Fam Zheng]

On Wed, 05/13 16:27, Alberto Garcia wrote:
> The current way to obtain the list of existing block jobs is to
> iterate over all root nodes and check which ones own a job.
> 
> Since we want to be able to support block jobs in other nodes as well,
> this patch keeps a list of jobs that is updated every time one is
> created or destroyed.
> 
> This also updates qmp_query_block_jobs() and bdrv_drain_all() to use
> this new list.
> 
> Signed-off-by: Alberto Garcia <address@hidden>
> Cc: Max Reitz <address@hidden>
> Cc: Eric Blake <address@hidden>
> ---
>  block/io.c               | 21 ++++++++-------------
>  blockdev.c               | 19 ++++++++-----------
>  blockjob.c               | 13 +++++++++++++
>  include/block/blockjob.h | 14 ++++++++++++++
>  4 files changed, 43 insertions(+), 24 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 1ce62c4..c38d776 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -310,21 +310,19 @@ void bdrv_drain_all(void)
>  {
>      /* Always run first iteration so any pending completion BHs run */
>      bool busy = true;
> -    BlockDriverState *bs = NULL;
> +    BlockJob *job = NULL;
>  
> -    while ((bs = bdrv_next(bs))) {
> -        AioContext *aio_context = bdrv_get_aio_context(bs);
> +    while ((job = block_job_next(job))) {
> +        AioContext *aio_context = bdrv_get_aio_context(job->bs);
>  
>          aio_context_acquire(aio_context);
> -        if (bs->job) {
> -            block_job_pause(bs->job);
> -        }
> +        block_job_pause(job);
>          aio_context_release(aio_context);
>      }
>  
>      while (busy) {
> +        BlockDriverState *bs = NULL;
>          busy = false;
> -        bs = NULL;
>  
>          while ((bs = bdrv_next(bs))) {
>              AioContext *aio_context = bdrv_get_aio_context(bs);
> @@ -335,14 +333,11 @@ void bdrv_drain_all(void)
>          }
>      }
>  
> -    bs = NULL;
> -    while ((bs = bdrv_next(bs))) {
> -        AioContext *aio_context = bdrv_get_aio_context(bs);
> +    while ((job = block_job_next(job))) {
> +        AioContext *aio_context = bdrv_get_aio_context(job->bs);
>  
>          aio_context_acquire(aio_context);
> -        if (bs->job) {
> -            block_job_resume(bs->job);
> -        }
> +        block_job_resume(job);
>          aio_context_release(aio_context);
>      }
>  }
> diff --git a/blockdev.c b/blockdev.c
> index 5eaf77e..bf36a0e 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3073,21 +3073,18 @@ fail:
>  BlockJobInfoList *qmp_query_block_jobs(Error **errp)
>  {
>      BlockJobInfoList *head = NULL, **p_next = &head;
> -    BlockDriverState *bs;
> +    BlockJob *job;
>  
> -    for (bs = bdrv_next(NULL); bs; bs = bdrv_next(bs)) {
> -        AioContext *aio_context = bdrv_get_aio_context(bs);
> +    for (job = block_job_next(NULL); job; job = block_job_next(job)) {
> +        BlockJobInfoList *elem = g_new0(BlockJobInfoList, 1);
> +        AioContext *aio_context = bdrv_get_aio_context(job->bs);
>  
>          aio_context_acquire(aio_context);
> -
> -        if (bs->job) {
> -            BlockJobInfoList *elem = g_new0(BlockJobInfoList, 1);
> -            elem->value = block_job_query(bs->job);
> -            *p_next = elem;
> -            p_next = &elem->next;
> -        }
> -
> +        elem->value = block_job_query(job);
>          aio_context_release(aio_context);
> +
> +        *p_next = elem;
> +        p_next = &elem->next;
>      }
>  
>      return head;
> diff --git a/blockjob.c b/blockjob.c
> index 2755465..c46984d 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -35,6 +35,16 @@
>  #include "qemu/timer.h"
>  #include "qapi-event.h"
>  
> +static QLIST_HEAD(, BlockJob) block_jobs = 
> QLIST_HEAD_INITIALIZER(block_jobs);
> +
> +BlockJob *block_job_next(BlockJob *job)
> +{
> +    if (!job) {
> +        return QLIST_FIRST(&block_jobs);
> +    }
> +    return QLIST_NEXT(job, job_list);
> +}
> +
>  void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
>                         int64_t speed, BlockCompletionFunc *cb,
>                         void *opaque, Error **errp)
> @@ -73,6 +83,8 @@ void *block_job_create(const BlockJobDriver *driver, 
> BlockDriverState *bs,
>              return NULL;
>          }
>      }
> +
> +    QLIST_INSERT_HEAD(&block_jobs, job, job_list);
>      return job;
>  }
>  
> @@ -85,6 +97,7 @@ void block_job_completed(BlockJob *job, int ret)
>      bs->job = NULL;
>      bdrv_op_unblock_all(bs, job->blocker);
>      error_free(job->blocker);
> +    QLIST_REMOVE(job, job_list);
>      g_free(job);
>  }
>  
> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index 57d8ef1..5431dd2 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -102,6 +102,9 @@ struct BlockJob {
>       */
>      bool ready;
>  
> +    /** Element of the list of block jobs */
> +    QLIST_ENTRY(BlockJob) job_list;
> +
>      /** Status that is published by the query-block-jobs QMP API */
>      BlockDeviceIoStatus iostatus;
>  
> @@ -125,6 +128,17 @@ struct BlockJob {
>  };
>  
>  /**
> + * block_job_next:
> + * @job: A block job, or %NULL.
> + *
> + * Get the next element from the list of block jobs after @job, or the
> + * first one if @job is %NULL.
> + *
> + * Returns the requested job, or %NULL if there are no more jobs left.
> + */
> +BlockJob *block_job_next(BlockJob *job);
> +
> +/**
>   * block_job_create:
>   * @job_type: The class object for the newly-created job.
>   * @bs: The block
> -- 
> 2.1.4
> 
> 

Reviewed-by: Fam Zheng <address@hidden>