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 <[email protected]> >> Signed-off-by: Max Reitz <[email protected]> >> --- >> 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. 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(). Max
signature.asc
Description: OpenPGP digital signature
