Re: [PATCH v5 00/31] block layer: split block APIs in global state and I/O

[Emanuele Giuseppe Esposito <eesposit@redhat.com>]

On 29/11/2021 14:32, Stefan Hajnoczi wrote:
> On Wed, Nov 24, 2021 at 01:43:47AM -0500, Emanuele Giuseppe Esposito wrote:
>> v5 -> v6:
>> * In short, apply all Hanna's comments. More in details,
>>    the following functions in the following headers have been moved:
>>    block-backend:
>>      blk_replace_bs (to gs)
>>      blk_nb_sectors (to io)
>>      blk_name (to io)
>>      blk_set_perm (to io)
>>      blk_get_perm (to io)
>>      blk_drain (to io)
>>      blk_abort_aio_request (to io)
>>      blk_make_empty (to gs)
>>      blk_invalidate_cache (was in io, but had GS assertion)
>>      blk_aio_cancel (to gs)
>>    block:
>>      bdrv_replace_child_bs (to gs)
>>      bdrv_get_device_name (to io)
>>      bdrv_get_device_or_node_name (to io)
>>      bdrv_drained_end_no_poll (to io)
>>      bdrv_block_status (was in io, but had GS assertion)
>>      bdrv_drain (to io)
>>      bdrv_co_drain (to io)
>>      bdrv_make_zero (was in GS, but did not have the assertion)
>>      bdrv_save_vmstate (to io)
>>      bdrv_load_vmstate (to io)
>>      bdrv_aio_cancel_async (to io)
>>    block_int:
>>      bdrv_get_parent_name (to io)
>>      bdrv_apply_subtree_drain (to io)
>>      bdrv_unapply_subtree_drain (to io)
>>      bdrv_co_copy_range_from (was in io, but had GS assertion)
>>      bdrv_co_copy_range_to (was in io, but had GS assertion)
>>      ->bdrv_save_vmstate (to io)
>>      ->bdrv_load_vmstate (to io)
>>
>>    coding style (assertion after definitions):
>>      bdrv_save_vmstate
>>      bdrv_load_vmstate
>>      block_job_next
>>      block_job_get
>>
>>    new patches:
>>      block.c: modify .attach and .detach callbacks of child_of_bds
>>      introduce pre_run as JobDriver callback to handle
>>        bdrv_co_amend usage of permission function
>>      leave blk_set/get_perm as a TODO in fuse.c
>>      make sure bdrv_co_invalidate_cache does not use permissions
>>        if BQL is not held
>>
>>    minor changes:
>>      put back TODO for include block/block.h in block-backend-common.h
>>      rebase on kwolf/block branch
>>      modify where are used assert_bdrv_graph_writable, due to rebase
> 
> These changes sound fine to me. Hanna or Kevin can merge the series when
> they are happy.
> 

Just a tiny nitpick, I would just not merge patch 10: "block.c: modify 
.attach and .detach callbacks of child_of_bds". It is not wrong and 
should produce no side effect, but does not do what it was intended for 
(ie using the drain to protect the bs's list of children). I will work 
on that in a future serie. Apologies.

Thank you,
Emanuele