09.08.2019 19:13, Max Reitz wrote: > In order to make filters work in backing chains, the associated > functions must be able to deal with them and freeze all filter links, be > they COW or R/W filter links. > > In the process, rename these functions to reflect that they now act on > generalized chains of filter nodes instead of backing chains alone. > > While at it, add some comments that note which functions require their > caller to ensure that a given child link is not frozen, and how the > callers do so. > > Signed-off-by: Max Reitz <mre...@redhat.com> > --- > include/block/block.h | 10 +++--- > block.c | 81 +++++++++++++++++++++++++------------------ > block/commit.c | 8 ++--- > block/mirror.c | 4 +-- > block/stream.c | 8 ++--- > 5 files changed, 62 insertions(+), 49 deletions(-) > > diff --git a/include/block/block.h b/include/block/block.h > index 50a07c1c33..f6f09b95cd 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -364,11 +364,11 @@ int bdrv_drop_intermediate(BlockDriverState *top, > BlockDriverState *base, > BlockDriverState *bdrv_find_overlay(BlockDriverState *active, > BlockDriverState *bs); > BlockDriverState *bdrv_find_base(BlockDriverState *bs); > -bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState > *base, > - Error **errp); > -int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, > - Error **errp); > -void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState > *base); > +bool bdrv_is_chain_frozen(BlockDriverState *bs, BlockDriverState *base, > + Error **errp); > +int bdrv_freeze_chain(BlockDriverState *bs, BlockDriverState *base, > + Error **errp); > +void bdrv_unfreeze_chain(BlockDriverState *bs, BlockDriverState *base); > > > typedef struct BdrvCheckResult { > diff --git a/block.c b/block.c > index adf82efb0e..650c00d182 100644 > --- a/block.c > +++ b/block.c > @@ -2303,12 +2303,15 @@ static void bdrv_replace_child_noperm(BdrvChild > *child, > * If @new_bs is not NULL, bdrv_check_perm() must be called beforehand, as > this > * function uses bdrv_set_perm() to update the permissions according to the > new > * reference that @new_bs gets. > + * > + * Callers must ensure that child->frozen is false. > */ > static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs) > { > BlockDriverState *old_bs = child->bs; > uint64_t perm, shared_perm; > > + /* Asserts that child->frozen == false */ > bdrv_replace_child_noperm(child, new_bs); > > /* > @@ -2468,6 +2471,7 @@ static void bdrv_detach_child(BdrvChild *child) > g_free(child); > } > > +/* Callers must ensure that child->frozen is false. */ > void bdrv_root_unref_child(BdrvChild *child) > { > BlockDriverState *child_bs; > @@ -2477,10 +2481,6 @@ void bdrv_root_unref_child(BdrvChild *child) > bdrv_unref(child_bs); > } > > -/** > - * Clear all inherits_from pointers from children and grandchildren of > - * @root that point to @root, where necessary. > - */
Hmm, unrelated chunk? Without it: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > static void bdrv_unset_inherits_from(BlockDriverState *root, BdrvChild > *child) > { > BdrvChild *c; > @@ -2505,6 +2505,7 @@ static void bdrv_unset_inherits_from(BlockDriverState > *root, BdrvChild *child) [..] -- Best regards, Vladimir