On Tue 12 Mar 2019 01:20:54 PM CET, Kevin Wolf wrote:
>> + reopen_state->replace_backing_bs = true;
>> + reopen_state->new_backing_bs = new_backing_bs;
>
> Do we need to bdrv_ref(new_backing_bs) here in case its only reference
> is dropped in the same reopen transaction?
I'm not sure if it can happen with the current code, but the reopen
state should have an extra reference so I'll add it.
>> + /* Check if new_backing_bs would accept the new permissions */
>> + if (reopen_state->replace_backing_bs && reopen_state->new_backing_bs) {
>> + uint64_t cur_perm, cur_shared;
>> + bdrv_child_perm(reopen_state->bs, reopen_state->new_backing_bs,
>> + NULL, &child_backing, NULL,
>
> Why do we pass NULL instead of queue here? Shouldn't the required
> permissions be calculated based on the post-reopen state?
You're right, I'll fix it.
Berto