Re: [PATCH v6 21/33] block: move BQL logic of bdrv_co_invalidate_cache in bdrv_activate

[Paolo Bonzini <pbonzini@redhat.com>]

On 1/27/22 12:03, Kevin Wolf wrote:
>> +int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp)
>> +{
>> +    Error *local_err = NULL;
>> +
>> +    if (bs->drv->bdrv_co_invalidate_cache) {
>> +        bs->drv->bdrv_co_invalidate_cache(bs, &local_err);
>> +        if (local_err) {
>> +            bs->open_flags |= BDRV_O_INACTIVE;
> 
> This doesn't feel like the right place. The flag is cleared by the
> caller, so it should also be set again on failure by the caller and not
> by this function.
> 
> What bdrv_co_invalidate_cache() could do is assert that BDRV_O_INACTIVE
> is cleared when it's called.

Do you think this would be handled more easily into its own series?

In general, the work in this series is more incremental than its size 
suggests.  Perhaps it should be flushed out in smaller pieces.

Paolo