On Tue 13 Oct 2015 10:39:22 AM CEST, Kevin Wolf <[email protected]> wrote:
>> > +static void change_parent_backing_link(BlockDriverState *from,
>> > + BlockDriverState *to)
>> > +{
>> > + BdrvChild *c, *next;
>> > +
>> > + QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) {
>> > + assert(c->role != &child_backing);
>> > + c->bs = to;
>> > + QLIST_REMOVE(c, next_parent);
>> > + QLIST_INSERT_HEAD(&to->parents, c, next_parent);
>> > + bdrv_ref(to);
>> > + bdrv_unref(from);
>> > + }
>> > + if (from->blk) {
>> > + blk_set_bs(from->blk, to);
>> > + if (!to->device_list.tqe_prev) {
>> > + QTAILQ_INSERT_BEFORE(from, to, device_list);
>> > + }
>>
>> Is it even possible that this last condition is false? In what case
>> would 'to' be already in bdrv_states?
>>
>> I understand that it would mean that it would already be attached to
>> a BlockBackend, but that's not possible in this case.
>
> Yes, I think it's not possible currently (hopefully, because that
> would cause other bugs), just being careful. Eventually we'll allow
> more than one BlockBackend pointing to the same BDS, and then this is
> a scenario that could happen.
blk_set_bs() already asserts that to->blk == NULL, so if this is not
possible here wouldn't it be better to use another assertion instead?
When I was reviewing the code I kept wondering in what kind of scenario
this condition could be false.
Berto
