Am 28.04.2020 um 15:26 hat Max Reitz geschrieben: > Right now, all users of bdrv_make_empty() call the BlockDriver method > directly. That is not only bad style, it is also wrong, unless the > caller has a BdrvChild with a WRITE permission. > > Introduce bdrv_make_empty() that verifies that it does. > > Signed-off-by: Max Reitz <mre...@redhat.com> > --- > include/block/block.h | 1 + > block.c | 23 +++++++++++++++++++++++ > 2 files changed, 24 insertions(+) > > diff --git a/include/block/block.h b/include/block/block.h > index b05995fe9c..d947fb4080 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -351,6 +351,7 @@ BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts > *opts, > void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr); > void bdrv_refresh_limits(BlockDriverState *bs, Error **errp); > int bdrv_commit(BlockDriverState *bs); > +int bdrv_make_empty(BdrvChild *c, Error **errp); > int bdrv_change_backing_file(BlockDriverState *bs, > const char *backing_file, const char *backing_fmt); > void bdrv_register(BlockDriver *bdrv); > diff --git a/block.c b/block.c > index 2e3905c99e..b0d5b98617 100644 > --- a/block.c > +++ b/block.c > @@ -6791,3 +6791,26 @@ void bdrv_del_child(BlockDriverState *parent_bs, > BdrvChild *child, Error **errp) > > parent_bs->drv->bdrv_del_child(parent_bs, child, errp); > } > + > +int bdrv_make_empty(BdrvChild *c, Error **errp) > +{ > + BlockDriver *drv = c->bs->drv; > + int ret; > + > + assert(c->perm & BLK_PERM_WRITE);
If I understand correctly, bdrv_make_empty() is called to drop an overlay whose content is identical to what it would read from its backing file (in particular after a commit operation). This means that the caller promises that the visible content doesn't change. So should we check BLK_PERM_WRITE_UNCHANGED instead? Kevin