Am 26.09.2013 um 04:01 hat Jeff Cody geschrieben: > On Wed, Sep 25, 2013 at 04:23:22PM +0200, Benoît Canet wrote: > > Drivers having a bs->file where set to recurse the call to their child. > > Protocol and drivers designed to be on the bottom of the stack where set to > > allow > > snapshots. > > Future protocols like quorum where creating snapshots does not make sense > > without block filters will be set to forbid snapshots. > > > > Signed-off-by: Benoit Canet <ben...@irqsave.net>
> > diff --git a/block.c b/block.c > > index 4a98250..ff296df 100644 > > --- a/block.c > > +++ b/block.c > > @@ -4651,3 +4651,30 @@ int bdrv_amend_options(BlockDriverState *bs, > > QEMUOptionParameter *options) > > } > > return bs->drv->bdrv_amend_options(bs, options); > > } > > + > > +bool bdrv_is_ext_snapshot_forbidden(BlockDriverState *bs) > > +{ > > I think either: > A) Name this function bdrv_forbid_ext_snapshots(), or > B) Name the BlockDriver function ptr to .bdrv_is_ext_snapshot_forbidden > > The idea being that this function and the BlockDriver function ptr > should have the same name (e.g. bdrv_has_zero_init, and > bs->drv->bdrv_has_zero_init, etc..) Yes, I agree, some consistent naming is desirable. I don't think bdrv_forbid_ext_snapshots() is a good name, because it implies that calling this function is what forbids the snapshot (i.e. an action similar to adding a migration blocker), whereas in fact it just checks whether snapshots are forbidden. How about bdrv_ext_snapshot_allowed(), which avoid double negations when we check for "not forbidden"? Or perhaps even bdrv_check_ext_snapshot(), which would be a more generic name that could be extended to the three-way distinction we intended to have in the end: - External snapshots are forbidden - May snapshot, but below this BDS (ask bs->file; this is for filters) - Do the snapshot here Kevin