[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v5 1/4] block: pass bdrv_* methods to bs->file b
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v5 1/4] block: pass bdrv_* methods to bs->file by default in block filters |
Date: |
Thu, 13 Jul 2017 06:23:14 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 07/13/2017 05:01 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.
>
> Reviewed-by: Eric Blake <address@hidden>
I would have dropped R-b on the grounds that you added comments to
'is_filter' (good! but non-trivial), to make sure it gets a
grammar/wording review of the new content. Furthermore, your patch had
a semantic change due to the rebase, and looking at that change...
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> Signed-off-by: Manos Pitsidianakis <address@hidden>
> ---
> @@ -3420,11 +3424,15 @@ int bdrv_truncate(BdrvChild *child, int64_t offset,
> PreallocMode prealloc,
>
> 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, prealloc, offset, errp);
...ouch - you got it wrong. s/prealloc, offset/offset, prealloc/
> +++ b/include/block/block_int.h
> @@ -87,7 +87,11 @@ struct BlockDriver {
> const char *format_name;
> int instance_size;
>
> - /* set to true if the BlockDriver is a block filter */
> + /* set to true if the BlockDriver is a block filter. Block filters pass
> + * certain callbacks that refer to data (see block.c) to their bs->file
> if
> + * the driver doesn't implement them. Drivers that do not wish to forward
> + * must implement them and return -ENOTSUP.
> + */
> bool is_filter;
I'm okay with the new comment.
With the swapped parameter order fixed (the maintainer can probably do
that),
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
- [Qemu-block] [PATCH v5 0/4] block: Block driver callbacks fixes, Manos Pitsidianakis, 2017/07/13
- [Qemu-block] [PATCH v5 4/4] block: add default implementations for bdrv_co_get_block_status(), Manos Pitsidianakis, 2017/07/13
- [Qemu-block] [PATCH v5 3/4] block: remove bdrv_truncate callback in blkdebug, Manos Pitsidianakis, 2017/07/13
- [Qemu-block] [PATCH v5 1/4] block: pass bdrv_* methods to bs->file by default in block filters, Manos Pitsidianakis, 2017/07/13
- Re: [Qemu-block] [PATCH v5 1/4] block: pass bdrv_* methods to bs->file by default in block filters,
Eric Blake <=
- [Qemu-block] [PATCH v5 2/4] block: remove unused bdrv_media_changed, Manos Pitsidianakis, 2017/07/13
- Re: [Qemu-block] [PATCH v5 0/4] block: Block driver callbacks fixes, Eric Blake, 2017/07/13