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

[Kevin Wolf <kwolf@redhat.com>]

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 <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> ---
>  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.

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

Kevin