Presently, even if a job is canceled post-completion as a result of
a failing peer in a transaction, it will still call .commit because
nothing has updated or changed its return code.
The reason why this does not cause problems currently is because
backup's implementation of .commit checks for cancellation itself.
I'd like to simplify this contract:
(1) Abort is called if the job/transaction fails
(2) Commit is called if the job/transaction succeeds
To this end: A job's return code, if 0, will be forcibly set as
-ECANCELED if that job has already concluded. Remove the now
redundant check in the backup job implementation.
This does NOT affect mirror jobs that are "canceled" during their
synchronous phase. The mirror job itself forcibly sets the canceled
property to false prior to ceding control, so such cases will invoke
the "commit" callback.
Signed-off-by: John Snow <js...@redhat.com>
---
block/backup.c | 2 +-
blockjob.c | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/block/backup.c b/block/backup.c
index d729263708..a17248feab 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -206,7 +206,7 @@ static void backup_cleanup_sync_bitmap(BackupBlockJob *job,
int ret)
BdrvDirtyBitmap *bm;
BlockDriverState *bs = blk_bs(job->common.blk);
- if (ret < 0 || block_job_is_cancelled(&job->common)) {
+ if (ret < 0) {
/* Merge the successor back into the parent, delete nothing. */
bm = bdrv_reclaim_dirty_bitmap(bs, job->sync_bitmap, NULL);
assert(bm);
diff --git a/blockjob.c b/blockjob.c
index 0083fd7b0c..3d678d6ce2 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -380,6 +380,11 @@ static void block_job_completed_single(BlockJob *job)
{
assert(job->completed);
+ /* Ensure abort is called and QMP client is notified of cancellation */
+ if (job->ret == 0 && block_job_is_cancelled(job)) {
+ job->ret = -ECANCELED;
+ }
+
if (!job->ret) {
if (job->driver->commit) {
job->driver->commit(job);
--
2.14.3
