Re: [PATCH v5 09/45] Revert "block: Let replace_child_noperm free children"

[Vladimir Sementsov-Ogievskiy]

On 6/7/22 17:03, Hanna Reitz wrote:
> On 30.03.22 23:28, Vladimir Sementsov-Ogievskiy wrote:
> > We are going to reimplement this behavior (clear bs->file / bs->backing
> > pointers automatically when child->bs is cleared) in a nicer way.
> >
> > This reverts commit b0a9f6fed3d80de610dcd04a7e66f9f30a04174f.
>
> This doesn’t really explain why it’s fine to revert this commit here. As far as 
> I understand, the bug that was fixed in that commit will resurface when it is 
> reverted without the proposed reimplementation, so technically, we cannot 
> revert before reimplementing.
>
> As far as I can guess, it’d be unwieldy to do the reimplementation while these 
> existing changes are in the way, and it’d be one bomb of a patch to squash 
> these five patches (9 to 14) into one, and that’s why you’ve chosen to do it 
> this way around.

Yes, that's the reason

> But technically, we can’t willingly break something just to keep patches nicer. 
>  We can make exceptions, but then there needs to be justification here in the 
> commit message.

Agree, will add.

As far as I remember (and after re-reading commit message) b0a9f6fed3d80de610dc 
was not a direct fix of some concrete bug. It was a measure to prevent 
theoretic problems. And we don't have any test for it. So I think, breaking 
bisect at this point for some future test is not too bad.

> (Or perhaps I’m wrong and it is fine at this point to revert the patch, but 
> then I’d like to see the explanation for that, too, because I can’t see it 
> myself.)
>
> Hanna


--
Best regards,
Vladimir