Re: [Qemu-devel] [PATCH v4 06/38] block: Make bdrv_is_inserted() recursive

[Max Reitz]

On 07.09.2015 19:43, Kevin Wolf wrote:
> Am 20.07.2015 um 19:45 hat Max Reitz geschrieben:
>> If bdrv_is_inserted() is called on the top level BDS, it should make
>> sure all nodes in the BDS tree are actually inserted.
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>> Reviewed-by: Alberto Garcia <address@hidden>
>> ---
>>  block.c | 7 +++----
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 494e08e..1d27b6a 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3235,10 +3235,9 @@ bool bdrv_is_inserted(BlockDriverState *bs)
>>      if (!drv) {
>>          return false;
>>      }
>> -    if (!drv->bdrv_is_inserted) {
>> -        return true;
>> -    }
>> -    return drv->bdrv_is_inserted(bs);
>> +    return (!drv->bdrv_is_inserted || drv->bdrv_is_inserted(bs)) &&
>> +           (!bs->file              || bdrv_is_inserted(bs->file)) &&
>> +           (!bs->backing_hd        || bdrv_is_inserted(bs->backing_hd));
>>  }
> 
> Hm... Recursion often makes the right semantics unclear. I think though
> what you're after here is good as a default behaviour, i.e. a non-leaf
> node is inserted iff all of its children are inserted. We can do things
> in various ways without breaking stuff because raw-posix is the only
> driver actually implementing .bdrv_is_inserted, but I think it would
> make most sense like this:
> 
> * If a driver implements .bdrv_is_inserted, we use this (and only this)
>   for determining whether a medium is inserted.
> 
> * The default behaviour for drivers which don't have .bdrv_is_inserted
>   is checking all children (in bs->children, not restricted to file and
>   backing_hd, so that quorum etc. work)
> 
> * Consequently, a driver that doesn't want all of its children
>   considered (which may be a very valid desire), can implement its own
>   handler and doesn't get the default handling then.
> 
> This seems to be the most consistent with other recursive operations.

You're right, I'll change this patch accordingly.

> Also, after this patch, raw_bsd.c should be able drop its
> implementation.

Indeed.

Max

signature.asc
Description: OpenPGP digital signature