From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Actually block job is not completed without this final flush. It's
rather unexpected to have broken target when job was successfully
completed long ago and now we fail to flush or process just
crashed/killed.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/backup.c | 7 +++++--
block/commit.c | 2 +-
block/mirror.c | 4 ++++
block/stream.c | 7 ++++++-
blockjob.c | 18 ++++++++++++++++++
include/block/blockjob_int.h | 11 +++++++++++
6 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index db3791f4d1..b9ff63359a 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -295,10 +295,13 @@ static int coroutine_fn backup_run(Job *job, Error **errp)
job_yield(job);
}
} else {
- return backup_loop(s);
+ ret = backup_loop(s);
+ if (ret < 0) {
+ return ret;
+ }
}
- return 0;
+ return block_job_final_target_flush(&s->common, s->target_bs);
}
static void coroutine_fn backup_pause(Job *job)
diff --git a/block/commit.c b/block/commit.c
index aa45beb0f0..15df96b4f3 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -187,7 +187,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
}
}
- return 0;
+ return block_job_final_target_flush(&s->common, blk_bs(s->base));
}
static const BlockJobDriver commit_job_driver = {
diff --git a/block/mirror.c b/block/mirror.c
index d3cacd1708..cd19b49f7f 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1143,6 +1143,10 @@ immediate_exit:
g_free(s->in_flight_bitmap);
bdrv_dirty_iter_free(s->dbi);
+ if (ret >= 0) {
+ ret = block_job_final_target_flush(&s->common, blk_bs(s->target));
+ }
+
if (need_drain) {
s->in_drain = true;
bdrv_drained_begin(bs);
diff --git a/block/stream.c b/block/stream.c
index e522bbdec5..f7e8b35e94 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -131,6 +131,7 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
BlockDriverState *unfiltered_bs = bdrv_skip_filters(s->target_bs);
int64_t len;
int64_t offset = 0;
+ int ret;
int error = 0;
int64_t n = 0; /* bytes */
@@ -149,7 +150,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
for ( ; offset < len; offset += n) {
bool copy;
- int ret;
/* Note that even when no rate limit is applied we need to yield
* with no pending I/O here so that bdrv_drain_all() returns.
@@ -207,6 +207,11 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
}
}
+ ret = block_job_final_target_flush(&s->common, s->target_bs);
+ if (error == 0) {
+ error = ret;
+ }
+
/* Do not remove the backing file if an error was there but ignored. */
return error;
}
diff --git a/blockjob.c b/blockjob.c
index 25fe8e625d..313e586b0d 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -611,3 +611,21 @@ AioContext *block_job_get_aio_context(BlockJob *job)
GLOBAL_STATE_CODE();
return job->job.aio_context;
}
+
+int coroutine_fn
+block_job_final_target_flush(BlockJob *job, BlockDriverState *target_bs)
+{
+ int ret;
+
+ WITH_GRAPH_RDLOCK_GUARD() {
+ ret = bdrv_co_flush(target_bs);
+ }
+
+ if (ret < 0 && !block_job_is_internal(job)) {
+ qapi_event_send_block_job_error(job->job.id,
+ IO_OPERATION_TYPE_WRITE,
+ BLOCK_ERROR_ACTION_REPORT);
+ }
+
+ return ret;
+}
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 104824040c..617e40b916 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -152,4 +152,15 @@ void block_job_ratelimit_sleep(BlockJob *job);
BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,
int is_read, int error);
+/**
+ * block_job_final_target_flush:
+ * @job: The job to signal an error for if flush failed.
+ * @target_bs: The bs to flush.
+ *
+ * The function is intended to be called at the end of .run() for any data
+ * copying job.
+ */
+int coroutine_fn
+block_job_final_target_flush(BlockJob *job, BlockDriverState *target_bs);
+
#endif