On Tue 21 Nov 2017 04:18:13 PM CET, Anton Nefedov wrote:
>  >> keep BlockJob referenced while it is
>  >> paused (by block_job_pause/resume_all()). That should prevent it from
>  >> deleting the BB.
>
> looks kind of hacky; maybe referencing in block_job_pause() (and not
> just pause_all) seems more correct? I think it didn't work for me
> right away though. But I can look more.

This fixes one crash for me (but only the test_stream_commit, it doesn't
fix iotest 030 completely), but I agree it looks kind of hacky.

Peharps replacing block_job_next() with QLIST_FOREACH_SAFE() is a good
idea, though, I guess resuming a block job can theoretically lead to its
destruction?

Berto

diff --git a/blockjob.c b/blockjob.c
index ff9a614531..53cffc7980 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -699,6 +699,7 @@ void block_job_pause_all(void)
         AioContext *aio_context = blk_get_aio_context(job->blk);
 
         aio_context_acquire(aio_context);
+        block_job_ref(job);
         block_job_pause(job);
         aio_context_release(aio_context);
     }
@@ -759,12 +760,14 @@ void coroutine_fn block_job_pause_point(BlockJob *job)
 
 void block_job_resume_all(void)
 {
-    BlockJob *job = NULL;
-    while ((job = block_job_next(job))) {
+    BlockJob *job, *next;
+
+    QLIST_FOREACH_SAFE(job, &block_jobs, job_list, next) {
         AioContext *aio_context = blk_get_aio_context(job->blk);
 
         aio_context_acquire(aio_context);
         block_job_resume(job);
+        block_job_unref(job);
         aio_context_release(aio_context);
     }
 }


Reply via email to