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