Re: [Qemu-devel] [PATCH 1/5] block_int: add basic conversion api

[Stefan Hajnoczi]

On Tue, Jul 05, 2011 at 12:41:20AM -0400, Devin Nakamura wrote:

Please use 'block' instead of 'block_int' as the tag for this patch.  I
usually do git log <filename> to find out which tag is used for a
particular file.  'block_int' has never been used in qemu.git but
'block' is used on block layer changes.

> add functions to block driver interface to support inplace image conversion
> 
> Signed-off-by: Devin Nakamura <address@hidden>
> ---
>  block_int.h |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/block_int.h b/block_int.h
> index 1e265d2..ef311c7 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -136,6 +136,16 @@ struct BlockDriver {
>       * zeros, 0 otherwise.
>       */
>      int (*bdrv_has_zero_init)(BlockDriverState *bs);
> +    
> +    /*  Image conversion stuff */

How about:

/* In-place image conversion */

> +    int (*bdrv_open_conversion_target)(BlockDriverState *bs, char *filename,
> +        QEMUOptionParameter *options);
> +    int (*bdrv_get_mapping)(BlockDriverState *bs, uint64_t *guest_offset,
> +        uint64_t *host_offset, uint64_t *contiguous_bytes);
> +    int (*bdrv_map)(BlockDriverState *bs, uint64_t *guest_offset, 
> +        uint64_t *host_offset, uint64_t *contiguous_bytes);
> +    int (*bdrv_copy_header) (BlockDriverState *bs);

Please document these functions so people working on block drivers know what
semantics they must implement.  Unfortunately other interfaces are not
documented but I don't see a reason to avoid documentation in the future.

Important bits would be:
 * How is .bdrv_open_conversion_target() different from a regular .bdrv_open()?
 * Which BlockDriver functions are fair game to be called while opened as a 
conversion target?
 * Any constraints or assumptions when a block device is in conversion target 
mode?
 * How do the pointer arguments of .bdrv_get_mapping() behave?  Which arguments 
are inputs, outputs, input/output?
 * Does .bdrv_copy_header() actually copy any header or does it "install" or 
"write out" a new header?
 * After .bdrv_copy_header() is the block device upgraded to first-class 
operation or do you need to close and then reopen the image file?

Stefan