On 20/05/2021 17:00, Vladimir Sementsov-Ogievskiy wrote:
18.05.2021 13:07, Emanuele Giuseppe Esposito wrote:
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

this change is unrelated, why not to put it into commit, which adds use of CoMutex in that function?

Ok I will move it in patch 4.



Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com>

[...]

you add an empty line before group, it looks good

+    /* 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;

but not here..

Because these are still State fields, so in the same State group. It is a different sub-category.


+    /* 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;
      /*

Thank you,
Emanuele


Reply via email to