[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 06/38] block: Make bdrv_is_inserted() recursi
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v4 06/38] block: Make bdrv_is_inserted() recursive |
Date: |
Mon, 7 Sep 2015 20:03:35 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
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