Re: [RFC PATCH v2 04/25] include/block/block: split header into I/O and global state API

[Stefan Hajnoczi]

On Thu, Oct 07, 2021 at 12:43:43PM +0100, Daniel P. Berrangé wrote:
> On Tue, Oct 05, 2021 at 10:31:54AM -0400, Emanuele Giuseppe Esposito wrote:
> > Similarly to the previous patch, split block.h
> > in block-io.h and block-global-state.h
> > 
> > block-common.h contains the structures shared between
> > the two headers, and the functions that can't be categorized as
> > I/O or global state.
> 
> This is nice from a code organization POV, but it doesn't do all
> that much from a code reviewer / author POV as I doubt anyone
> will remember which header file the respective APIs/structures/
> constants are in, without having to look it up each time.
> 
> It would make life easier if we had distinct namning conventions
> for APIs/struct/contsants in the respective headers.
> 
> eg instead of  "bdrv_" have "bdrv_state_" and "bdrv_io_" as
> the two naming conventions for -global-state.h and -io.h
> respectively, nad only use the bare 'bdrv_' for -common.h
> 
> Yes, this would be major code churn, but I think it'd make
> the code clearer to understand which will be a win over the
> long term.
> 
> NB, I'm not suggesting doing a rename as part of this patch
> though. Any rename would have to be separate, and likely
> split over many patches to make it manageable.

Yes. Taking it one step further, BlockDriverState could be split into
two struct so that I/O code doesn't even have access to the struct
needed to invoke GS APIs. This is a type-safe way of enforcing the API
split.

Unfortunately that's a lot of code churn and I think the separation is
not very clean. For example, block drivers need to forward requests to
their children, so they need to traverse the graph (which we think of as
global state).

Stefan

signature.asc
Description: PGP signature