yeahuh / rpms / qemu-kvm

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