Re: [Qemu-block] [Qemu-devel] [PATCH 18/42] job: Move coroutine and related code to Job

[John Snow]

On 05/16/2018 12:50 PM, Kevin Wolf wrote:
> Am 15.05.2018 um 01:02 hat John Snow geschrieben:
>>
>>
>> On 05/09/2018 12:26 PM, Kevin Wolf wrote:
>>> This commit moves some core functions for dealing with the job coroutine
>>> from BlockJob to Job. This includes primarily entering the coroutine
>>> (both for the first and reentering) and yielding explicitly and at pause
>>> points.
>>>
>>> Signed-off-by: Kevin Wolf <address@hidden>
>>> ---
>>>  include/block/blockjob.h     |  40 ---------
>>>  include/block/blockjob_int.h |  26 ------
>>>  include/qemu/job.h           |  76 ++++++++++++++++
>>>  block/backup.c               |   2 +-
>>>  block/commit.c               |   4 +-
>>>  block/mirror.c               |  22 ++---
>>>  block/replication.c          |   2 +-
>>>  block/stream.c               |   4 +-
>>>  blockdev.c                   |   8 +-
>>>  blockjob.c                   | 201 
>>> +++++++------------------------------------
>>>  job.c                        | 137 +++++++++++++++++++++++++++++
>>>  tests/test-bdrv-drain.c      |  38 ++++----
>>>  tests/test-blockjob-txn.c    |  12 +--
>>>  tests/test-blockjob.c        |  14 +--
>>>  14 files changed, 296 insertions(+), 290 deletions(-)
>>>
>>
>> [...]
>>
>>>  
>>>  /* Assumes the block_job_mutex is held */
>>> -static bool block_job_timer_pending(BlockJob *job)
>>> +static bool job_timer_pending(Job *job)
>>
>> Is this intentionally left behind in blockjob.c, even once you've
>> changed the name (and moved the state to job.c?)
> 
> Yes, it's just a small callback that is structually part of
> block_job_set_speed(). Maybe I shouldn't rename it, but it does get a
> Job rather than a BlockJob now, so I'm not sure.
> 
> Kevin
> 

Your choice; it's not clear to me yet which things truly belong in which
file because I haven't had a chance to look at the finished result. I'm
sure if it looks like it needs to move later that someone will
eventually move it.

--js