Skip to content

Commit

Permalink
Merge remote-tracking branch 'remotes/xanclic/tags/pull-block-2018-08…
Browse files Browse the repository at this point in the history 
…-31-v2' into staging

Block patches:
- (Block) job exit refactoring, part 1
  (removing job_defer_to_main_loop())
- test-bdrv-drain leak fix

# gpg: Signature made Fri 31 Aug 2018 15:30:33 BST
# gpg:                using RSA key F407DB0061D5CF40
# gpg: Good signature from "Max Reitz <mreitz@redhat.com>"
# Primary key fingerprint: 91BE B60A 30DB 3E88 57D1  1829 F407 DB00 61D5 CF40

* remotes/xanclic/tags/pull-block-2018-08-31-v2:
  jobs: remove job_defer_to_main_loop
  jobs: remove ret argument to job_completed; privatize it
  block/backup: make function variables consistently named
  jobs: utilize job_exit shim
  block/mirror: utilize job_exit shim
  block/commit: utilize job_exit shim
  jobs: add exit shim
  jobs: canonize Error object
  jobs: change start callback to run callback
  tests: fix bdrv-drain leak

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
  • Loading branch information
@pm215
pm215 committed Sep 24, 2018
2 parents 539c251 + e21a1c9 commit d6f71af
Show file tree
Hide file tree
Showing 12 changed files with 161 additions and 242 deletions.
81 changes: 33 additions & 48 deletions block/backup.c
View file
Expand Up @@ -380,18 +380,6 @@ static BlockErrorAction backup_error_action(BackupBlockJob *job,
}
}

typedef struct {
int ret;
} BackupCompleteData;

static void backup_complete(Job *job, void *opaque)
{
BackupCompleteData *data = opaque;

job_completed(job, data->ret, NULL);
g_free(data);
}

static bool coroutine_fn yield_and_check(BackupBlockJob *job)
{
uint64_t delay_ns;
Expand Down Expand Up @@ -480,60 +468,59 @@ static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
bdrv_dirty_iter_free(dbi);
}

static void coroutine_fn backup_run(void *opaque)
static int coroutine_fn backup_run(Job *job, Error **errp)
{
BackupBlockJob *job = opaque;
BackupCompleteData *data;
BlockDriverState *bs = blk_bs(job->common.blk);
BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
BlockDriverState *bs = blk_bs(s->common.blk);
int64_t offset, nb_clusters;
int ret = 0;

QLIST_INIT(&job->inflight_reqs);
qemu_co_rwlock_init(&job->flush_rwlock);
QLIST_INIT(&s->inflight_reqs);
qemu_co_rwlock_init(&s->flush_rwlock);

nb_clusters = DIV_ROUND_UP(job->len, job->cluster_size);
job_progress_set_remaining(&job->common.job, job->len);
nb_clusters = DIV_ROUND_UP(s->len, s->cluster_size);
job_progress_set_remaining(job, s->len);

job->copy_bitmap = hbitmap_alloc(nb_clusters, 0);
if (job->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
backup_incremental_init_copy_bitmap(job);
s->copy_bitmap = hbitmap_alloc(nb_clusters, 0);
if (s->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
backup_incremental_init_copy_bitmap(s);
} else {
hbitmap_set(job->copy_bitmap, 0, nb_clusters);
hbitmap_set(s->copy_bitmap, 0, nb_clusters);
}


job->before_write.notify = backup_before_write_notify;
bdrv_add_before_write_notifier(bs, &job->before_write);
s->before_write.notify = backup_before_write_notify;
bdrv_add_before_write_notifier(bs, &s->before_write);

if (job->sync_mode == MIRROR_SYNC_MODE_NONE) {
if (s->sync_mode == MIRROR_SYNC_MODE_NONE) {
/* All bits are set in copy_bitmap to allow any cluster to be copied.
* This does not actually require them to be copied. */
while (!job_is_cancelled(&job->common.job)) {
while (!job_is_cancelled(job)) {
/* Yield until the job is cancelled. We just let our before_write
* notify callback service CoW requests. */
job_yield(&job->common.job);
job_yield(job);
}
} else if (job->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
ret = backup_run_incremental(job);
} else if (s->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
ret = backup_run_incremental(s);
} else {
/* Both FULL and TOP SYNC_MODE's require copying.. */
for (offset = 0; offset < job->len;
offset += job->cluster_size) {
for (offset = 0; offset < s->len;
offset += s->cluster_size) {
bool error_is_read;
int alloced = 0;

if (yield_and_check(job)) {
if (yield_and_check(s)) {
break;
}

if (job->sync_mode == MIRROR_SYNC_MODE_TOP) {
if (s->sync_mode == MIRROR_SYNC_MODE_TOP) {
int i;
int64_t n;

/* Check to see if these blocks are already in the
* backing file. */

for (i = 0; i < job->cluster_size;) {
for (i = 0; i < s->cluster_size;) {
/* bdrv_is_allocated() only returns true/false based
* on the first set of sectors it comes across that
* are are all in the same state.
Expand All @@ -542,7 +529,7 @@ static void coroutine_fn backup_run(void *opaque)
* needed but at some point that is always the case. */
alloced =
bdrv_is_allocated(bs, offset + i,
job->cluster_size - i, &n);
s->cluster_size - i, &n);
i += n;

if (alloced || n == 0) {
Expand All @@ -560,33 +547,31 @@ static void coroutine_fn backup_run(void *opaque)
if (alloced < 0) {
ret = alloced;
} else {
ret = backup_do_cow(job, offset, job->cluster_size,
ret = backup_do_cow(s, offset, s->cluster_size,
&error_is_read, false);
}
if (ret < 0) {
/* Depending on error action, fail now or retry cluster */
BlockErrorAction action =
backup_error_action(job, error_is_read, -ret);
backup_error_action(s, error_is_read, -ret);
if (action == BLOCK_ERROR_ACTION_REPORT) {
break;
} else {
offset -= job->cluster_size;
offset -= s->cluster_size;
continue;
}
}
}
}

notifier_with_return_remove(&job->before_write);
notifier_with_return_remove(&s->before_write);

/* wait until pending backup_do_cow() calls have completed */
qemu_co_rwlock_wrlock(&job->flush_rwlock);
qemu_co_rwlock_unlock(&job->flush_rwlock);
hbitmap_free(job->copy_bitmap);
qemu_co_rwlock_wrlock(&s->flush_rwlock);
qemu_co_rwlock_unlock(&s->flush_rwlock);
hbitmap_free(s->copy_bitmap);

data = g_malloc(sizeof(*data));
data->ret = ret;
job_defer_to_main_loop(&job->common.job, backup_complete, data);
return ret;
}

static const BlockJobDriver backup_job_driver = {
Expand All @@ -596,7 +581,7 @@ static const BlockJobDriver backup_job_driver = {
.free = block_job_free,
.user_resume = block_job_user_resume,
.drain = block_job_drain,
.start = backup_run,
.run = backup_run,
.commit = backup_commit,
.abort = backup_abort,
.clean = backup_clean,
Expand Down
29 changes: 9 additions & 20 deletions block/commit.c
View file
Expand Up @@ -68,19 +68,13 @@ static int coroutine_fn commit_populate(BlockBackend *bs, BlockBackend *base,
return 0;
}

typedef struct {
int ret;
} CommitCompleteData;

static void commit_complete(Job *job, void *opaque)
static void commit_exit(Job *job)
{
CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
BlockJob *bjob = &s->common;
CommitCompleteData *data = opaque;
BlockDriverState *top = blk_bs(s->top);
BlockDriverState *base = blk_bs(s->base);
BlockDriverState *commit_top_bs = s->commit_top_bs;
int ret = data->ret;
bool remove_commit_top_bs = false;

/* Make sure commit_top_bs and top stay around until bdrv_replace_node() */
Expand All @@ -91,10 +85,10 @@ static void commit_complete(Job *job, void *opaque)
* the normal backing chain can be restored. */
blk_unref(s->base);

if (!job_is_cancelled(job) && ret == 0) {
if (!job_is_cancelled(job) && job->ret == 0) {
/* success */
ret = bdrv_drop_intermediate(s->commit_top_bs, base,
s->backing_file_str);
job->ret = bdrv_drop_intermediate(s->commit_top_bs, base,
s->backing_file_str);
} else {
/* XXX Can (or should) we somehow keep 'consistent read' blocked even
* after the failed/cancelled commit job is gone? If we already wrote
Expand All @@ -117,9 +111,6 @@ static void commit_complete(Job *job, void *opaque)
* bdrv_set_backing_hd() to fail. */
block_job_remove_all_bdrv(bjob);

job_completed(job, ret, NULL);
g_free(data);

/* If bdrv_drop_intermediate() didn't already do that, remove the commit
* filter driver from the backing chain. Do this as the final step so that
* the 'consistent read' permission can be granted. */
Expand All @@ -134,10 +125,9 @@ static void commit_complete(Job *job, void *opaque)
bdrv_unref(top);
}

static void coroutine_fn commit_run(void *opaque)
static int coroutine_fn commit_run(Job *job, Error **errp)
{
CommitBlockJob *s = opaque;
CommitCompleteData *data;
CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
int64_t offset;
uint64_t delay_ns = 0;
int ret = 0;
Expand Down Expand Up @@ -210,9 +200,7 @@ static void coroutine_fn commit_run(void *opaque)
out:
qemu_vfree(buf);

data = g_malloc(sizeof(*data));
data->ret = ret;
job_defer_to_main_loop(&s->common.job, commit_complete, data);
return ret;
}

static const BlockJobDriver commit_job_driver = {
Expand All @@ -222,7 +210,8 @@ static const BlockJobDriver commit_job_driver = {
.free = block_job_free,
.user_resume = block_job_user_resume,
.drain = block_job_drain,
.start = commit_run,
.run = commit_run,
.exit = commit_exit,
},
};

Expand Down
19 changes: 6 additions & 13 deletions block/create.c
View file
Expand Up @@ -34,33 +34,26 @@ typedef struct BlockdevCreateJob {
Job common;
BlockDriver *drv;
BlockdevCreateOptions *opts;
int ret;
Error *err;
} BlockdevCreateJob;

static void blockdev_create_complete(Job *job, void *opaque)
static int coroutine_fn blockdev_create_run(Job *job, Error **errp)
{
BlockdevCreateJob *s = container_of(job, BlockdevCreateJob, common);

job_completed(job, s->ret, s->err);
}

static void coroutine_fn blockdev_create_run(void *opaque)
{
BlockdevCreateJob *s = opaque;
int ret;

job_progress_set_remaining(&s->common, 1);
s->ret = s->drv->bdrv_co_create(s->opts, &s->err);
ret = s->drv->bdrv_co_create(s->opts, errp);
job_progress_update(&s->common, 1);

qapi_free_BlockdevCreateOptions(s->opts);
job_defer_to_main_loop(&s->common, blockdev_create_complete, NULL);

return ret;
}

static const JobDriver blockdev_create_job_driver = {
.instance_size = sizeof(BlockdevCreateJob),
.job_type = JOB_TYPE_CREATE,
.start = blockdev_create_run,
.run = blockdev_create_run,
};

void qmp_blockdev_create(const char *job_id, BlockdevCreateOptions *options,
Expand Down

0 comments on commit d6f71af

Please sign in to comment.