Re: [Qemu-devel] [PATCH 2/5] block: add bdrv_open_conversion_target

[Stefan Hajnoczi <stefanha@gmail.com>]

On Tue, Jul 05, 2011 at 12:41:21AM -0400, Devin Nakamura wrote:
> +int bdrv_open_conversion_target(BlockDriverState **bs,
> +        char *filename, char *target_fmt, QEMUOptionParameter *options)
> +{
> +    BlockDriver *drv;
> +
> +    drv = bdrv_find_format(target_fmt);
> +    if(!drv){

Please run scripts/checkpatch.pl before submitting patches.  It will
report coding style issues.

> +        return -ENOENT;
> +    }
> +
> +    if(!drv->bdrv_open_conversion_target){
> +        return -ENOTSUP;
> +    }
> +    *bs = bdrv_new("");
> +    (*bs)->opaque = qemu_malloc(drv->instance_size);

Please use qemu_mallocz() like bdrv_open_common() does.  This is safer
in case a block driver relies on the memory being zeroed (which is a
useful property).

> +    return drv->bdrv_open_conversion_target(*bs, filename, options);

Have you considered using bdrv_open_common() with a flag to avoid
duplicating bs initialization code?  You have not dealt with several
fields that bdrv_open_common() initializes.

Stefan