On Fri, Nov 18, 2022 at 4:01 PM Emanuele Giuseppe Esposito <eespo...@redhat.com> wrote: > > - generated_co_wrapper_simple -> coroutine_wrapper > > - generated_co_wrapper_blk -> coroutine_wrapper_mixed > > - generated_co_wrapper -> coroutine_wrapper_mixed_bdrv > > > > ? It is not clear to me yet if you will have bdrv_* functions that take > > the rdlock as well - in which case however coroutine_wrapper_bdrv would > > not be hard to add. > > Why _mixed? I thought _blk was a nice name to identify only the blk_* > API that have this slightly different behavior (ie do not take the > rwlock anywhere).
_mixed means that they are callable from both coroutine and non-coroutine function, but (unlike "normal" functions) they will suspend if called in coroutine context. In fact we could also have a coroutine_mixed_fn static analysis annotation for functions that *call* coroutine_wrapper_mixed (so ultimately they can suspend when called from coroutine context, e.g. vhdx_log_write_and_flush or qcow2's update_refcount): - coroutine_fn can call coroutine_mixed_fn if needed, but cannot call any coroutine_wrapper* - coroutine_mixed_fn can call coroutine_mixed_fn or coroutine_wrapper_mixed{,_bdrv}, but cannot call coroutine_wrapper This of course is completely independent of your series, but since the naming is adjacent it would be nice to only change it once. > No, I don't think there will be bdrv_* functions that will behave as > blk_*, if that's what you mean. > > Regarding the bdrv_* functions in general, there are a couple of > additional places (which should be all in the main loop) where we need > to use assert_bdrv_grapg_readable. In those places we theoretically need > nothing, but if we use the automatic TSA locking checker that is being > tested by kevin, we need some sort of "placeholder" so that cc/gcc (I > don't remember anymore which) won't throw false positives. clang. :) That can be sorted out with review, but in general I think asserting is always okay. Paolo