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

[Emanuele Giuseppe Esposito]

On 07/10/2021 11:33, Stefan Hajnoczi wrote:
> On Tue, Oct 05, 2021 at 10:31:54AM -0400, Emanuele Giuseppe Esposito wrote:
> > +int bdrv_pwrite_sync(BdrvChild *child, int64_t offset,
> > +                     const void *buf, int64_t bytes);
>
> Why is this bit of a surprise since the other synchronous I/O functions
> aren't included in this header. Why did you put it here? This one may be
> safe to move to the I/O API.

Considering that in the next patch I did not even add an assertion for 
it, I am confident enough that it was a copy-paste mistake. This goes 
into I/O.

> > +int bdrv_block_status(BlockDriverState *bs, int64_t offset,
> > +                      int64_t bytes, int64_t *pnum, int64_t *map,
> > +                      BlockDriverState **file);
>
> This function just called bdrv_block_status_above(), which is in the I/O
> API. I think it's safe to move this to the I/O API or else
> bdrv_block_status_above() shouldn't be there :).

It *seems* that while bdrv_block_status_above() is an I/O, probably 
running in some coroutine (from here its internal qemu_in_coroutine 
check), bdrv_block_status might be called from the main loop (or 
alternatively the function is never invoked in the tests, so the 
assertion never triggered).

Maybe bdrv_block_status_above is one of the few functions that are both 
I/O and Main loop? I put it in I/O as it can't have the assertion.

Thank you,
Emanuele