As done in BlockCopyCallState, categorize BlockCopyTask and BlockCopyState in IN, State and OUT fields. This is just to understand which field has to be protected with a lock.
BlockCopyTask .zeroes is a special case, because it is only initialized and then read by the coroutine in block_copy_task_entry. Also set block_copy_task_create as coroutine_fn because: 1) it is static and only invoked by coroutine functions 2) next patches will introduce and use a CoMutex lock there Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com> --- block/block-copy.c | 49 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index 10ce51a244..d2d3839dec 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -67,13 +67,28 @@ typedef struct BlockCopyCallState { typedef struct BlockCopyTask { AioTask task; + /* + * IN parameters. Initialized in block_copy_task_create() + * and never changed. + */ BlockCopyState *s; BlockCopyCallState *call_state; int64_t offset; - int64_t bytes; + int64_t bytes; /* only re-set in task_shrink, before running the task */ + + /* + * "local" parameter: used only to communicate between + * the caller (block_copy_dirty_clusters) and the aiotask + * coroutine running block_copy_task_entry + */ bool zeroes; - QLIST_ENTRY(BlockCopyTask) list; + + /* State */ CoQueue wait_queue; /* coroutines blocked on this task */ + + /* To reference all call states from BlockCopyState */ + QLIST_ENTRY(BlockCopyTask) list; + } BlockCopyTask; static int64_t task_end(BlockCopyTask *task) @@ -89,15 +104,25 @@ typedef struct BlockCopyState { */ BdrvChild *source; BdrvChild *target; - BdrvDirtyBitmap *copy_bitmap; + + /* State */ int64_t in_flight_bytes; - int64_t cluster_size; BlockCopyMethod method; - int64_t max_transfer; - uint64_t len; QLIST_HEAD(, BlockCopyTask) tasks; /* All tasks from all block-copy calls */ QLIST_HEAD(, BlockCopyCallState) calls; + /* State fields that use a thread-safe API */ + BdrvDirtyBitmap *copy_bitmap; + ProgressMeter *progress; + SharedResource *mem; + RateLimit rate_limit; + /* + * IN parameters. Initialized in block_copy_state_new() + * and never changed. + */ + int64_t cluster_size; + int64_t max_transfer; + uint64_t len; BdrvRequestFlags write_flags; /* @@ -115,12 +140,6 @@ typedef struct BlockCopyState { * block_copy_reset_unallocated() every time it does. */ bool skip_unallocated; - - ProgressMeter *progress; - - SharedResource *mem; - - RateLimit rate_limit; } BlockCopyState; static BlockCopyTask *find_conflicting_task(BlockCopyState *s, @@ -176,9 +195,9 @@ static inline int64_t block_copy_chunk_size(BlockCopyState *s) * Search for the first dirty area in offset/bytes range and create task at * the beginning of it. */ -static BlockCopyTask *block_copy_task_create(BlockCopyState *s, - BlockCopyCallState *call_state, - int64_t offset, int64_t bytes) +static coroutine_fn BlockCopyTask *block_copy_task_create(BlockCopyState *s, + BlockCopyCallState *call_state, + int64_t offset, int64_t bytes) { BlockCopyTask *task; int64_t max_chunk = block_copy_chunk_size(s); -- 2.30.2