yeahuh / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone
26ba25
From 2f7ae3f2886ec34dfb0a51e75d0f62998213928a Mon Sep 17 00:00:00 2001
26ba25
From: Kevin Wolf <kwolf@redhat.com>
26ba25
Date: Tue, 26 Jun 2018 09:48:05 +0200
26ba25
Subject: [PATCH 097/268] job: Add reference counting
26ba25
26ba25
RH-Author: Kevin Wolf <kwolf@redhat.com>
26ba25
Message-id: <20180626094856.6924-23-kwolf@redhat.com>
26ba25
Patchwork-id: 81073
26ba25
O-Subject: [RHV-7.6 qemu-kvm-rhev PATCH v2 22/73] job: Add reference counting
26ba25
Bugzilla: 1513543
26ba25
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
26ba25
RH-Acked-by: Max Reitz <mreitz@redhat.com>
26ba25
RH-Acked-by: Fam Zheng <famz@redhat.com>
26ba25
26ba25
This moves reference counting from BlockJob to Job.
26ba25
26ba25
In order to keep calling the BlockJob cleanup code when the job is
26ba25
deleted via job_unref(), introduce a new JobDriver.free callback. Every
26ba25
block job must use block_job_free() for this callback, this is asserted
26ba25
in block_job_create().
26ba25
26ba25
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
26ba25
Reviewed-by: Max Reitz <mreitz@redhat.com>
26ba25
Reviewed-by: John Snow <jsnow@redhat.com>
26ba25
(cherry picked from commit 80fa2c756b3241f24015a7503a01f7999d4a942d)
26ba25
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
26ba25
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
26ba25
---
26ba25
 block/backup.c               |  1 +
26ba25
 block/commit.c               |  1 +
26ba25
 block/mirror.c               |  2 ++
26ba25
 block/stream.c               |  1 +
26ba25
 blockjob.c                   | 48 +++++++++++++++++++-------------------------
26ba25
 include/block/blockjob.h     | 21 -------------------
26ba25
 include/block/blockjob_int.h |  7 +++++++
26ba25
 include/qemu/job.h           | 19 ++++++++++++++++--
26ba25
 job.c                        | 22 ++++++++++++++++----
26ba25
 qemu-img.c                   |  4 ++--
26ba25
 tests/test-bdrv-drain.c      |  1 +
26ba25
 tests/test-blockjob-txn.c    |  1 +
26ba25
 tests/test-blockjob.c        |  6 ++++--
26ba25
 13 files changed, 76 insertions(+), 58 deletions(-)
26ba25
26ba25
diff --git a/block/backup.c b/block/backup.c
26ba25
index baf8d43..cfdb89d 100644
26ba25
--- a/block/backup.c
26ba25
+++ b/block/backup.c
26ba25
@@ -526,6 +526,7 @@ static const BlockJobDriver backup_job_driver = {
26ba25
     .job_driver = {
26ba25
         .instance_size          = sizeof(BackupBlockJob),
26ba25
         .job_type               = JOB_TYPE_BACKUP,
26ba25
+        .free                   = block_job_free,
26ba25
     },
26ba25
     .start                  = backup_run,
26ba25
     .commit                 = backup_commit,
26ba25
diff --git a/block/commit.c b/block/commit.c
26ba25
index 32d29c8..925c96a 100644
26ba25
--- a/block/commit.c
26ba25
+++ b/block/commit.c
26ba25
@@ -218,6 +218,7 @@ static const BlockJobDriver commit_job_driver = {
26ba25
     .job_driver = {
26ba25
         .instance_size = sizeof(CommitBlockJob),
26ba25
         .job_type      = JOB_TYPE_COMMIT,
26ba25
+        .free          = block_job_free,
26ba25
     },
26ba25
     .start         = commit_run,
26ba25
 };
26ba25
diff --git a/block/mirror.c b/block/mirror.c
26ba25
index 35fcc1f..0df4f70 100644
26ba25
--- a/block/mirror.c
26ba25
+++ b/block/mirror.c
26ba25
@@ -989,6 +989,7 @@ static const BlockJobDriver mirror_job_driver = {
26ba25
     .job_driver = {
26ba25
         .instance_size          = sizeof(MirrorBlockJob),
26ba25
         .job_type               = JOB_TYPE_MIRROR,
26ba25
+        .free                   = block_job_free,
26ba25
     },
26ba25
     .start                  = mirror_run,
26ba25
     .complete               = mirror_complete,
26ba25
@@ -1001,6 +1002,7 @@ static const BlockJobDriver commit_active_job_driver = {
26ba25
     .job_driver = {
26ba25
         .instance_size          = sizeof(MirrorBlockJob),
26ba25
         .job_type               = JOB_TYPE_COMMIT,
26ba25
+        .free                   = block_job_free,
26ba25
     },
26ba25
     .start                  = mirror_run,
26ba25
     .complete               = mirror_complete,
26ba25
diff --git a/block/stream.c b/block/stream.c
26ba25
index cb723f1..7273d22 100644
26ba25
--- a/block/stream.c
26ba25
+++ b/block/stream.c
26ba25
@@ -212,6 +212,7 @@ static const BlockJobDriver stream_job_driver = {
26ba25
     .job_driver = {
26ba25
         .instance_size = sizeof(StreamBlockJob),
26ba25
         .job_type      = JOB_TYPE_STREAM,
26ba25
+        .free          = block_job_free,
26ba25
     },
26ba25
     .start         = stream_run,
26ba25
 };
26ba25
diff --git a/blockjob.c b/blockjob.c
26ba25
index 0fba01e..0bf0a26 100644
26ba25
--- a/blockjob.c
26ba25
+++ b/blockjob.c
26ba25
@@ -190,31 +190,25 @@ static void block_job_resume(BlockJob *job)
26ba25
     block_job_enter_cond(job, block_job_timer_not_pending);
26ba25
 }
26ba25
 
26ba25
-void block_job_ref(BlockJob *job)
26ba25
-{
26ba25
-    ++job->refcnt;
26ba25
-}
26ba25
-
26ba25
 static void block_job_attached_aio_context(AioContext *new_context,
26ba25
                                            void *opaque);
26ba25
 static void block_job_detach_aio_context(void *opaque);
26ba25
 
26ba25
-void block_job_unref(BlockJob *job)
26ba25
+void block_job_free(Job *job)
26ba25
 {
26ba25
-    if (--job->refcnt == 0) {
26ba25
-        assert(job->job.status == JOB_STATUS_NULL);
26ba25
-        assert(!job->txn);
26ba25
-        BlockDriverState *bs = blk_bs(job->blk);
26ba25
-        bs->job = NULL;
26ba25
-        block_job_remove_all_bdrv(job);
26ba25
-        blk_remove_aio_context_notifier(job->blk,
26ba25
-                                        block_job_attached_aio_context,
26ba25
-                                        block_job_detach_aio_context, job);
26ba25
-        blk_unref(job->blk);
26ba25
-        error_free(job->blocker);
26ba25
-        assert(!timer_pending(&job->sleep_timer));
26ba25
-        job_delete(&job->job);
26ba25
-    }
26ba25
+    BlockJob *bjob = container_of(job, BlockJob, job);
26ba25
+    BlockDriverState *bs = blk_bs(bjob->blk);
26ba25
+
26ba25
+    assert(!bjob->txn);
26ba25
+
26ba25
+    bs->job = NULL;
26ba25
+    block_job_remove_all_bdrv(bjob);
26ba25
+    blk_remove_aio_context_notifier(bjob->blk,
26ba25
+                                    block_job_attached_aio_context,
26ba25
+                                    block_job_detach_aio_context, bjob);
26ba25
+    blk_unref(bjob->blk);
26ba25
+    error_free(bjob->blocker);
26ba25
+    assert(!timer_pending(&bjob->sleep_timer));
26ba25
 }
26ba25
 
26ba25
 static void block_job_attached_aio_context(AioContext *new_context,
26ba25
@@ -245,7 +239,7 @@ static void block_job_detach_aio_context(void *opaque)
26ba25
     BlockJob *job = opaque;
26ba25
 
26ba25
     /* In case the job terminates during aio_poll()... */
26ba25
-    block_job_ref(job);
26ba25
+    job_ref(&job->job);
26ba25
 
26ba25
     block_job_pause(job);
26ba25
 
26ba25
@@ -253,7 +247,7 @@ static void block_job_detach_aio_context(void *opaque)
26ba25
         block_job_drain(job);
26ba25
     }
26ba25
 
26ba25
-    block_job_unref(job);
26ba25
+    job_unref(&job->job);
26ba25
 }
26ba25
 
26ba25
 static char *child_job_get_parent_desc(BdrvChild *c)
26ba25
@@ -367,7 +361,7 @@ static void block_job_decommission(BlockJob *job)
26ba25
     job->deferred_to_main_loop = true;
26ba25
     block_job_txn_del_job(job);
26ba25
     job_state_transition(&job->job, JOB_STATUS_NULL);
26ba25
-    block_job_unref(job);
26ba25
+    job_unref(&job->job);
26ba25
 }
26ba25
 
26ba25
 static void block_job_do_dismiss(BlockJob *job)
26ba25
@@ -506,14 +500,14 @@ static int block_job_finish_sync(BlockJob *job,
26ba25
 
26ba25
     assert(blk_bs(job->blk)->job == job);
26ba25
 
26ba25
-    block_job_ref(job);
26ba25
+    job_ref(&job->job);
26ba25
 
26ba25
     if (finish) {
26ba25
         finish(job, &local_err);
26ba25
     }
26ba25
     if (local_err) {
26ba25
         error_propagate(errp, local_err);
26ba25
-        block_job_unref(job);
26ba25
+        job_unref(&job->job);
26ba25
         return -EBUSY;
26ba25
     }
26ba25
     /* block_job_drain calls block_job_enter, and it should be enough to
26ba25
@@ -526,7 +520,7 @@ static int block_job_finish_sync(BlockJob *job,
26ba25
         aio_poll(qemu_get_aio_context(), true);
26ba25
     }
26ba25
     ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
26ba25
-    block_job_unref(job);
26ba25
+    job_unref(&job->job);
26ba25
     return ret;
26ba25
 }
26ba25
 
26ba25
@@ -909,6 +903,7 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
26ba25
     }
26ba25
 
26ba25
     assert(is_block_job(&job->job));
26ba25
+    assert(job->job.driver->free == &block_job_free);
26ba25
 
26ba25
     job->driver        = driver;
26ba25
     job->blk           = blk;
26ba25
@@ -917,7 +912,6 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
26ba25
     job->busy          = false;
26ba25
     job->paused        = true;
26ba25
     job->pause_count   = 1;
26ba25
-    job->refcnt        = 1;
26ba25
     job->auto_finalize = !(flags & BLOCK_JOB_MANUAL_FINALIZE);
26ba25
     job->auto_dismiss  = !(flags & BLOCK_JOB_MANUAL_DISMISS);
26ba25
     aio_timer_init(qemu_get_aio_context(), &job->sleep_timer,
26ba25
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
26ba25
index 01cdee6..087e782 100644
26ba25
--- a/include/block/blockjob.h
26ba25
+++ b/include/block/blockjob.h
26ba25
@@ -132,9 +132,6 @@ typedef struct BlockJob {
26ba25
     /** The opaque value that is passed to the completion function.  */
26ba25
     void *opaque;
26ba25
 
26ba25
-    /** Reference count of the block job */
26ba25
-    int refcnt;
26ba25
-
26ba25
     /** True when job has reported completion by calling block_job_completed. */
26ba25
     bool completed;
26ba25
 
26ba25
@@ -400,24 +397,6 @@ void block_job_iostatus_reset(BlockJob *job);
26ba25
 BlockJobTxn *block_job_txn_new(void);
26ba25
 
26ba25
 /**
26ba25
- * block_job_ref:
26ba25
- *
26ba25
- * Add a reference to BlockJob refcnt, it will be decreased with
26ba25
- * block_job_unref, and then be freed if it comes to be the last
26ba25
- * reference.
26ba25
- */
26ba25
-void block_job_ref(BlockJob *job);
26ba25
-
26ba25
-/**
26ba25
- * block_job_unref:
26ba25
- *
26ba25
- * Release a reference that was previously acquired with block_job_ref
26ba25
- * or block_job_create. If it's the last reference to the object, it will be
26ba25
- * freed.
26ba25
- */
26ba25
-void block_job_unref(BlockJob *job);
26ba25
-
26ba25
-/**
26ba25
  * block_job_txn_unref:
26ba25
  *
26ba25
  * Release a reference that was previously acquired with block_job_txn_add_job
26ba25
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
26ba25
index 1e62d6d..6f0fe3c 100644
26ba25
--- a/include/block/blockjob_int.h
26ba25
+++ b/include/block/blockjob_int.h
26ba25
@@ -144,6 +144,13 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
26ba25
                        BlockCompletionFunc *cb, void *opaque, Error **errp);
26ba25
 
26ba25
 /**
26ba25
+ * block_job_free:
26ba25
+ * Callback to be used for JobDriver.free in all block jobs. Frees block job
26ba25
+ * specific resources in @job.
26ba25
+ */
26ba25
+void block_job_free(Job *job);
26ba25
+
26ba25
+/**
26ba25
  * block_job_sleep_ns:
26ba25
  * @job: The job that calls the function.
26ba25
  * @ns: How many nanoseconds to stop for.
26ba25
diff --git a/include/qemu/job.h b/include/qemu/job.h
26ba25
index 0b78778..0751e2a 100644
26ba25
--- a/include/qemu/job.h
26ba25
+++ b/include/qemu/job.h
26ba25
@@ -41,6 +41,9 @@ typedef struct Job {
26ba25
     /** The type of this job. */
26ba25
     const JobDriver *driver;
26ba25
 
26ba25
+    /** Reference count of the block job */
26ba25
+    int refcnt;
26ba25
+
26ba25
     /** Current state; See @JobStatus for details. */
26ba25
     JobStatus status;
26ba25
 
26ba25
@@ -57,6 +60,9 @@ struct JobDriver {
26ba25
 
26ba25
     /** Enum describing the operation */
26ba25
     JobType job_type;
26ba25
+
26ba25
+    /** Called when the job is freed */
26ba25
+    void (*free)(Job *job);
26ba25
 };
26ba25
 
26ba25
 
26ba25
@@ -69,8 +75,17 @@ struct JobDriver {
26ba25
  */
26ba25
 void *job_create(const char *job_id, const JobDriver *driver, Error **errp);
26ba25
 
26ba25
-/** Frees the @job object. */
26ba25
-void job_delete(Job *job);
26ba25
+/**
26ba25
+ * Add a reference to Job refcnt, it will be decreased with job_unref, and then
26ba25
+ * be freed if it comes to be the last reference.
26ba25
+ */
26ba25
+void job_ref(Job *job);
26ba25
+
26ba25
+/**
26ba25
+ * Release a reference that was previously acquired with job_ref() or
26ba25
+ * job_create(). If it's the last reference to the object, it will be freed.
26ba25
+ */
26ba25
+void job_unref(Job *job);
26ba25
 
26ba25
 /** Returns the JobType of a given Job. */
26ba25
 JobType job_type(const Job *job);
26ba25
diff --git a/job.c b/job.c
26ba25
index b049a32..926f1de 100644
26ba25
--- a/job.c
26ba25
+++ b/job.c
26ba25
@@ -134,6 +134,7 @@ void *job_create(const char *job_id, const JobDriver *driver, Error **errp)
26ba25
     job = g_malloc0(driver->instance_size);
26ba25
     job->driver        = driver;
26ba25
     job->id            = g_strdup(job_id);
26ba25
+    job->refcnt        = 1;
26ba25
 
26ba25
     job_state_transition(job, JOB_STATUS_CREATED);
26ba25
 
26ba25
@@ -142,10 +143,23 @@ void *job_create(const char *job_id, const JobDriver *driver, Error **errp)
26ba25
     return job;
26ba25
 }
26ba25
 
26ba25
-void job_delete(Job *job)
26ba25
+void job_ref(Job *job)
26ba25
 {
26ba25
-    QLIST_REMOVE(job, job_list);
26ba25
+    ++job->refcnt;
26ba25
+}
26ba25
+
26ba25
+void job_unref(Job *job)
26ba25
+{
26ba25
+    if (--job->refcnt == 0) {
26ba25
+        assert(job->status == JOB_STATUS_NULL);
26ba25
 
26ba25
-    g_free(job->id);
26ba25
-    g_free(job);
26ba25
+        if (job->driver->free) {
26ba25
+            job->driver->free(job);
26ba25
+        }
26ba25
+
26ba25
+        QLIST_REMOVE(job, job_list);
26ba25
+
26ba25
+        g_free(job->id);
26ba25
+        g_free(job);
26ba25
+    }
26ba25
 }
26ba25
diff --git a/qemu-img.c b/qemu-img.c
26ba25
index e2395b9..f745919 100644
26ba25
--- a/qemu-img.c
26ba25
+++ b/qemu-img.c
26ba25
@@ -873,7 +873,7 @@ static void run_block_job(BlockJob *job, Error **errp)
26ba25
     int ret = 0;
26ba25
 
26ba25
     aio_context_acquire(aio_context);
26ba25
-    block_job_ref(job);
26ba25
+    job_ref(&job->job);
26ba25
     do {
26ba25
         aio_poll(aio_context, true);
26ba25
         qemu_progress_print(job->len ?
26ba25
@@ -885,7 +885,7 @@ static void run_block_job(BlockJob *job, Error **errp)
26ba25
     } else {
26ba25
         ret = job->ret;
26ba25
     }
26ba25
-    block_job_unref(job);
26ba25
+    job_unref(&job->job);
26ba25
     aio_context_release(aio_context);
26ba25
 
26ba25
     /* publish completion progress only when success */
26ba25
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
26ba25
index fe9f412..f9e37d4 100644
26ba25
--- a/tests/test-bdrv-drain.c
26ba25
+++ b/tests/test-bdrv-drain.c
26ba25
@@ -522,6 +522,7 @@ static void test_job_complete(BlockJob *job, Error **errp)
26ba25
 BlockJobDriver test_job_driver = {
26ba25
     .job_driver = {
26ba25
         .instance_size  = sizeof(TestBlockJob),
26ba25
+        .free           = block_job_free,
26ba25
     },
26ba25
     .start          = test_job_start,
26ba25
     .complete       = test_job_complete,
26ba25
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
26ba25
index 48b12d1..b49b28c 100644
26ba25
--- a/tests/test-blockjob-txn.c
26ba25
+++ b/tests/test-blockjob-txn.c
26ba25
@@ -76,6 +76,7 @@ static void test_block_job_cb(void *opaque, int ret)
26ba25
 static const BlockJobDriver test_block_job_driver = {
26ba25
     .job_driver = {
26ba25
         .instance_size = sizeof(TestBlockJob),
26ba25
+        .free          = block_job_free,
26ba25
     },
26ba25
     .start = test_block_job_run,
26ba25
 };
26ba25
diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
26ba25
index 6ccd585..e24fc3f 100644
26ba25
--- a/tests/test-blockjob.c
26ba25
+++ b/tests/test-blockjob.c
26ba25
@@ -19,6 +19,7 @@
26ba25
 static const BlockJobDriver test_block_job_driver = {
26ba25
     .job_driver = {
26ba25
         .instance_size = sizeof(BlockJob),
26ba25
+        .free          = block_job_free,
26ba25
     },
26ba25
 };
26ba25
 
26ba25
@@ -196,6 +197,7 @@ static void coroutine_fn cancel_job_start(void *opaque)
26ba25
 static const BlockJobDriver test_cancel_driver = {
26ba25
     .job_driver = {
26ba25
         .instance_size = sizeof(CancelJob),
26ba25
+        .free          = block_job_free,
26ba25
     },
26ba25
     .start         = cancel_job_start,
26ba25
     .complete      = cancel_job_complete,
26ba25
@@ -210,7 +212,7 @@ static CancelJob *create_common(BlockJob **pjob)
26ba25
     blk = create_blk(NULL);
26ba25
     job = mk_job(blk, "Steve", &test_cancel_driver, true,
26ba25
                  BLOCK_JOB_MANUAL_FINALIZE | BLOCK_JOB_MANUAL_DISMISS);
26ba25
-    block_job_ref(job);
26ba25
+    job_ref(&job->job);
26ba25
     assert(job->job.status == JOB_STATUS_CREATED);
26ba25
     s = container_of(job, CancelJob, common);
26ba25
     s->blk = blk;
26ba25
@@ -231,7 +233,7 @@ static void cancel_common(CancelJob *s)
26ba25
         block_job_dismiss(&dummy, &error_abort);
26ba25
     }
26ba25
     assert(job->job.status == JOB_STATUS_NULL);
26ba25
-    block_job_unref(job);
26ba25
+    job_unref(&job->job);
26ba25
     destroy_blk(blk);
26ba25
 }
26ba25
 
26ba25
-- 
26ba25
1.8.3.1
26ba25