[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 1/5] block: Allow mirror_start to return job referen
From: |
John Snow |
Subject: |
[Qemu-devel] [PATCH 1/5] block: Allow mirror_start to return job references |
Date: |
Mon, 11 Jan 2016 19:36:08 -0500 |
Pick up an extra reference in mirror_start_job to allow callers
of mirror_start and commit_active_start to get a reference to
the job they have created. Phase out callers from fishing the job
out of bs->job -- use the return value instead.
Callers of mirror_start_job and commit_active_start are now
responsible for putting down their reference to the job.
No callers of mirror_start yet seem to need the reference, so
that's left as a void return for now.
Ultimately, this patch fixes qemu-img's reliance on bs->job.
Signed-off-by: John Snow <address@hidden>
---
block/mirror.c | 72 ++++++++++++++++++++++++++---------------------
blockdev.c | 8 ++++--
include/block/block_int.h | 10 +++----
qemu-img.c | 12 +++++---
4 files changed, 59 insertions(+), 43 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index f201f2b..92706ab 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -706,17 +706,18 @@ static const BlockJobDriver commit_active_job_driver = {
.complete = mirror_complete,
};
-static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
- const char *replaces,
- int64_t speed, uint32_t granularity,
- int64_t buf_size,
- BlockdevOnError on_source_error,
- BlockdevOnError on_target_error,
- bool unmap,
- BlockCompletionFunc *cb,
- void *opaque, Error **errp,
- const BlockJobDriver *driver,
- bool is_none_mode, BlockDriverState *base)
+static BlockJob *mirror_start_job(BlockDriverState *bs,
+ BlockDriverState *target,
+ const char *replaces,
+ int64_t speed, uint32_t granularity,
+ int64_t buf_size,
+ BlockdevOnError on_source_error,
+ BlockdevOnError on_target_error,
+ bool unmap,
+ BlockCompletionFunc *cb,
+ void *opaque, Error **errp,
+ const BlockJobDriver *driver,
+ bool is_none_mode, BlockDriverState *base)
{
MirrorBlockJob *s;
BlockDriverState *replaced_bs;
@@ -731,12 +732,12 @@ static void mirror_start_job(BlockDriverState *bs,
BlockDriverState *target,
on_source_error == BLOCKDEV_ON_ERROR_ENOSPC) &&
(!bs->blk || !blk_iostatus_is_enabled(bs->blk))) {
error_setg(errp, QERR_INVALID_PARAMETER, "on-source-error");
- return;
+ return NULL;
}
if (buf_size < 0) {
error_setg(errp, "Invalid parameter 'buf-size'");
- return;
+ return NULL;
}
if (buf_size == 0) {
@@ -748,19 +749,19 @@ static void mirror_start_job(BlockDriverState *bs,
BlockDriverState *target,
if (replaces) {
replaced_bs = bdrv_lookup_bs(replaces, replaces, errp);
if (replaced_bs == NULL) {
- return;
+ return NULL;
}
} else {
replaced_bs = bs;
}
if (replaced_bs->blk && target->blk) {
error_setg(errp, "Can't create node with two BlockBackends");
- return;
+ return NULL;
}
s = block_job_create(driver, bs, speed, cb, opaque, errp);
if (!s) {
- return;
+ return NULL;
}
s->replaces = g_strdup(replaces);
@@ -777,7 +778,7 @@ static void mirror_start_job(BlockDriverState *bs,
BlockDriverState *target,
if (!s->dirty_bitmap) {
g_free(s->replaces);
block_job_unref(&s->common);
- return;
+ return NULL;
}
bdrv_op_block_all(s->target, s->common.blocker);
@@ -787,9 +788,11 @@ static void mirror_start_job(BlockDriverState *bs,
BlockDriverState *target,
blk_set_on_error(s->target->blk, on_target_error, on_target_error);
blk_iostatus_enable(s->target->blk);
}
+ block_job_ref(&s->common);
s->common.co = qemu_coroutine_create(mirror_run);
trace_mirror_start(bs, s, s->common.co, opaque);
qemu_coroutine_enter(s->common.co, s);
+ return &s->common;
}
void mirror_start(BlockDriverState *bs, BlockDriverState *target,
@@ -803,6 +806,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState
*target,
{
bool is_none_mode;
BlockDriverState *base;
+ BlockJob *job;
if (mode == MIRROR_SYNC_MODE_INCREMENTAL) {
error_setg(errp, "Sync mode 'incremental' not supported");
@@ -810,27 +814,31 @@ void mirror_start(BlockDriverState *bs, BlockDriverState
*target,
}
is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
base = mode == MIRROR_SYNC_MODE_TOP ? backing_bs(bs) : NULL;
- mirror_start_job(bs, target, replaces,
- speed, granularity, buf_size,
- on_source_error, on_target_error, unmap, cb, opaque, errp,
- &mirror_job_driver, is_none_mode, base);
+ job = mirror_start_job(bs, target, replaces,
+ speed, granularity, buf_size,
+ on_source_error, on_target_error, unmap, cb, opaque,
+ errp, &mirror_job_driver, is_none_mode, base);
+ if (job) {
+ block_job_unref(job);
+ }
}
-void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
- int64_t speed,
- BlockdevOnError on_error,
- BlockCompletionFunc *cb,
- void *opaque, Error **errp)
+BlockJob *commit_active_start(BlockDriverState *bs, BlockDriverState *base,
+ int64_t speed,
+ BlockdevOnError on_error,
+ BlockCompletionFunc *cb,
+ void *opaque, Error **errp)
{
int64_t length, base_length;
int orig_base_flags;
int ret;
Error *local_err = NULL;
+ BlockJob *job;
orig_base_flags = bdrv_get_flags(base);
if (bdrv_reopen(base, bs->open_flags, errp)) {
- return;
+ return NULL;
}
length = bdrv_getlength(bs);
@@ -859,19 +867,19 @@ void commit_active_start(BlockDriverState *bs,
BlockDriverState *base,
}
bdrv_ref(base);
- mirror_start_job(bs, base, NULL, speed, 0, 0,
- on_error, on_error, false, cb, opaque, &local_err,
- &commit_active_job_driver, false, base);
+ job = mirror_start_job(bs, base, NULL, speed, 0, 0,
+ on_error, on_error, false, cb, opaque, &local_err,
+ &commit_active_job_driver, false, base);
if (local_err) {
error_propagate(errp, local_err);
goto error_restore_flags;
}
- return;
+ return job;
error_restore_flags:
/* ignore error and errp for bdrv_reopen, because we want to propagate
* the original error */
bdrv_reopen(base, orig_base_flags, NULL);
- return;
+ return NULL;
}
diff --git a/blockdev.c b/blockdev.c
index 2df0c6d..d31bb03 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2956,6 +2956,7 @@ void qmp_block_commit(const char *device,
BlockBackend *blk;
BlockDriverState *bs;
BlockDriverState *base_bs, *top_bs;
+ BlockJob *job = NULL;
AioContext *aio_context;
Error *local_err = NULL;
/* This will be part of the QMP command, if/when the
@@ -3037,8 +3038,8 @@ void qmp_block_commit(const char *device,
" but 'top' is the active layer");
goto out;
}
- commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
- bs, &local_err);
+ job = commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
+ bs, &local_err);
} else {
commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
has_backing_file ? backing_file : NULL, &local_err);
@@ -3049,6 +3050,9 @@ void qmp_block_commit(const char *device,
}
out:
+ if (job) {
+ block_job_unref(job);
+ }
aio_context_release(aio_context);
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 256609d..a68c7dc 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -630,11 +630,11 @@ void commit_start(BlockDriverState *bs, BlockDriverState
*base,
* @errp: Error object.
*
*/
-void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
- int64_t speed,
- BlockdevOnError on_error,
- BlockCompletionFunc *cb,
- void *opaque, Error **errp);
+BlockJob *commit_active_start(BlockDriverState *bs, BlockDriverState *base,
+ int64_t speed,
+ BlockdevOnError on_error,
+ BlockCompletionFunc *cb,
+ void *opaque, Error **errp);
/*
* mirror_start:
* @bs: Block device to operate on.
diff --git a/qemu-img.c b/qemu-img.c
index 3d48b4f..7649f80 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -647,8 +647,9 @@ static void common_block_job_cb(void *opaque, int ret)
}
}
-static void run_block_job(BlockJob *job, Error **errp)
+static void run_block_job(BlockJob **pjob, Error **errp)
{
+ BlockJob *job = *pjob;
AioContext *aio_context = bdrv_get_aio_context(job->bs);
do {
@@ -662,6 +663,8 @@ static void run_block_job(BlockJob *job, Error **errp)
/* A block job may finish instantaneously without publishing any progress,
* so just signal completion here */
qemu_progress_print(100.f, 0);
+ block_job_unref(job);
+ *pjob = NULL;
}
static int img_commit(int argc, char **argv)
@@ -670,6 +673,7 @@ static int img_commit(int argc, char **argv)
const char *filename, *fmt, *cache, *base;
BlockBackend *blk;
BlockDriverState *bs, *base_bs;
+ BlockJob *job;
bool progress = false, quiet = false, drop = false;
Error *local_err = NULL;
CommonBlockJobCBInfo cbi;
@@ -758,8 +762,8 @@ static int img_commit(int argc, char **argv)
.bs = bs,
};
- commit_active_start(bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
- common_block_job_cb, &cbi, &local_err);
+ job = commit_active_start(bs, base_bs, 0, BLOCKDEV_ON_ERROR_REPORT,
+ common_block_job_cb, &cbi, &local_err);
if (local_err) {
goto done;
}
@@ -772,7 +776,7 @@ static int img_commit(int argc, char **argv)
bdrv_ref(bs);
}
- run_block_job(bs->job, &local_err);
+ run_block_job(&job, &local_err);
if (local_err) {
goto unref_backing;
}
--
2.4.3
- [Qemu-devel] [PATCH 0/5] block: reduce reliance on bs->job pointer, John Snow, 2016/01/11
- [Qemu-devel] [PATCH 4/5] block/backup: Add subclassed notifier, John Snow, 2016/01/11
- Re: [Qemu-devel] [PATCH 4/5] block/backup: Add subclassed notifier, Paolo Bonzini, 2016/01/12
- Re: [Qemu-devel] [PATCH 4/5] block/backup: Add subclassed notifier, John Snow, 2016/01/12
- Re: [Qemu-devel] [PATCH 4/5] block/backup: Add subclassed notifier, Paolo Bonzini, 2016/01/12
- Re: [Qemu-devel] [PATCH 4/5] block/backup: Add subclassed notifier, John Snow, 2016/01/12
- Re: [Qemu-devel] [PATCH 4/5] block/backup: Add subclassed notifier, Kevin Wolf, 2016/01/18
- Re: [Qemu-devel] [PATCH 4/5] block/backup: Add subclassed notifier, John Snow, 2016/01/18
[Qemu-devel] [PATCH 2/5] block: Allow stream_start to return job references, John Snow, 2016/01/11
[Qemu-devel] [PATCH 1/5] block: Allow mirror_start to return job references,
John Snow <=
[Qemu-devel] [PATCH 3/5] block: allow backup_start to return job references, John Snow, 2016/01/11
[Qemu-devel] [PATCH 5/5] blockjob: add Job parameter to BlockCompletionFunc, John Snow, 2016/01/11