Re: [PATCH v4 04/25] include/sysemu/block-backend: split header into I/O and global state (GS) API

[Hanna Reitz <hreitz@redhat.com>]

On 16.11.21 15:24, Emanuele Giuseppe Esposito wrote:
>
>
> On 12/11/2021 13:30, Hanna Reitz wrote:
>> On 25.10.21 12:17, Emanuele Giuseppe Esposito wrote:
>>> Similarly to the previous patches, split block-backend.h
>>> in block-backend-io.h and block-backend-global-state.h
>>>
>>> In addition, remove "block/block.h" include as it seems
>>> it is not necessary anymore, together with "qemu/iov.h"
>>>
>>> block-backend-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>
>>> ---
>>>   block/block-backend.c                       |   9 +-
>>>   include/sysemu/block-backend-common.h       |  74 ++++++
>>>   include/sysemu/block-backend-global-state.h | 122 +++++++++
>>>   include/sysemu/block-backend-io.h           | 139 ++++++++++
>>>   include/sysemu/block-backend.h              | 269 
>>> +-------------------
>>>   5 files changed, 344 insertions(+), 269 deletions(-)
>>>   create mode 100644 include/sysemu/block-backend-common.h
>>>   create mode 100644 include/sysemu/block-backend-global-state.h
>>>   create mode 100644 include/sysemu/block-backend-io.h
>>
>> [...]
>>
>>> diff --git a/include/sysemu/block-backend.h 
>>> b/include/sysemu/block-backend.h
>>> index e5e1524f06..038be9fc40 100644
>>> --- a/include/sysemu/block-backend.h
>>> +++ b/include/sysemu/block-backend.h
>>> @@ -13,272 +13,9 @@
>>>   #ifndef BLOCK_BACKEND_H
>>>   #define BLOCK_BACKEND_H
>>> -#include "qemu/iov.h"
>>> -#include "block/throttle-groups.h"
>>> +#include "block-backend-global-state.h"
>>> +#include "block-backend-io.h"
>>> -/*
>>> - * TODO Have to include block/block.h for a bunch of block layer
>>> - * types.  Unfortunately, this pulls in the whole BlockDriverState
>>> - * API, which we don't want used by many BlockBackend users. Some of
>>> - * the types belong here, and the rest should be split into a common
>>> - * header and one for the BlockDriverState API.
>>> - */
>>> -#include "block/block.h"
>>
>> This note and the include is gone.  Sounds like something positive, 
>> but why is this possible?
>>
>
> Basically block/throttle-groups.h includes block/block_int.h that 
> internally includes block/block.h.
>
> But I am not sure if you actually want to keep this comment as 
> reminder for future work. Should I keep it?

Good question.  I think I’d keep it and the block.h include; I mean, the 
throttle-groups.h include was there before already, so perhaps this was 
indeed only intended as a reminder.

The other reason to keep it is that ideal this is just a refactoring 
patch, so I wouldn’t touch anything that needn’t be touched.

Hanna