Re: [PATCH v2 5/7] block: bdrv_set_perm() drop redundant parameters.

[Vladimir Sementsov-Ogievskiy]

09.11.2020 15:20, Max Reitz wrote:
> On 06.11.20 13:42, Vladimir Sementsov-Ogievskiy wrote:
> > We should never set permissions other than cumulative permissions of
> > parents. During bdrv_reopen_multiple() we _check_ for synthetic
> > permissions but when we do _set_ the graph is already updated.
> > Add an assertion to bdrv_reopen_multiple(), other cases are more
> > obvious.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > ---
> >   block.c | 29 +++++++++++++++--------------
> >   1 file changed, 15 insertions(+), 14 deletions(-)
>
> (Perhaps bdrv_commit_perm() might be a better name then, but I’m afraid such a 
> name change might be quite invasive (because AFAIR *_set_perm is used quite 
> often).)
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>

Thanks for reviewing!

Actually, I plan to split and organize in a similar transactional way 
graph-update operations:

 - aio context change
 - child replacement
 - permission update

So, we'll have a chance to discuss final names later. I think about prepare/commit/abort too, as it is more 
common than check/set/abort. Also, check now actually do set permissions in BdrvChild, so it isn't "just 
check" (and the fact that we should do "abort" after "check" was always a bit odd).

--
Best regards,
Vladimir