Re: [PATCH v4 27/34] block: Use child_of_bds in remaining places

[Eric Blake]

On 5/13/20 6:05 AM, Max Reitz wrote:
> Replace child_file by child_of_bds in all remaining places (excluding
> tests).
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
>   block.c              |  3 ++-
>   block/backup-top.c   |  4 ++--
>   block/blklogwrites.c |  4 ++--
>   block/raw-format.c   | 15 +++++++++++++--
>   4 files changed, 19 insertions(+), 7 deletions(-)

> @@ -448,8 +449,18 @@ static int raw_open(BlockDriverState *bs, QDict *options, 
> int flags,
>           return ret;
>       }
>   
> -    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file, 0,
> -                               false, errp);
> +    /*
> +     * Without offset and a size limit, this driver behaves very much
> +     * like a filter.  With any such limit, it does not.
> +     */
> +    if (offset || has_size) {
> +        file_role = BDRV_CHILD_DATA | BDRV_CHILD_PRIMARY;
> +    } else {
> +        file_role = BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY;
> +    }

I wonder if we'll hit subtle bugs where one but not the other use of raw 
breaks because of the difference in roles.  I know we have _some_ iotest 
coverage of raw with offsets, but wonder if we might need more (or a way 
to add it on top of existing images, similar to how we can easily toggle 
qcow2 options like refcount size for interesting non-default test runs).

> +
> +    bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
> +                               file_role, false, errp);
>       if (!bs->file) {
>           return -EINVAL;
>       }

At any rate, if we have corner-case bugs, I don't think this is making 
them worse.
Reviewed-by: Eric Blake <address@hidden>

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