On 2018-05-09 18:26, Kevin Wolf wrote: > block_job_cancel_async() did two things that were still block job > specific: > > * Setting job->force. This field makes sense on the Job level, so we can > just move it. While at it, rename it to job->force_cancel to make its > purpose more obvious. > > * Resetting the I/O status. This can't be moved because generic Jobs > don't have an I/O status. What the function really implements is a > user resume, except without entering the coroutine. Consequently, it > makes sense to call the .user_resume driver callback here which > already resets the I/O status. > > The old block_job_cancel_async() has two separate if statements that > check job->iostatus != BLOCK_DEVICE_IO_STATUS_OK and job->user_paused. > However, the former condition always implies the latter (as is > asserted in block_job_iostatus_reset()), so changing the explicit call > of block_job_iostatus_reset() on the former condition with the > .user_resume callback on the latter condition is equivalent and > doesn't need to access any BlockJob specific state. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > include/block/blockjob.h | 6 ------ > include/qemu/job.h | 6 ++++++ > block/mirror.c | 4 ++-- > blockjob.c | 25 +++++++++++++------------ > 4 files changed, 21 insertions(+), 20 deletions(-)
I'm not quite sure why you keep this function in blockjob.c, when you've previously moved such static functions over to job.c and made them temporarily public (e.g. job_state_transition()). But I don't really care either way, in fact keeping the function in the same file makes reviewing easier for me, so: Reviewed-by: Max Reitz <mre...@redhat.com>
signature.asc
Description: OpenPGP digital signature
