Re: [PATCH 2/7] block: Manipulate children list in .attach/.detach

[Hanna Reitz <hreitz@redhat.com>]

On 10.11.21 13:46, Vladimir Sementsov-Ogievskiy wrote:
> 04.11.2021 13:38, Hanna Reitz wrote:
>> The children list is specific to BDS parents.  We should not modify it
>> in the general children modification code, but let BDS parents deal with
>> it in their .attach() and .detach() methods.
>>
>> This also has the advantage that a BdrvChild is removed from the
>> children list before its .bs pointer can become NULL.  BDS parents
>> generally assume that their children's .bs pointer is never NULL, so
>> this is actually a bug fix.
>>
>> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
>> ---
>>   block.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 580cb77a70..243ae206b5 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1387,6 +1387,8 @@ static void bdrv_child_cb_attach(BdrvChild *child)
>>   {
>>       BlockDriverState *bs = child->opaque;
>>   +    QLIST_INSERT_HEAD(&bs->children, child, next);
>> +
>>       if (child->role & BDRV_CHILD_COW) {
>>           bdrv_backing_attach(child);
>>       }
>> @@ -1403,6 +1405,8 @@ static void bdrv_child_cb_detach(BdrvChild *child)
>>       }
>>         bdrv_unapply_subtree_drain(child, bs);
>> +
>> +    QLIST_REMOVE(child, next);
>>   }
>>     static int bdrv_child_cb_update_filename(BdrvChild *c, 
>> BlockDriverState *base,
>> @@ -2747,7 +2751,7 @@ static void bdrv_child_free(void *opaque)
>>   static void bdrv_remove_empty_child(BdrvChild *child)
>>   {
>>       assert(!child->bs);
>> -    QLIST_SAFE_REMOVE(child, next);
>> +    assert(!child->next.le_prev); /* not in children list */
>>       bdrv_child_free(child);
>>   }
>>   @@ -2913,7 +2917,6 @@ static int 
>> bdrv_attach_child_noperm(BlockDriverState *parent_bs,
>>           return ret;
>>       }
>>   -    QLIST_INSERT_HEAD(&parent_bs->children, *child, next);
>
> The following comment become stale. We should remove it too. With 
> comment removed:

Ah, right, thanks!

> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>