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

Reply via email to