Re: [Qemu-devel] [PATCH 6/7] jobs: utilize job_exit shim

[John Snow]

On 08/22/2018 08:20 AM, Max Reitz wrote:
> On 2018-08-17 21:04, John Snow wrote:
>> Utilize the job_exit shim by not calling job_defer_to_main_loop, and
>> where applicable, converting the deferred callback into the job_exit
>> callback.
>>
>> This converts backup, stream, create, and the unit tests all at once.
>> None of these jobs undergo and order of operations changes, so it should
>> be a mechanical change.
>>
>> Signed-off-by: John Snow <address@hidden>
>> ---
>>  block/backup.c            | 16 ----------------
>>  block/create.c            | 14 +++-----------
>>  block/stream.c            | 22 +++++++---------------
>>  tests/test-bdrv-drain.c   |  6 ------
>>  tests/test-blockjob-txn.c | 11 ++---------
>>  tests/test-blockjob.c     | 10 ++++------
>>  6 files changed, 16 insertions(+), 63 deletions(-)
> 
> [...]
> 
>> diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
>> index 82cedee78b..ef29f35e44 100644
>> --- a/tests/test-blockjob-txn.c
>> +++ b/tests/test-blockjob-txn.c
>> @@ -24,17 +24,11 @@ typedef struct {
>>      int *result;
>>  } TestBlockJob;
>>  
>> -static void test_block_job_complete(Job *job, void *opaque)
>> +static void test_block_job_exit(Job *job)
>>  {
>>      BlockJob *bjob = container_of(job, BlockJob, job);
>>      BlockDriverState *bs = blk_bs(bjob->blk);
>> -    int rc = (intptr_t)opaque;
>>  
>> -    if (job_is_cancelled(job)) {
>> -        rc = -ECANCELED;
>> -    }
>> -
>> -    job_completed(job, rc);
>>      bdrv_unref(bs);
>>  }
> 
> That is a change in the order of operations, actually.  It's OK though
> because the BDS is still owned by the block job's BlockBackend.
> 
> (So I'd give an R-b, but I'm still unsure about the .exit() interface.)
> 
> Max
> 

Good eye; it does change the order of operations here, but like the
preceding two patches it should be safe for the same reasons.

I'll just update the commit message to keep things easier.