Re: [Qemu-devel] [PATCH v4 1/4] block: pass bdrv_* methods to bs->file by default in block filters

2017-07-12 Thread Eric Blake 
On 07/11/2017 11:37 AM, Manos Pitsidianakis wrote:
> The following functions fail if bs->drv is a filter and does not
> implement them:
> 
> bdrv_probe_blocksizes
> bdrv_probe_geometry
> bdrv_truncate
> bdrv_has_zero_init
> bdrv_get_info
> 
> Instead, the call should be passed to bs->file if it exists, to allow
> filter drivers to support those methods without implementing them. This
> commit makes `drv->is_filter = true` imply that these callbacks will be
> forwarded to bs->file by default, so disabling support for these
> functions must be done explicitly.
> 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  block.c   | 21 +++--
>  include/block/block_int.h |  6 +-
>  2 files changed, 24 insertions(+), 3 deletions(-)

> @@ -3406,11 +3410,15 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, 
> Error **errp)
>  
>  assert(child->perm & BLK_PERM_RESIZE);
>  
> +/* if bs->drv == NULL, bs is closed, so there's nothing to do here */
>  if (!drv) {
>  error_setg(errp, "No medium inserted");
>  return -ENOMEDIUM;
>  }
>  if (!drv->bdrv_truncate) {
> +if (bs->file && drv->is_filter) {
> +return bdrv_truncate(bs->file, offset, errp);
> +}

This has a semantic (but not merge) conflict with Max's preallocation
work, and it started to be non-trivial for me to adjust locally in my
testing. It may be easiest if you send a v5 on top of
'https://github.com/XanClic/qemu.git block', since Max already has a
pending pull request:

https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg02841.html

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 1/4] block: pass bdrv_* methods to bs->file by default in block filters

2017-07-12 Thread Stefan Hajnoczi 
On Tue, Jul 11, 2017 at 07:37:45PM +0300, Manos Pitsidianakis wrote:
> The following functions fail if bs->drv is a filter and does not
> implement them:
> 
> bdrv_probe_blocksizes
> bdrv_probe_geometry
> bdrv_truncate
> bdrv_has_zero_init
> bdrv_get_info
> 
> Instead, the call should be passed to bs->file if it exists, to allow
> filter drivers to support those methods without implementing them. This
> commit makes `drv->is_filter = true` imply that these callbacks will be
> forwarded to bs->file by default, so disabling support for these
> functions must be done explicitly.
> 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  block.c   | 21 +++--
>  include/block/block_int.h |  6 +-
>  2 files changed, 24 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v4 1/4] block: pass bdrv_* methods to bs->file by default in block filters

2017-07-11 Thread Eric Blake 
On 07/11/2017 11:37 AM, Manos Pitsidianakis wrote:
> The following functions fail if bs->drv is a filter and does not
> implement them:
> 
> bdrv_probe_blocksizes
> bdrv_probe_geometry
> bdrv_truncate
> bdrv_has_zero_init
> bdrv_get_info
> 
> Instead, the call should be passed to bs->file if it exists, to allow
> filter drivers to support those methods without implementing them. This
> commit makes `drv->is_filter = true` imply that these callbacks will be
> forwarded to bs->file by default, so disabling support for these
> functions must be done explicitly.
> 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  block.c   | 21 +++--
>  include/block/block_int.h |  6 +-
>  2 files changed, 24 insertions(+), 3 deletions(-)
> @@ -3778,6 +3786,9 @@ int bdrv_has_zero_init(BlockDriverState *bs)
>  if (bs->drv->bdrv_has_zero_init) {
>  return bs->drv->bdrv_has_zero_init(bs);
>  }
> +if (bs->file && bs->drv->is_filter) {
> +return bdrv_has_zero_init(bs->file->bs);
> +}
>  
>  /* safe default */
>  return 0;

Someday, we should probably clean this function (and all callback
implementations) to return bool rather than int.  But not this patch.

Certainly more conservative than the earlier versions.  I'd still trust
Kevin's review over mine, but looks okay to me.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature