Am 10.09.2019 um 14:04 hat Max Reitz geschrieben: > On 10.09.19 13:56, Kevin Wolf wrote: > > Am 09.08.2019 um 18:13 hat Max Reitz geschrieben: > >> If the top node's driver does not provide snapshot functionality and we > >> want to fall back to a node down the chain, we need to snapshot all > >> non-COW children. For simplicity's sake, just do not fall back if there > >> is more than one such child. > >> > >> bdrv_snapshot_goto() becomes a bit weird because we may have to redirect > >> the actual child pointer, so it only works if the fallback child is > >> bs->file or bs->backing (and then we have to find out which it is). > >> > >> Suggested-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > >> Signed-off-by: Max Reitz <mre...@redhat.com> > >> --- > >> block/snapshot.c | 100 +++++++++++++++++++++++++++++++++++++---------- > >> 1 file changed, 79 insertions(+), 21 deletions(-) > >> > >> diff --git a/block/snapshot.c b/block/snapshot.c > >> index f2f48f926a..35403c167f 100644 > >> --- a/block/snapshot.c > >> +++ b/block/snapshot.c > >> @@ -146,6 +146,32 @@ bool > >> bdrv_snapshot_find_by_id_and_name(BlockDriverState *bs, > >> return ret; > >> } > >> > >> +/** > >> + * Return the child BDS to which we can fall back if the given BDS > >> + * does not support snapshots. > >> + * Return NULL if there is no BDS to (safely) fall back to. > >> + */ > >> +static BlockDriverState *bdrv_snapshot_fallback(BlockDriverState *bs) > >> +{ > >> + BlockDriverState *child_bs = NULL; > >> + BdrvChild *child; > >> + > >> + QLIST_FOREACH(child, &bs->children, next) { > >> + if (child == bdrv_filtered_cow_child(bs)) { > >> + /* Ignore: COW children need not be included in snapshots */ > >> + continue; > >> + } > >> + > >> + if (child_bs) { > >> + /* Cannot fall back to a single child if there are multiple */ > >> + return NULL; > >> + } > >> + child_bs = child->bs; > >> + } > >> + > >> + return child_bs; > >> +} > > > > Why do we return child->bs here when bdrv_snapshot_goto() then needs to > > reconstruct what the associated BdrvChild was? Wouldn't it make more > > sense to return BdrvChild** from here and maybe have a small wrapper for > > the other functions that only need a BDS? > > What would you return instead? &child doesn’t work.
Oops, brain fart. :-) > We could limit ourselves to bs->file and bs->backing. It just seemed > like a bit of an artificial limit to me, because we only really have it > for bdrv_snapshot_goto(). Hm, but then, what use is supporting other children for creating a snapshot when you can't load it any more afterwards? Kevin
signature.asc
Description: PGP signature