@@ -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);
@@ -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);
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 <jsnow@redhat.com> --- block/backup.c | 2 +- blockjob.c | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-)