Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/3] block: pass bdrv_* methods to bs->file by default

[Eric Blake]

On 06/29/2017 01:43 PM, Manos Pitsidianakis wrote:
> The following functions fail if bs->drv does not implement them:
> 
> bdrv_probe_blocksizes
> bdrv_probe_geometry
> bdrv_truncate
> bdrv_has_zero_init
> bdrv_get_info
> bdrv_media_changed
> bdrv_eject
> bdrv_lock_medium
> bdrv_co_ioctl
> 
> Instead, the call should be passed to bs->file if it exists, to allow
> filter drivers to support those methods without implementing them.
> 
> Signed-off-by: Manos Pitsidianakis <address@hidden>
> ---
>  block.c    | 27 +++++++++++++++++++++++++--
>  block/io.c |  5 +++++
>  2 files changed, 30 insertions(+), 2 deletions(-)

Did you check whether any other existing drivers can be cleaned up to
rely on the defaults?  Or is it just the raw driver that you cleaned in 2/3?

At any rate, this makes sense to me.  However,

> @@ -4205,6 +4222,8 @@ int bdrv_media_changed(BlockDriverState *bs)
>  
>      if (drv && drv->bdrv_media_changed) {
>          return drv->bdrv_media_changed(bs);
> +    } else if (drv && bs->file && bs->file->bs) {
> +        bdrv_media_changed(bs->file->bs);
>      }
>      return -ENOTSUP;

This one returns -ENOTSUP unconditionally after recursing; looks like
you omitted 'return'.

If that's the only fix you make for v3, you can add:
Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature