yeahuh / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone
ae23c9
From 98c027dc5d49edb499749092d647edd620ca7d68 Mon Sep 17 00:00:00 2001
ae23c9
From: John Snow <jsnow@redhat.com>
ae23c9
Date: Tue, 25 Sep 2018 22:34:12 +0100
ae23c9
Subject: [PATCH 09/28] jobs: utilize job_exit shim
ae23c9
ae23c9
RH-Author: John Snow <jsnow@redhat.com>
ae23c9
Message-id: <20180925223431.24791-7-jsnow@redhat.com>
ae23c9
Patchwork-id: 82267
ae23c9
O-Subject: [RHEL8/rhel qemu-kvm PATCH 06/25] jobs: utilize job_exit shim
ae23c9
Bugzilla: 1632939
ae23c9
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
ae23c9
RH-Acked-by: Max Reitz <mreitz@redhat.com>
ae23c9
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
ae23c9
ae23c9
Utilize the job_exit shim by not calling job_defer_to_main_loop, and
ae23c9
where applicable, converting the deferred callback into the job_exit
ae23c9
callback.
ae23c9
ae23c9
This converts backup, stream, create, and the unit tests all at once.
ae23c9
Most of these jobs do not see any changes to the order in which they
ae23c9
clean up their resources, except the test-blockjob-txn test, which
ae23c9
now puts down its bs before job_completed is called.
ae23c9
ae23c9
This is safe for the same reason the reordering in the mirror job is
ae23c9
safe, because job_completed no longer runs under two locks, making
ae23c9
the unref safe even if it causes a flush.
ae23c9
ae23c9
Signed-off-by: John Snow <jsnow@redhat.com>
ae23c9
Reviewed-by: Max Reitz <mreitz@redhat.com>
ae23c9
Message-id: 20180830015734.19765-7-jsnow@redhat.com
ae23c9
Signed-off-by: Max Reitz <mreitz@redhat.com>
ae23c9
(cherry picked from commit eb23654dbe43b549ea2a9ebff9d8edf544d34a73)
ae23c9
Signed-off-by: John Snow <jsnow@redhat.com>
ae23c9
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
ae23c9
---
ae23c9
 block/backup.c            | 16 ----------------
ae23c9
 block/create.c            | 14 +++-----------
ae23c9
 block/stream.c            | 22 +++++++---------------
ae23c9
 tests/test-bdrv-drain.c   |  6 ------
ae23c9
 tests/test-blockjob-txn.c | 11 ++---------
ae23c9
 tests/test-blockjob.c     | 10 ++++------
ae23c9
 6 files changed, 16 insertions(+), 63 deletions(-)
ae23c9
ae23c9
diff --git a/block/backup.c b/block/backup.c
ae23c9
index a5bf699..08a5b74 100644
ae23c9
--- a/block/backup.c
ae23c9
+++ b/block/backup.c
ae23c9
@@ -380,18 +380,6 @@ static BlockErrorAction backup_error_action(BackupBlockJob *job,
ae23c9
     }
ae23c9
 }
ae23c9
 
ae23c9
-typedef struct {
ae23c9
-    int ret;
ae23c9
-} BackupCompleteData;
ae23c9
-
ae23c9
-static void backup_complete(Job *job, void *opaque)
ae23c9
-{
ae23c9
-    BackupCompleteData *data = opaque;
ae23c9
-
ae23c9
-    job_completed(job, data->ret);
ae23c9
-    g_free(data);
ae23c9
-}
ae23c9
-
ae23c9
 static bool coroutine_fn yield_and_check(BackupBlockJob *job)
ae23c9
 {
ae23c9
     uint64_t delay_ns;
ae23c9
@@ -483,7 +471,6 @@ static void backup_incremental_init_copy_bitmap(BackupBlockJob *job)
ae23c9
 static int coroutine_fn backup_run(Job *opaque_job, Error **errp)
ae23c9
 {
ae23c9
     BackupBlockJob *job = container_of(opaque_job, BackupBlockJob, common.job);
ae23c9
-    BackupCompleteData *data;
ae23c9
     BlockDriverState *bs = blk_bs(job->common.blk);
ae23c9
     int64_t offset, nb_clusters;
ae23c9
     int ret = 0;
ae23c9
@@ -584,9 +571,6 @@ static int coroutine_fn backup_run(Job *opaque_job, Error **errp)
ae23c9
     qemu_co_rwlock_unlock(&job->flush_rwlock);
ae23c9
     hbitmap_free(job->copy_bitmap);
ae23c9
 
ae23c9
-    data = g_malloc(sizeof(*data));
ae23c9
-    data->ret = ret;
ae23c9
-    job_defer_to_main_loop(&job->common.job, backup_complete, data);
ae23c9
     return ret;
ae23c9
 }
ae23c9
 
ae23c9
diff --git a/block/create.c b/block/create.c
ae23c9
index 26a385c..9534121 100644
ae23c9
--- a/block/create.c
ae23c9
+++ b/block/create.c
ae23c9
@@ -34,28 +34,20 @@ typedef struct BlockdevCreateJob {
ae23c9
     Job common;
ae23c9
     BlockDriver *drv;
ae23c9
     BlockdevCreateOptions *opts;
ae23c9
-    int ret;
ae23c9
 } BlockdevCreateJob;
ae23c9
 
ae23c9
-static void blockdev_create_complete(Job *job, void *opaque)
ae23c9
-{
ae23c9
-    BlockdevCreateJob *s = container_of(job, BlockdevCreateJob, common);
ae23c9
-
ae23c9
-    job_completed(job, s->ret);
ae23c9
-}
ae23c9
-
ae23c9
 static int coroutine_fn blockdev_create_run(Job *job, Error **errp)
ae23c9
 {
ae23c9
     BlockdevCreateJob *s = container_of(job, BlockdevCreateJob, common);
ae23c9
+    int ret;
ae23c9
 
ae23c9
     job_progress_set_remaining(&s->common, 1);
ae23c9
-    s->ret = s->drv->bdrv_co_create(s->opts, errp);
ae23c9
+    ret = s->drv->bdrv_co_create(s->opts, errp);
ae23c9
     job_progress_update(&s->common, 1);
ae23c9
 
ae23c9
     qapi_free_BlockdevCreateOptions(s->opts);
ae23c9
-    job_defer_to_main_loop(&s->common, blockdev_create_complete, NULL);
ae23c9
 
ae23c9
-    return s->ret;
ae23c9
+    return ret;
ae23c9
 }
ae23c9
 
ae23c9
 static const JobDriver blockdev_create_job_driver = {
ae23c9
diff --git a/block/stream.c b/block/stream.c
ae23c9
index 26a7753..67e1e72 100644
ae23c9
--- a/block/stream.c
ae23c9
+++ b/block/stream.c
ae23c9
@@ -54,20 +54,16 @@ static int coroutine_fn stream_populate(BlockBackend *blk,
ae23c9
     return blk_co_preadv(blk, offset, qiov.size, &qiov, BDRV_REQ_COPY_ON_READ);
ae23c9
 }
ae23c9
 
ae23c9
-typedef struct {
ae23c9
-    int ret;
ae23c9
-} StreamCompleteData;
ae23c9
-
ae23c9
-static void stream_complete(Job *job, void *opaque)
ae23c9
+static void stream_exit(Job *job)
ae23c9
 {
ae23c9
     StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
ae23c9
     BlockJob *bjob = &s->common;
ae23c9
-    StreamCompleteData *data = opaque;
ae23c9
     BlockDriverState *bs = blk_bs(bjob->blk);
ae23c9
     BlockDriverState *base = s->base;
ae23c9
     Error *local_err = NULL;
ae23c9
+    int ret = job->ret;
ae23c9
 
ae23c9
-    if (!job_is_cancelled(job) && bs->backing && data->ret == 0) {
ae23c9
+    if (!job_is_cancelled(job) && bs->backing && ret == 0) {
ae23c9
         const char *base_id = NULL, *base_fmt = NULL;
ae23c9
         if (base) {
ae23c9
             base_id = s->backing_file_str;
ae23c9
@@ -75,11 +71,11 @@ static void stream_complete(Job *job, void *opaque)
ae23c9
                 base_fmt = base->drv->format_name;
ae23c9
             }
ae23c9
         }
ae23c9
-        data->ret = bdrv_change_backing_file(bs, base_id, base_fmt);
ae23c9
+        ret = bdrv_change_backing_file(bs, base_id, base_fmt);
ae23c9
         bdrv_set_backing_hd(bs, base, &local_err);
ae23c9
         if (local_err) {
ae23c9
             error_report_err(local_err);
ae23c9
-            data->ret = -EPERM;
ae23c9
+            ret = -EPERM;
ae23c9
             goto out;
ae23c9
         }
ae23c9
     }
ae23c9
@@ -93,14 +89,12 @@ out:
ae23c9
     }
ae23c9
 
ae23c9
     g_free(s->backing_file_str);
ae23c9
-    job_completed(job, data->ret);
ae23c9
-    g_free(data);
ae23c9
+    job->ret = ret;
ae23c9
 }
ae23c9
 
ae23c9
 static int coroutine_fn stream_run(Job *job, Error **errp)
ae23c9
 {
ae23c9
     StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
ae23c9
-    StreamCompleteData *data;
ae23c9
     BlockBackend *blk = s->common.blk;
ae23c9
     BlockDriverState *bs = blk_bs(blk);
ae23c9
     BlockDriverState *base = s->base;
ae23c9
@@ -203,9 +197,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
ae23c9
 
ae23c9
 out:
ae23c9
     /* Modify backing chain and close BDSes in main loop */
ae23c9
-    data = g_malloc(sizeof(*data));
ae23c9
-    data->ret = ret;
ae23c9
-    job_defer_to_main_loop(&s->common.job, stream_complete, data);
ae23c9
     return ret;
ae23c9
 }
ae23c9
 
ae23c9
@@ -215,6 +206,7 @@ static const BlockJobDriver stream_job_driver = {
ae23c9
         .job_type      = JOB_TYPE_STREAM,
ae23c9
         .free          = block_job_free,
ae23c9
         .run           = stream_run,
ae23c9
+        .exit          = stream_exit,
ae23c9
         .user_resume   = block_job_user_resume,
ae23c9
         .drain         = block_job_drain,
ae23c9
     },
ae23c9
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
ae23c9
index 8937894..403446e 100644
ae23c9
--- a/tests/test-bdrv-drain.c
ae23c9
+++ b/tests/test-bdrv-drain.c
ae23c9
@@ -496,11 +496,6 @@ typedef struct TestBlockJob {
ae23c9
     bool should_complete;
ae23c9
 } TestBlockJob;
ae23c9
 
ae23c9
-static void test_job_completed(Job *job, void *opaque)
ae23c9
-{
ae23c9
-    job_completed(job, 0);
ae23c9
-}
ae23c9
-
ae23c9
 static int coroutine_fn test_job_run(Job *job, Error **errp)
ae23c9
 {
ae23c9
     TestBlockJob *s = container_of(job, TestBlockJob, common.job);
ae23c9
@@ -510,7 +505,6 @@ static int coroutine_fn test_job_run(Job *job, Error **errp)
ae23c9
         job_sleep_ns(&s->common.job, 100000);
ae23c9
     }
ae23c9
 
ae23c9
-    job_defer_to_main_loop(&s->common.job, test_job_completed, NULL);
ae23c9
     return 0;
ae23c9
 }
ae23c9
 
ae23c9
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
ae23c9
index 82cedee..ef29f35 100644
ae23c9
--- a/tests/test-blockjob-txn.c
ae23c9
+++ b/tests/test-blockjob-txn.c
ae23c9
@@ -24,17 +24,11 @@ typedef struct {
ae23c9
     int *result;
ae23c9
 } TestBlockJob;
ae23c9
 
ae23c9
-static void test_block_job_complete(Job *job, void *opaque)
ae23c9
+static void test_block_job_exit(Job *job)
ae23c9
 {
ae23c9
     BlockJob *bjob = container_of(job, BlockJob, job);
ae23c9
     BlockDriverState *bs = blk_bs(bjob->blk);
ae23c9
-    int rc = (intptr_t)opaque;
ae23c9
 
ae23c9
-    if (job_is_cancelled(job)) {
ae23c9
-        rc = -ECANCELED;
ae23c9
-    }
ae23c9
-
ae23c9
-    job_completed(job, rc);
ae23c9
     bdrv_unref(bs);
ae23c9
 }
ae23c9
 
ae23c9
@@ -54,8 +48,6 @@ static int coroutine_fn test_block_job_run(Job *job, Error **errp)
ae23c9
         }
ae23c9
     }
ae23c9
 
ae23c9
-    job_defer_to_main_loop(job, test_block_job_complete,
ae23c9
-                           (void *)(intptr_t)s->rc);
ae23c9
     return s->rc;
ae23c9
 }
ae23c9
 
ae23c9
@@ -81,6 +73,7 @@ static const BlockJobDriver test_block_job_driver = {
ae23c9
         .user_resume   = block_job_user_resume,
ae23c9
         .drain         = block_job_drain,
ae23c9
         .run           = test_block_job_run,
ae23c9
+        .exit          = test_block_job_exit,
ae23c9
     },
ae23c9
 };
ae23c9
 
ae23c9
diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
ae23c9
index 408a226..ad4a65b 100644
ae23c9
--- a/tests/test-blockjob.c
ae23c9
+++ b/tests/test-blockjob.c
ae23c9
@@ -163,11 +163,10 @@ typedef struct CancelJob {
ae23c9
     bool completed;
ae23c9
 } CancelJob;
ae23c9
 
ae23c9
-static void cancel_job_completed(Job *job, void *opaque)
ae23c9
+static void cancel_job_exit(Job *job)
ae23c9
 {
ae23c9
-    CancelJob *s = opaque;
ae23c9
+    CancelJob *s = container_of(job, CancelJob, common.job);
ae23c9
     s->completed = true;
ae23c9
-    job_completed(job, 0);
ae23c9
 }
ae23c9
 
ae23c9
 static void cancel_job_complete(Job *job, Error **errp)
ae23c9
@@ -182,7 +181,7 @@ static int coroutine_fn cancel_job_run(Job *job, Error **errp)
ae23c9
 
ae23c9
     while (!s->should_complete) {
ae23c9
         if (job_is_cancelled(&s->common.job)) {
ae23c9
-            goto defer;
ae23c9
+            return 0;
ae23c9
         }
ae23c9
 
ae23c9
         if (!job_is_ready(&s->common.job) && s->should_converge) {
ae23c9
@@ -192,8 +191,6 @@ static int coroutine_fn cancel_job_run(Job *job, Error **errp)
ae23c9
         job_sleep_ns(&s->common.job, 100000);
ae23c9
     }
ae23c9
 
ae23c9
- defer:
ae23c9
-    job_defer_to_main_loop(&s->common.job, cancel_job_completed, s);
ae23c9
     return 0;
ae23c9
 }
ae23c9
 
ae23c9
@@ -204,6 +201,7 @@ static const BlockJobDriver test_cancel_driver = {
ae23c9
         .user_resume   = block_job_user_resume,
ae23c9
         .drain         = block_job_drain,
ae23c9
         .run           = cancel_job_run,
ae23c9
+        .exit          = cancel_job_exit,
ae23c9
         .complete      = cancel_job_complete,
ae23c9
     },
ae23c9
 };
ae23c9
-- 
ae23c9
1.8.3.1
ae23c9