Re: [PATCH v4 06/25] include/block/block_int: split header into I/O and global state API

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

On 12/11/2021 13:17, Hanna Reitz wrote:
> On 25.10.21 12:17, Emanuele Giuseppe Esposito wrote:
>> Similarly to the previous patch, split block_int.h
>> in block_int-io.h and block_int-global-state.h
>>
>> block_int-common.h contains the structures shared between
>> the two headers, and the functions that can't be categorized as
>> I/O or global state.
>>
>> Assertions are added in the next patch.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>   blockdev.c                             |    5 +
>>   include/block/block_int-common.h       | 1164 +++++++++++++++++++
>>   include/block/block_int-global-state.h |  319 +++++
>>   include/block/block_int-io.h           |  163 +++
>>   include/block/block_int.h              | 1478 +-----------------------
>>   5 files changed, 1654 insertions(+), 1475 deletions(-)
>>   create mode 100644 include/block/block_int-common.h
>>   create mode 100644 include/block/block_int-global-state.h
>>   create mode 100644 include/block/block_int-io.h
> 
> [...]
> 
>> diff --git a/include/block/block_int-common.h 
>> b/include/block/block_int-common.h
>> new file mode 100644
>> index 0000000000..79a3d801d2
>> --- /dev/null
>> +++ b/include/block/block_int-common.h
> 
> [...]
> 
>> +struct BlockDriver {
> 
> [...]
> 
>> +    /**
>> +     * Try to get @bs's logical and physical block size.
>> +     * On success, store them in @bsz and return zero.
>> +     * On failure, return negative errno.
>> +     */
>> +    /* I/O API, even though if it's a filter jumps on parent */
> 
> I don’t understand this...
> 
>> +    int (*bdrv_probe_blocksizes)(BlockDriverState *bs, BlockSizes *bsz);
>> +    /**
>> +     * Try to get @bs's geometry (cyls, heads, sectors)
>> +     * On success, store them in @geo and return 0.
>> +     * On failure return -errno.
>> +     * Only drivers that want to override guest geometry implement this
>> +     * callback; see hd_geometry_guess().
>> +     */
>> +    /* I/O API, even though if it's a filter jumps on parent */
> 
> ...or this comment.  bdrv_probe_blocksizes() and bdrv_probe_geometry() 
> are in block-global-state.h, so why are these methods part of the I/O 
> API?  (And I’m afraid I can’t parse “even though if it’s a filter jumps 
> on parent”.)
>

Ok this is weird. This comment should not have been there, please ignore 
it. It was just a note for myself while I was doing one the the many 
pass to split all these functions. This is not I/O and as you probably 
have already seen, I did not put in I/O. Also patch 19 takes care of the 
function pointers in BlockDriver, not this (but you discovered it already).

Apologies.

Emanuele

> Hanna
> 
>> +    int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
>