On 14.06.19 15:26, Vladimir Sementsov-Ogievskiy wrote: > 13.06.2019 1:09, Max Reitz wrote: >> Use child access functions when iterating through backing chains so >> filters do not break the chain. >> >> Signed-off-by: Max Reitz <mre...@redhat.com> >> --- >> block.c | 40 ++++++++++++++++++++++++++++------------ >> 1 file changed, 28 insertions(+), 12 deletions(-) >> >> diff --git a/block.c b/block.c >> index 11f37983d9..505b3e9a01 100644 >> --- a/block.c >> +++ b/block.c
[...] >> @@ -4273,11 +4274,18 @@ int bdrv_change_backing_file(BlockDriverState *bs, >> BlockDriverState *bdrv_find_overlay(BlockDriverState *active, >> BlockDriverState *bs) >> { >> - while (active && bs != backing_bs(active)) { >> - active = backing_bs(active); >> + bs = bdrv_skip_rw_filters(bs); >> + active = bdrv_skip_rw_filters(active); >> + >> + while (active) { >> + BlockDriverState *next = bdrv_backing_chain_next(active); >> + if (bs == next) { >> + return active; >> + } >> + active = next; >> } >> >> - return active; >> + return NULL; >> } > > Semantics changed for this function. > It is used in two places > 1. from bdrv_find_base wtih @bs=NULL, it should be unchanged, as I hope we > will never have > filter node as a bottom of some valid chain > > 2. from qmp_block_commit, only to check op-blocker... hmmm. I really don't > understand, > why do we check BLOCK_OP_TYPE_COMMIT_TARGET on top_bs overlay.. top_bs > overlay is out of the job, > what is this check for? There is a loop before this check which checks that the same blocker is not set on any nodes between top and base (both inclusive). I guess non-active commit checks the node above @top, too, because its backing file will change. >> /* Given a BDS, searches for the base layer. */ [...] >> @@ -5149,7 +5165,7 @@ BlockDriverState >> *bdrv_find_backing_image(BlockDriverState *bs, >> char *backing_file_full_ret; >> >> if (strcmp(backing_file, curr_bs->backing_file) == 0) { > > hmm, interesting, what bs->backing_file now means? It's strange enough to > store such field on > bds, when we have backing link anyway.. Patch 37 has you covered. :-) Max
signature.asc
Description: OpenPGP digital signature