[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 21/33] block: move BQL logic of bdrv_co_invalidate_cache i
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v6 21/33] block: move BQL logic of bdrv_co_invalidate_cache in bdrv_activate |
Date: |
Wed, 2 Feb 2022 19:27:03 +0100 |
Am 02.02.2022 um 18:27 hat Paolo Bonzini geschrieben:
> 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.
Smaller pieces are always easier to handle, so if things make sense
independently, splitting them off is usually a good idea.
Kevin