[Qemu-devel] [PATCH 16/20] qemu-img: call aio_context_acquire/release around block job

[Paolo Bonzini]

This will be needed by bdrv_reopen_multiple, which calls
bdrv_drain_all and thus will *release* the AioContext.

Signed-off-by: Paolo Bonzini 
---
 qemu-img.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index afcd51f..ac7f40d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -795,6 +795,7 @@ static void run_block_job(BlockJob *job, Error **errp)
 {
 AioContext *aio_context = blk_get_aio_context(job->blk);
 
+aio_context_acquire(aio_context);
 do {
 aio_poll(aio_context, true);
 qemu_progress_print(job->len ?
@@ -802,6 +803,7 @@ static void run_block_job(BlockJob *job, Error **errp)
 } while (!job->ready);
 
 block_job_complete_sync(job, errp);
+aio_context_release(aio_context);
 
 /* A block job may finish instantaneously without publishing any progress,
  * so just signal completion here */
@@ -819,6 +821,7 @@ static int img_commit(int argc, char **argv)
 Error *local_err = NULL;
 CommonBlockJobCBInfo cbi;
 bool image_opts = false;
+AioContext *aio_context;
 
 fmt = NULL;
 cache = BDRV_DEFAULT_CACHE;
@@ -928,8 +931,11 @@ static int img_commit(int argc, char **argv)
 .bs   = bs,
 };
 
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
 commit_active_start("commit", bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
 common_block_job_cb, , _err, false);
+aio_context_release(aio_context);
 if (local_err) {
 goto done;
 }
-- 
2.7.4

Re: [Qemu-devel] [PATCH 16/20] qemu-img: call aio_context_acquire/release around block job

[Paolo Bonzini]

On 19/10/2016 02:54, Fam Zheng wrote:
> On Mon, 10/17 15:54, Paolo Bonzini wrote:
>> This will be needed by bdrv_reopen_multiple, which calls
>> bdrv_drain_all and thus will *release* the AioContext.
> 
> Looks okay, but I wonder how bdrv_drain_all releasing AioContext break 
> anything?

If you don't acquire it first, you get an assertion failure for
releasing a mutex you haven't acquired yet.

Initially I made aio_context_acquire a nop for the default QEMU context,
but then decided it was a hack.

Paolo

> Fam
> 
>>
>> Signed-off-by: Paolo Bonzini 
>> ---
>> v1->v2: new [qemu-iotests]
>>
>>  qemu-img.c | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 02c07b9..ad7c964 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -795,6 +795,7 @@ static void run_block_job(BlockJob *job, Error **errp)
>>  {
>>  AioContext *aio_context = blk_get_aio_context(job->blk);
>>  
>> +aio_context_acquire(aio_context);
>>  do {
>>  aio_poll(aio_context, true);
>>  qemu_progress_print(job->len ?
>> @@ -802,6 +803,7 @@ static void run_block_job(BlockJob *job, Error **errp)
>>  } while (!job->ready);
>>  
>>  block_job_complete_sync(job, errp);
>> +aio_context_release(aio_context);
>>  
>>  /* A block job may finish instantaneously without publishing any 
>> progress,
>>   * so just signal completion here */
>> @@ -819,6 +821,7 @@ static int img_commit(int argc, char **argv)
>>  Error *local_err = NULL;
>>  CommonBlockJobCBInfo cbi;
>>  bool image_opts = false;
>> +AioContext *aio_context;
>>  
>>  fmt = NULL;
>>  cache = BDRV_DEFAULT_CACHE;
>> @@ -928,8 +931,11 @@ static int img_commit(int argc, char **argv)
>>  .bs   = bs,
>>  };
>>  
>> +aio_context = bdrv_get_aio_context(bs);
>> +aio_context_acquire(aio_context);
>>  commit_active_start("commit", bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
>>  common_block_job_cb, , _err, false);
>> +aio_context_release(aio_context);
>>  if (local_err) {
>>  goto done;
>>  }
>> -- 
>> 2.7.4
>>
>>

Re: [Qemu-devel] [PATCH 16/20] qemu-img: call aio_context_acquire/release around block job

[Fam Zheng]

On Mon, 10/17 15:54, Paolo Bonzini wrote:
> This will be needed by bdrv_reopen_multiple, which calls
> bdrv_drain_all and thus will *release* the AioContext.

Looks okay, but I wonder how bdrv_drain_all releasing AioContext break anything?

Fam

> 
> Signed-off-by: Paolo Bonzini 
> ---
> v1->v2: new [qemu-iotests]
> 
>  qemu-img.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 02c07b9..ad7c964 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -795,6 +795,7 @@ static void run_block_job(BlockJob *job, Error **errp)
>  {
>  AioContext *aio_context = blk_get_aio_context(job->blk);
>  
> +aio_context_acquire(aio_context);
>  do {
>  aio_poll(aio_context, true);
>  qemu_progress_print(job->len ?
> @@ -802,6 +803,7 @@ static void run_block_job(BlockJob *job, Error **errp)
>  } while (!job->ready);
>  
>  block_job_complete_sync(job, errp);
> +aio_context_release(aio_context);
>  
>  /* A block job may finish instantaneously without publishing any 
> progress,
>   * so just signal completion here */
> @@ -819,6 +821,7 @@ static int img_commit(int argc, char **argv)
>  Error *local_err = NULL;
>  CommonBlockJobCBInfo cbi;
>  bool image_opts = false;
> +AioContext *aio_context;
>  
>  fmt = NULL;
>  cache = BDRV_DEFAULT_CACHE;
> @@ -928,8 +931,11 @@ static int img_commit(int argc, char **argv)
>  .bs   = bs,
>  };
>  
> +aio_context = bdrv_get_aio_context(bs);
> +aio_context_acquire(aio_context);
>  commit_active_start("commit", bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
>  common_block_job_cb, , _err, false);
> +aio_context_release(aio_context);
>  if (local_err) {
>  goto done;
>  }
> -- 
> 2.7.4
> 
> 

[Qemu-devel] [PATCH 16/20] qemu-img: call aio_context_acquire/release around block job

[Paolo Bonzini]

This will be needed by bdrv_reopen_multiple, which calls
bdrv_drain_all and thus will *release* the AioContext.

Signed-off-by: Paolo Bonzini 
---
v1->v2: new [qemu-iotests]

 qemu-img.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index 02c07b9..ad7c964 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -795,6 +795,7 @@ static void run_block_job(BlockJob *job, Error **errp)
 {
 AioContext *aio_context = blk_get_aio_context(job->blk);
 
+aio_context_acquire(aio_context);
 do {
 aio_poll(aio_context, true);
 qemu_progress_print(job->len ?
@@ -802,6 +803,7 @@ static void run_block_job(BlockJob *job, Error **errp)
 } while (!job->ready);
 
 block_job_complete_sync(job, errp);
+aio_context_release(aio_context);
 
 /* A block job may finish instantaneously without publishing any progress,
  * so just signal completion here */
@@ -819,6 +821,7 @@ static int img_commit(int argc, char **argv)
 Error *local_err = NULL;
 CommonBlockJobCBInfo cbi;
 bool image_opts = false;
+AioContext *aio_context;
 
 fmt = NULL;
 cache = BDRV_DEFAULT_CACHE;
@@ -928,8 +931,11 @@ static int img_commit(int argc, char **argv)
 .bs   = bs,
 };
 
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
 commit_active_start("commit", bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
 common_block_job_cb, , _err, false);
+aio_context_release(aio_context);
 if (local_err) {
 goto done;
 }
-- 
2.7.4