Re: [Qemu-devel] [RFC 03/24] block: add bdrv_open_conversion_target()

[Kevin Wolf <kwolf@redhat.com>]

Am 29.07.2011 06:49, schrieb Devin Nakamura:
> Conflicts:
> 
> 	block.h

You can probably remove this note. ;-)

> 
> Signed-off-by: Devin Nakamura <devin122@gmail.com>
> ---
>  block.c |   32 ++++++++++++++++++++++++++++++++
>  block.h |    4 ++++
>  2 files changed, 36 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 4503b7b..9530577 100644
> --- a/block.c
> +++ b/block.c
> @@ -3038,6 +3038,38 @@ out:
>      return ret;
>  }
>  
> +int bdrv_open_conversion_target(BlockDriverState **bs, BlockDriverState *file,
> +                                BlockConversionOptions *drv_options,
> +                                QEMUOptionParameter *usr_options,
> +                                const char *target_fmt)
> +{
> +    BlockDriver *drv;
> +    BlockDriverState *bss;
> +
> +    drv = bdrv_find_format(target_fmt);
> +    if (!drv) {
> +        return -ENOENT;
> +    }
> +
> +    if (!drv->bdrv_open_conversion_target) {
> +        return -ENOTSUP;
> +    }
> +
> +    *bs = bdrv_new("");
> +    bss = *bs;
> +    bss->file = file;
> +    bss->total_sectors = drv_options->image_size >> BDRV_SECTOR_BITS;
> +    bss->encrypted = 0;
> +    bss->valid_key = 0;
> +    bss->open_flags = 0;
> +    /* buffer_alignment defaulted to 512, drivers can change this value */
> +    bss->buffer_alignment = 512;
> +    bss->opaque = qemu_mallocz(drv->instance_size);
> +    bss->drv = drv;
> +    return drv->bdrv_open_conversion_target(bss, drv_options, usr_options);
> +
> +}

How big are the differences really to bdrv_open_common? Have you checked
if you could reuse it? It looks to me as if you're just handling fewer
cases and could achieve the same by passing the right flags to
bdrv_open_common. The only real difference is drv->bdrv_open vs.
drv->bdrv_open_conversion_target, which could probably be handled by
another flag.

The problem with keeping it separate is that it makes it easy to change
bdrv_open without changing bdrv_open_conversion_target. Most people
touching the code won't use in-place conversion very often, so they
won't notice any breakage in their testing.

Kevin