[Qemu-devel] [PATCH 08/10] blockjob: introduce block_job_cancel_async

Paolo Bonzini posted 10 patches 8 years, 10 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 08/10] blockjob: introduce block_job_cancel_async
Posted by Paolo Bonzini 8 years, 10 months ago
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 blockjob.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index c9cb5b1..093962b 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -373,6 +373,12 @@ static void block_job_completed_single(BlockJob *job)
     block_job_unref(job);
 }
 
+static void block_job_cancel_async(BlockJob *job)
+{
+    job->cancelled = true;
+    block_job_iostatus_reset(job);
+}
+
 static void block_job_completed_txn_abort(BlockJob *job)
 {
     AioContext *ctx;
@@ -397,7 +403,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
              * them; this job, however, may or may not be cancelled, depending
              * on the caller, so leave it. */
             if (other_job != job) {
-                other_job->cancelled = true;
+                block_job_cancel_async(other_job);
             }
             continue;
         }
@@ -489,8 +495,7 @@ void block_job_user_resume(BlockJob *job)
 void block_job_cancel(BlockJob *job)
 {
     if (block_job_started(job)) {
-        job->cancelled = true;
-        block_job_iostatus_reset(job);
+        block_job_cancel_async(job);
         block_job_enter(job);
     } else {
         block_job_completed(job, -ECANCELED);
-- 
2.9.3



Re: [Qemu-devel] [PATCH 08/10] blockjob: introduce block_job_cancel_async
Posted by John Snow 8 years, 10 months ago

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

What was the bad design that required you to fix the previous test? :)

> ---
>  blockjob.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index c9cb5b1..093962b 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -373,6 +373,12 @@ static void block_job_completed_single(BlockJob *job)
>      block_job_unref(job);
>  }
>  
> +static void block_job_cancel_async(BlockJob *job)
> +{
> +    job->cancelled = true;
> +    block_job_iostatus_reset(job);
> +}
> +
>  static void block_job_completed_txn_abort(BlockJob *job)
>  {
>      AioContext *ctx;
> @@ -397,7 +403,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
>               * them; this job, however, may or may not be cancelled, depending
>               * on the caller, so leave it. */
>              if (other_job != job) {
> -                other_job->cancelled = true;
> +                block_job_cancel_async(other_job);

Adds an ioreset here, which I think is probably fine...

>              }
>              continue;
>          }
> @@ -489,8 +495,7 @@ void block_job_user_resume(BlockJob *job)
>  void block_job_cancel(BlockJob *job)
>  {
>      if (block_job_started(job)) {
> -        job->cancelled = true;
> -        block_job_iostatus_reset(job);
> +        block_job_cancel_async(job);
>          block_job_enter(job);
>      } else {
>          block_job_completed(job, -ECANCELED);
> 

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

Re: [Qemu-devel] [PATCH 08/10] blockjob: introduce block_job_cancel_async
Posted by Paolo Bonzini 8 years, 10 months ago

On 08/04/2017 09:13, John Snow wrote:
> 
> 
> On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> What was the bad design that required you to fix the previous test? :)

It was actually patch 9 that had the bug, I'll reorder to match the
commit message.

Paolo

>> ---
>>  blockjob.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/blockjob.c b/blockjob.c
>> index c9cb5b1..093962b 100644
>> --- a/blockjob.c
>> +++ b/blockjob.c
>> @@ -373,6 +373,12 @@ static void block_job_completed_single(BlockJob *job)
>>      block_job_unref(job);
>>  }
>>  
>> +static void block_job_cancel_async(BlockJob *job)
>> +{
>> +    job->cancelled = true;
>> +    block_job_iostatus_reset(job);
>> +}
>> +
>>  static void block_job_completed_txn_abort(BlockJob *job)
>>  {
>>      AioContext *ctx;
>> @@ -397,7 +403,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
>>               * them; this job, however, may or may not be cancelled, depending
>>               * on the caller, so leave it. */
>>              if (other_job != job) {
>> -                other_job->cancelled = true;
>> +                block_job_cancel_async(other_job);
> 
> Adds an ioreset here, which I think is probably fine...
> 
>>              }
>>              continue;
>>          }
>> @@ -489,8 +495,7 @@ void block_job_user_resume(BlockJob *job)
>>  void block_job_cancel(BlockJob *job)
>>  {
>>      if (block_job_started(job)) {
>> -        job->cancelled = true;
>> -        block_job_iostatus_reset(job);
>> +        block_job_cancel_async(job);
>>          block_job_enter(job);
>>      } else {
>>          block_job_completed(job, -ECANCELED);
>>
> 
> Reviewed-by: John Snow <jsnow@redhat.com>
> 

Re: [Qemu-devel] [PATCH 08/10] blockjob: introduce block_job_cancel_async
Posted by Stefan Hajnoczi 8 years, 10 months ago
On Thu, Mar 23, 2017 at 06:39:26PM +0100, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  blockjob.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>