qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 13/16] block: Implement bdrv_append() without


From: Alberto Garcia
Subject: Re: [Qemu-devel] [PATCH v3 13/16] block: Implement bdrv_append() without bdrv_swap()
Date: Tue, 13 Oct 2015 11:01:34 +0200
User-agent: Notmuch/0.13.2 (http://notmuchmail.org) Emacs/23.2.1 (i486-pc-linux-gnu)

On Tue 13 Oct 2015 10:39:22 AM CEST, Kevin Wolf <address@hidden> 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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]