On Wed, Apr 08, 2020 at 12:30:47PM +0300, Vladimir Sementsov-Ogievskiy wrote: > It's safer to expand in_flight request to start before enter to
Please explain what exeactly "safer" means. If I understand correctly this is just a refactoring and does not fix bugs that have been hit in the real world. Is this just a generate attempt to avoid accidentally performing operations that need to happen as part of the request after the dec call? > @@ -2718,17 +2746,18 @@ bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector > *qiov, int64_t pos, > ret = drv->bdrv_save_vmstate(bs, qiov, pos); > } > } else if (bs->file) { > - ret = bdrv_co_rw_vmstate(bs->file->bs, qiov, pos, is_read); > + bdrv_inc_in_flight(bs->file->bs); > + ret = bdrv_do_rw_vmstate(bs->file->bs, qiov, pos, is_read); > + bdrv_dec_in_flight(bs->file->bs); Here we inc/dec... > } > > - bdrv_dec_in_flight(bs); > return ret; > } > > static void coroutine_fn bdrv_co_rw_vmstate_entry(void *opaque) > { > BdrvVmstateCo *co = opaque; > - co->ret = bdrv_co_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read); > + co->ret = bdrv_do_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read); ...here we don't. The code is correct, but bdrv_co_rw_vmstate_entry() should also document that its caller must inc/dec. > @@ -2950,7 +2994,7 @@ static void coroutine_fn bdrv_flush_co_entry(void > *opaque) > { > FlushCo *rwco = opaque; > > - rwco->ret = bdrv_co_flush(rwco->bs); > + rwco->ret = bdrv_do_flush(rwco->bs); > aio_wait_kick(); > } This function should also document that the caller must inc/dec.
signature.asc
Description: PGP signature