aboutsummaryrefslogtreecommitdiff
path: root/block
diff options
context:
space:
mode:
authorJohn Snow <jsnow@redhat.com>2018-03-10 03:27:37 -0500
committerKevin Wolf <kwolf@redhat.com>2018-03-19 12:01:24 +0100
commit35d6b368f2fcc52de01773b80246bfbe55211903 (patch)
treebac0072d2722d8d12ce5a94933c030c9e84979a3 /block
parent75f710599f2c35bad1a724d0243f372d05cc07ed (diff)
downloadqemu-35d6b368f2fcc52de01773b80246bfbe55211903.zip
qemu-35d6b368f2fcc52de01773b80246bfbe55211903.tar.gz
qemu-35d6b368f2fcc52de01773b80246bfbe55211903.tar.bz2
blockjobs: ensure abort is called for cancelled jobs
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. We need to check for cancellation in both block_job_completed AND block_job_completed_single, because jobs may be cancelled between those two calls; for instance in transactions. This also necessitates an ABORTING -> ABORTING transition to be allowed. The check in block_job_completed could be removed, but there's no point in starting to attempt to succeed a transaction that we know in advance will fail. 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> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Diffstat (limited to 'block')
-rw-r--r--block/backup.c2
-rw-r--r--block/trace-events1
2 files changed, 2 insertions, 1 deletions
diff --git a/block/backup.c b/block/backup.c
index 7e254da..453cd62 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/block/trace-events b/block/trace-events
index 7212f2a..1c6edb0 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -5,6 +5,7 @@ bdrv_open_common(void *bs, const char *filename, int flags, const char *format_n
bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d"
# blockjob.c
+block_job_completed(void *job, int ret, int jret) "job %p ret %d corrected ret %d"
block_job_state_transition(void *job, int ret, const char *legal, const char *s0, const char *s1) "job %p (ret: %d) attempting %s transition (%s-->%s)"
block_job_apply_verb(void *job, const char *state, const char *verb, const char *legal) "job %p in state %s; applying verb %s (%s)"