On Wed, Oct 06, 2021 at 05:19:35PM +0200, Hanna Reitz wrote: > Most callers of job_is_cancelled() actually want to know whether the job > is on its way to immediate termination. For example, we refuse to pause > jobs that are cancelled; but this only makes sense for jobs that are > really actually cancelled. > > A mirror job that is cancelled during READY with force=false should > absolutely be allowed to pause. This "cancellation" (which is actually > a kind of completion) may take an indefinite amount of time, and so > should behave like any job during normal operation. For example, with > on-target-error=stop, the job should stop on write errors. (In > contrast, force-cancelled jobs should not get write errors, as they > should just terminate and not do further I/O.) > > Therefore, redefine job_is_cancelled() to only return true for jobs that > are force-cancelled (which as of HEAD^ means any job that interprets the
Is this reference still accurate with the addition of new patch 7? > cancellation request as a request for immediate termination), and add > job_cancel_requested() as the general variant, which returns true for > any jobs which have been requested to be cancelled, whether it be > immediately or after an arbitrarily long completion phase. > > Finally, here is a justification for how different job_is_cancelled() > invocations are treated by this patch: ... > - job_update_rc(), job_finalize_single(), job_finish_sync(): These > functions are all called after the job has left its main loop. The > mirror job (the only job that can be soft-cancelled) will clear > .cancelled before leaving the main loop if it has been > soft-cancelled. Therefore, these functions will observe .cancelled > to be true only if the job has been force-cancelled. We can > continue to use job_is_cancelled(). > (Furthermore, conceptually, a soft-cancelled mirror job should not > report to have been cancelled. It should report completion (see > also the block-job-cancel QAPI documentation). Therefore, it makes > sense for these functions not to distinguish between a > soft-cancelled mirror job and a job that has completed as normal.) Thanks for the careful explanation. > > Buglink: https://gitlab.com/qemu-project/qemu/-/issues/462 > Signed-off-by: Hanna Reitz <hre...@redhat.com> > --- > include/qemu/job.h | 8 +++++++- > block/mirror.c | 10 ++++------ > job.c | 14 ++++++++++++-- > 3 files changed, 23 insertions(+), 9 deletions(-) The commit message is loads longer than the patch itself, but for good reason. Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
