qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Subject: [Qemu-devel] [PULL 21/31] block: Convert block job core to BlockBackend
Date: Wed, 25 May 2016 19:39:46 +0200	[thread overview]
Message-ID: <1464197996-4581-22-git-send-email-kwolf@redhat.com> (raw)
In-Reply-To: <1464197996-4581-1-git-send-email-kwolf@redhat.com>

This adds a new BlockBackend field to the BlockJob struct, which
coexists with the BlockDriverState while converting the individual jobs.

When creating a block job, a new BlockBackend is created on top of the
given BlockDriverState, and it is destroyed when the BlockJob ends. The
reference to the BDS is now held by the BlockBackend instead of calling
bdrv_ref/unref manually.

We have to be careful when we use bdrv_replace_in_backing_chain() in
block jobs because this changes the BDS that job->blk points to. At the
moment block jobs are too tightly coupled with their BDS, so that moving
a job to another BDS isn't easily possible; therefore, we need to just
manually undo this change afterwards.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/mirror.c           |  3 +++
 blockjob.c               | 37 ++++++++++++++++++++-----------------
 include/block/blockjob.h |  3 ++-
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index b9986d8..efca8fc 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -478,6 +478,9 @@ static void mirror_exit(BlockJob *job, void *opaque)
             bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
         }
         bdrv_replace_in_backing_chain(to_replace, s->target);
+        /* We just changed the BDS the job BB refers to */
+        blk_remove_bs(job->blk);
+        blk_insert_bs(job->blk, src);
     }
 
 out:
diff --git a/blockjob.c b/blockjob.c
index e916b41..2097e1d 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -64,13 +64,17 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
                        int64_t speed, BlockCompletionFunc *cb,
                        void *opaque, Error **errp)
 {
+    BlockBackend *blk;
     BlockJob *job;
 
     if (bs->job) {
         error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs));
         return NULL;
     }
-    bdrv_ref(bs);
+
+    blk = blk_new();
+    blk_insert_bs(blk, bs);
+
     job = g_malloc0(driver->instance_size);
     error_setg(&job->blocker, "block device is in use by block job: %s",
                BlockJobType_lookup[driver->job_type]);
@@ -80,6 +84,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
     job->driver        = driver;
     job->id            = g_strdup(bdrv_get_device_name(bs));
     job->bs            = bs;
+    job->blk           = blk;
     job->cb            = cb;
     job->opaque        = opaque;
     job->busy          = true;
@@ -110,9 +115,10 @@ void block_job_ref(BlockJob *job)
 void block_job_unref(BlockJob *job)
 {
     if (--job->refcnt == 0) {
-        job->bs->job = NULL;
-        bdrv_op_unblock_all(job->bs, job->blocker);
-        bdrv_unref(job->bs);
+        BlockDriverState *bs = blk_bs(job->blk);
+        bs->job = NULL;
+        bdrv_op_unblock_all(bs, job->blocker);
+        blk_unref(job->blk);
         error_free(job->blocker);
         g_free(job->id);
         QLIST_REMOVE(job, job_list);
@@ -153,7 +159,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
     txn->aborting = true;
     /* We are the first failed job. Cancel other jobs. */
     QLIST_FOREACH(other_job, &txn->jobs, txn_list) {
-        ctx = bdrv_get_aio_context(other_job->bs);
+        ctx = blk_get_aio_context(other_job->blk);
         aio_context_acquire(ctx);
     }
     QLIST_FOREACH(other_job, &txn->jobs, txn_list) {
@@ -170,7 +176,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
         assert(other_job->completed);
     }
     QLIST_FOREACH_SAFE(other_job, &txn->jobs, txn_list, next) {
-        ctx = bdrv_get_aio_context(other_job->bs);
+        ctx = blk_get_aio_context(other_job->blk);
         block_job_completed_single(other_job);
         aio_context_release(ctx);
     }
@@ -192,7 +198,7 @@ static void block_job_completed_txn_success(BlockJob *job)
     }
     /* We are the last completed job, commit the transaction. */
     QLIST_FOREACH_SAFE(other_job, &txn->jobs, txn_list, next) {
-        ctx = bdrv_get_aio_context(other_job->bs);
+        ctx = blk_get_aio_context(other_job->blk);
         aio_context_acquire(ctx);
         assert(other_job->ret == 0);
         block_job_completed_single(other_job);
@@ -202,9 +208,7 @@ static void block_job_completed_txn_success(BlockJob *job)
 
 void block_job_completed(BlockJob *job, int ret)
 {
-    BlockDriverState *bs = job->bs;
-
-    assert(bs->job == job);
+    assert(blk_bs(job->blk)->job == job);
     assert(!job->completed);
     job->completed = true;
     job->ret = ret;
@@ -295,11 +299,10 @@ static int block_job_finish_sync(BlockJob *job,
                                  void (*finish)(BlockJob *, Error **errp),
                                  Error **errp)
 {
-    BlockDriverState *bs = job->bs;
     Error *local_err = NULL;
     int ret;
 
-    assert(bs->job == job);
+    assert(blk_bs(job->blk)->job == job);
 
     block_job_ref(job);
     finish(job, &local_err);
@@ -310,7 +313,7 @@ static int block_job_finish_sync(BlockJob *job,
     }
     while (!job->completed) {
         aio_poll(job->deferred_to_main_loop ? qemu_get_aio_context() :
-                                              bdrv_get_aio_context(bs),
+                                              blk_get_aio_context(job->blk),
                  true);
     }
     ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
@@ -337,7 +340,7 @@ void block_job_cancel_sync_all(void)
     AioContext *aio_context;
 
     while ((job = QLIST_FIRST(&block_jobs))) {
-        aio_context = bdrv_get_aio_context(job->bs);
+        aio_context = blk_get_aio_context(job->blk);
         aio_context_acquire(aio_context);
         block_job_cancel_sync(job);
         aio_context_release(aio_context);
@@ -362,7 +365,7 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
     if (block_job_is_paused(job)) {
         qemu_coroutine_yield();
     } else {
-        co_aio_sleep_ns(bdrv_get_aio_context(job->bs), type, ns);
+        co_aio_sleep_ns(blk_get_aio_context(job->blk), type, ns);
     }
     job->busy = true;
 }
@@ -491,7 +494,7 @@ static void block_job_defer_to_main_loop_bh(void *opaque)
     aio_context_acquire(data->aio_context);
 
     /* Fetch BDS AioContext again, in case it has changed */
-    aio_context = bdrv_get_aio_context(data->job->bs);
+    aio_context = blk_get_aio_context(data->job->blk);
     aio_context_acquire(aio_context);
 
     data->job->deferred_to_main_loop = false;
@@ -511,7 +514,7 @@ void block_job_defer_to_main_loop(BlockJob *job,
     BlockJobDeferToMainLoopData *data = g_malloc(sizeof(*data));
     data->job = job;
     data->bh = qemu_bh_new(block_job_defer_to_main_loop_bh, data);
-    data->aio_context = bdrv_get_aio_context(job->bs);
+    data->aio_context = blk_get_aio_context(job->blk);
     data->fn = fn;
     data->opaque = opaque;
     job->deferred_to_main_loop = true;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 4ac6831..32012af 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -82,7 +82,8 @@ struct BlockJob {
     const BlockJobDriver *driver;
 
     /** The block device on which the job is operating.  */
-    BlockDriverState *bs;
+    BlockDriverState *bs; /* TODO Remove */
+    BlockBackend *blk;
 
     /**
      * The ID of the block job. Currently the BlockBackend name of the BDS
-- 
1.8.3.1

  parent reply	other threads:[~2016-05-25 17:41 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-25 17:39 [Qemu-devel] [PULL 00/31] Block layer patches Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 01/31] block: Fix bdrv_next() memory leak Kevin Wolf
2016-06-06  8:41   ` Paolo Bonzini
2016-05-25 17:39 ` [Qemu-devel] [PULL 02/31] block: Drop useless bdrv_new() call Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 03/31] block: Let bdrv_open_inherit() return the snapshot Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 04/31] tests: Drop BDS from test-throttle.c Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 05/31] block: Drop blk_new_with_bs() Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 06/31] block: Drop bdrv_new_root() Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 07/31] block: Make bdrv_open() return a BDS Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 08/31] block: Assert !bs->refcnt in bdrv_close() Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 09/31] block: Drop bdrv_parent_cb_...() from bdrv_close() Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 10/31] block: Drop errp parameter from blk_new() Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 11/31] block: Introduce bdrv_replace_child() Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 12/31] block: Make bdrv_drain() use bdrv_drained_begin/end() Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 13/31] block: Fix reconfiguring graph with drained nodes Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 14/31] block: Propagate .drained_begin/end callbacks Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 15/31] dma-helpers: change interface to byte-based Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 16/31] dma-helpers: change BlockBackend to opaque value in DMAIOFunc Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 17/31] block: Rename blk_write_zeroes() Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 18/31] block: keep a list of block jobs Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 19/31] block: Cancel jobs first in bdrv_close_all() Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 20/31] block: Default to enabled write cache in blk_new() Kevin Wolf
2016-05-25 17:39 ` Kevin Wolf [this message]
2016-05-25 17:39 ` [Qemu-devel] [PULL 22/31] block: Make blk_co_preadv/pwritev() public Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 23/31] stream: Use BlockBackend for I/O Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 24/31] mirror: Allow target that already has a BlockBackend Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 25/31] mirror: Use BlockBackend for I/O Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 26/31] backup: Don't leak BackupBlockJob in error path Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 27/31] backup: Pack Notifier within BackupBlockJob Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 28/31] backup: Remove bs parameter from backup_do_cow() Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 29/31] backup: Use BlockBackend for I/O Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 30/31] commit: " Kevin Wolf
2016-05-25 17:39 ` [Qemu-devel] [PULL 31/31] blockjob: Remove BlockJob.bs Kevin Wolf
2016-05-26 14:06 ` [Qemu-devel] [PULL 00/31] Block layer patches Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1464197996-4581-22-git-send-email-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).