Re: [PATCH v3] blockcommit: Reopen base image as RO after abort

[Vladimir Sementsov-Ogievskiy]

On 15.03.24 12:55, Alexander Ivanov wrote:
> On 2/28/24 17:48, Vladimir Sementsov-Ogievskiy wrote:
> > On 09.02.24 15:29, Alexander Ivanov wrote:
> > > Could you please review the patch?
> >
> > Sorry for long delay.
> >
> > Honestly, I don't like refcnt in block-driver. It violate incapsulation, refcnt 
> > is interal thing of common block layer. And actually, you can't make any 
> > assumptions from value of refcnt, as you don't know which additional parents 
> > were created and why, and when they are going unref their children.
> Hmmm... Maybe I can just exclude refcnt check from the condition, can't I. If 
> BDS will be removed it doesn't matter if we make it RO. What do you think?

Sounds good. I even don't see, why you need bdrv_chain_has_significant_parent() 
check. We just roll-back ro->rw transition on failure case, isn't just always 
correct thing to do?

> > What was wrong with v2?
> My bad, it seems, I didn't send v2 before I decided to change the patch.

Hmm, somehow, I don't have it in my mailbox, but here it is: 
https://patchew.org/QEMU/20240109093128.157460-1-alexander.ivanov@virtuozzo.com/


===

More: in commit message you say about failure case. And it seems OK to roll-back 
ro->rw transition on failure, if we did it. But mirror_exit_common() called on 
success path too. I think, on success patch, we should do any additional 
reopenings?

--
Best regards,
Vladimir