Re: [RFC] migration/block-dirty-bitmap: make loading bitmap for device with iothread future-proof

[Vladimir Sementsov-Ogievskiy]

On 31.07.23 10:35, Juan Quintela wrote:
> Fiona Ebner <f.ebner@proxmox.com> wrote:
> > The bdrv_create_dirty_bitmap() function (which is also called by
> > bdrv_dirty_bitmap_create_successor()) uses bdrv_getlength(bs). This is
> > a wrapper around a coroutine, and when not called in coroutine context
> > would use bdrv_poll_co(). Such a call would trigger an assert() if the
> > correct AioContext hasn't been acquired before, because polling would
> > try to release the AioContext.
>
> The ingenous in me thinks:
>
> If the problem is that bdrv_poll_co() release an AioContext that it
> don't have acquired, perhaps we should fix bdrv_poll_co().
>
> Ha!!!
>
> $ find . -type f -exec grep --color=auto -nH --null -e bdrv_poll_co \{\} +
> ./scripts/block-coroutine-wrapper.py\0173:
> bdrv_poll_co(&s.poll_state);
> ./scripts/block-coroutine-wrapper.py\0198:
> bdrv_poll_co(&s.poll_state);
> ./block/block-gen.h\038:static inline void bdrv_poll_co(BdrvPollCo *s)
> $
>
> /me retreats

The function is used in generated code. There are a lot of calls in 
build/block/block-gen.c if grep after make.

> > The issue does not happen for migration, because the call happens
> > from process_incoming_migration_co(), i.e. in coroutine context. So
> > the bdrv_getlength() wrapper will just call bdrv_co_getlength()
> > directly without polling.
>
> The ingenous in me wonders why bdrv_getlength() needs to use coroutines
> at all, but as I have been burned on the previous paragraph, I learn not
> to even try.
>
> Ok, I never learn, so I do a grep and I see two appearces of
> bdrv_getlength in include files, but grep only shows uses of the
> function, not a real definition.

the function is generated, and after building, it's definition is in 
build/block/block-gen.c

The information is in comment in include/block/block-common.h.

The link, which lead from function declaration to the comment is "co_wrapper", 
but that's not obvious when just grep the function name.


--
Best regards,
Vladimir